airbyte icon indicating copy to clipboard operation
airbyte copied to clipboard

Convert airbyte-workers to Micronaut

Open jdpgrailsdev opened this issue 3 years ago • 1 comments
trafficstars

What

  • Converts the airbyte-workers service from a custom application framework to a Micronaut-based service
  • Introduces dependency injection for management of resources
  • Uses Micronaut's application configuration mechanism for management of configuration properties

How

This PR converts the airbyte-workers service to use Micronaut for framework-level code, including dependency injection for management of resources and configuration management. The main changes involve removing the WorkerApp class and splitting its logic between the new Application.java class (used to start the Micronaut service), ApplicationInitializer.java (used to perform code execution upon service startup, such as registering workflows with Temporal) and the various Micronaut Factory classes and Micronaut annotated Singleton classes to ensure creation of those resources at startup for dependency injection. In addition to these changes, there are a few less than optimal changes introduced as a bridge between the existing code and conversion to Micronaut. These changes are temporary and will be further migrated towards what Micronaut considers the pattern in future PR's:

Configuration Management

The existing configuration management in the OSS project heavily uses environment variables. Currently, this is all controlled by the Config/EnvConfig interface/implementation. These classes not only include references to all of the environment variables used for configuration, but also includes some validation logic AND default values for missing configuration. In addition to this logic, there are three different ways that environment variables are currently handled at runtime:

  1. Environment variables are explicitly set in the .env file and passed through to the Docker container via the docker-compose.yml file.
  2. Environment variables are NOT set in the .env file, but are passed through to the Docker container via the docker-compose.yml, resulting in the variable being set to an empty string.
  3. Environment variables are NOT set in the .env file and are NOT declared in the docker-compose.yml, resulting in the environment variable not being present. In some cases, this will result in a default value being provided when retrieving the environment variable value via the Config/EnvConfig class.

Micronaut, on the other hand, provides a built-in configuration property management system. This system allows you to manage configuration via external configuration files, command line arguments or via environment variables directly. Typically, services provides an application.yml file that maps these environment variables to well named configuration properties, removing the need to litter the code with references to the environment variable names, which may change over time. Additionally, it allows you to centralize the declaration of default values when said environment variables are not present. For example:

my:
  config:
     property: ${MY_ENV_VAR:5}

The above maps the MY_ENV_VAR to a property named my.config.property, providing a default value of 5 if the environment variable is not set. However, due to #2 above, this causes problems for Micronaut, as it sees the empty string value as being a set value, leading to the default value being ignored. To solve for this (temporarily), you will see the use of the @Property annotation instead of the @Value annotation in spots in the code that use these env vars that are defaulted to an empty string by the Docker configuration. This is obviously not a sustainable solution and will require us to revisit how to define and pass through environment variables in order to have consistent references in the code.

Lombok and Micronaut

As both Lombok and Micronaut use AST transformations via annotation processing to perform their code changes, it is totally possible to use both at the same time in the code. However, there are limitations, particularly with using the @AllArgsConstructor Lombok annotation. Micronaut supports both constructor injection and directed field injection via the @Inject annotation for dependencies. When a declared constructor with arguments is present on the class (including the one created by Lombok), Micronaut will attempt to use that constructor for injection by matching existing singletons to the parameters. If there is only one bean of each type, it will work just fine. However, if any of those dependencies in the constructor argument requires a qualifier (e.g. the use of the @Named annotation to choose the proper instance when more than one exists) OR the parameter references a configuration property (e.g. the use of @Value or @Property), then it will not work as there is no way to have Lombok add these annotations to the generated constructor. In these cases, we either need to manually declare the constructor OR introduce an intermediate object that can use the annotations and have that object be injected into the class that uses the Lombok annotations.

Temporal and Micronaut

Temporal controls the instantiation of workflow instance classes at runtime, meaning that we cannot use Micronaut to create and manage those instances. However, we want those workflow implementation instances to be able to use objects managed by Micronaut, particularly to avoid having multiple ways to load and reference configuration properties. To solve for this, I adapted a solution from an open source project that attempts to solve this problem for Spring. Largely this will be a transparent change, but for those interested in the details, it introduces a proxy class and interceptor to ensure that we can link together singletons and configuration managed by Micronaut with the object managed by Temporal. This code can be found in the TemporalProxyHelper and TemporalActivityStubInterceptor classes.

Use of Factory Classes for Singleton Declaration

Where possible, I have added annotations directly to classes that can easily be adapted into a singleton managed by Micronaut. However, there is a bunch of legacy code that is either not in a formation that is easily convertible to a format that makes it easy to annotation and/or are used by other code in non-Micronaut services. For now, these beans are being declared via factory classes that allow us to create the object using existing mechanisms that are not very annotation-driven friendly (e.g. is created via a static method or currently violates inversion of control principles by creating dependency objects internally). To reduce the scope of the PR, changes to these objects have been deferred to another pass/after other consuming services have also been converted. Therefore, you will see a mix of classes that have the @Singleton annotation directly on them and those that are created via factories.

Configuration Property Enum Value Mapping

Existing code maps some environment variable values to Java Enum values. To support this, these values have been declared as beans in the ApplicationBeanFactory that read the string environment variable and map it to the associated Enum value.

Default Values for Conditional Beans

There are a few objects in the existing code that have different versions given the current configuration values. Examples of this include choosing the appropriate log storage implementation based on configuration. This is handled in Micronaut by creating the beans conditionally. There are two options for dealing with the case where none of the conditions are met:

  1. Create a default NONE option/no-op implementation that is returned for the singleton
  2. Make the factory creation method return Optional<X> instead of X type directly. This allows any objects that rely on this bean to receive an Optional and check to see if it is set before using.

You will see both choices in play in the PR. This was done to limit the amount of additional changes required in the code. In some cases, the consuming code always expected an implementation to be provided. In these cases, I have introduced no-op/none/default implementations to ensure the provided object is never null. In other cases, the code checked for existing of the implementation, allowing for the use of Optional. Obviously, we will want to standardize on one approach for consistency.

Recommended reading order

A meeting will be scheduled to walk through the changes in this PR. Most changes in the PR are related to dependency injection annotation/injection/changes to support DI.

Tests

Unit tests have been added to test the new components created to support Micronaut and the Micronaut/Temporal interaction. In addition, tests have been updated to reflect any changes related to dependency injection.

In addition, the application has been tested by running locally via Docker with the changes and verifying that syncs and resets work as expected. There has also been testing done to verify that the new control plane/data plane changes introduced to support the multi-cloud effort works.

cc: @pmossman @evantahler @salima-airbyte

jdpgrailsdev avatar Sep 08 '22 14:09 jdpgrailsdev

I got some concern about what is going to happen for the next projects we move to micronaut about the fact that they will need to geenratetons of Bean but it should be a problem for later.

@benmoriceau Agreed. I originally intended for us to identify, extract and annotate the singletons first before attempting to migrate a service. However, I quickly discovered that we needed to at least migrate one service to better understand how it would all work. Going forward, I hope that we will be able to identify the shared classes/singletons and move them to shared library before attempting to migrate the next service.

jdpgrailsdev avatar Sep 12 '22 19:09 jdpgrailsdev

@jdpgrailsdev this Micronaut update is causing our workers to constantly crash (see below). Any idea why this might be happening?

2022-09-19 20:44:51 ERROR i.m.r.Micronaut(handleStartupException):338 - Error starting Micronaut server: Failed to inject value for parameter [dslContext] of method [configDatabase] of class: io.airbyte.db.Database

image

kyle-cheung avatar Sep 19 '22 20:09 kyle-cheung

@jdpgrailsdev this Micronaut update is causing our workers to constantly crash (see below). Any idea why this might be happening?

2022-09-19 20:44:51 ERROR i.m.r.Micronaut(handleStartupException):338 - Error starting Micronaut server: Failed to inject value for parameter [dslContext] of method [configDatabase] of class: io.airbyte.db.Database

image

@kyle-cheung It is most likely missing the control environment which can be set using the MICRONAUT_ENVIRONMENTS environment variable: MICRONAUT_ENVIRONMENTS=control. Out of curiosity, are you using the provided Docker compose file or Helm charts?

jdpgrailsdev avatar Sep 19 '22 21:09 jdpgrailsdev

what value should be "control" or "control-plane" ? https://github.com/airbytehq/airbyte/blob/master/charts/airbyte/templates/env-configmap.yaml#L66

navi86 avatar Sep 29 '22 13:09 navi86

what value should be "control" or "control-plane" ? https://github.com/airbytehq/airbyte/blob/master/charts/airbyte/templates/env-configmap.yaml#L66

@navi86 We just merged a change that sets it to control-plane, however that only applies to version v0.40.10 and beyond. If you are on a version < v0.40.10, it is just control until you upgrade.

jdpgrailsdev avatar Sep 29 '22 14:09 jdpgrailsdev