Add initial support in the sidecar for feature flags #76

rhauch · 2024-10-09T22:08:24Z

Summary of Changes

Fixes #75

This adds the infrastructure for using LaunchDarkly feature flags and exposing a new REST API to allow the VS Code extension to get the current evaluated value for a specified feature flag.

Any additional details or context that should be provided?

LaunchDarkly does not yet have a Java client SDK that uses a non-secret client ID. (There is a Java server SDK, but this is intended as a multitenant services and requires an API key and secret that should not be shared.)

This framework uses a simple HTTP client to poll the HTTP API to get the currently evaluated flags given a context. The flags are evaluated on a schedule (e.g., every 60 seconds) and cached. If the sidecar is unable to evaluate the flags the first time (e.g., LaunchDarkly is not available when the sidecar starts), a set of default flags is used. However, as soon as flags can be evaluated the results are cached. Subsequent failures to refresh the flags (e.g., LaunchDarkly goes down after the sidecar process was started) will simply keep the cached flags (there is no TTL).

This framework also allows code within the sidecar to easily check feature flags. Simply @Inject a FeatureFlags reference, and call any of the methods to get feature flag values given a FlagId enum literal. There are default values defined for all FlagIds (verified by unit tests), which means the methods will always return a value for those feature flags.

To add a feature flag:

Define the flag in LaunchDarkly
Add a literal to the FlagId enum
Add a default value to the src/main/resources/feature-flag-defaults.json file
Use the flag in your code by calling a FeatureFlags.evaluateAs<ValueType>() method
Profit!

Because the sidecar version is needed in the device context, this also refactors how that information is kept (moving it out of the VersionResource and into a new SidecarInfo application bean with several other attributes. This bean and the various attributes can be injected into other beans, including the new FeatureFlags.

Currently the OS information is read from the System properties, and the VS Code version and extension version information is read from either System properties (lowercase dot-delimited), such as

ide-sidecar-*-runner -Dvscode.version=1.93.2 -Dvscode.extension.version=v0.17.1 …

or if those are absent from environment variables (capital case underscore-delimited) used to start the sidecar process, such as:

VSCODE_VERSION=1.93.1 VSCODE_EXTENSION_VERSION=v0.17.0 ide-sidecar-*-runner …

These are currently optional, but we do want to change the extension to start supplying them soon, as the VS Code information is passed in the device contexts passed to LaunchDarkly during flag evaluation.

Pull request checklist

Please check if your PR fulfills the following (if applicable):

Tests:
- Added new
- Updated existing
- Deleted existing
Have you validated this change locally against a running instance of the Quarkus dev server?
```
make quarkus-dev
```
Have you validated this change against a locally running native executable?
```
make mvn-package-native && ./target/ide-sidecar-*-runner
```

This adds the infrastructure for using LaunchDarkly feature flags and exposing a new REST API to allow the VS Code extension to _list_ feature flags, _read_ a specific feature flag (including the current evaluated value of the flag), and get just the currently evaluated value of a specific feature flag. LaunchDarkly does not yet have a Java client SDK that uses a non-secret client ID. (There is a Java server SDK, but this is intended as a multitenant services and requires an API key and secret that should not be shared.) This framework uses a simple HTTP client to poll the HTTP API to get the currently evaluated flags given a context. The flags are evaluated on a schedule (e.g., every 60 seconds) and cached. If the sidecar is unable to evaluate the flags the first time (e.g., LaunchDarkly is not available when the sidecar starts), a set of default flags is used. However, as soon as flags can be evaluated the results are cached. Subsequent failures to refresh the flags (e.g., LaunchDarkly goes down after the sidecar process was started) will simply keep the cached flags (there is no TTL). This framework also allows code within the sidecar to easily check feature flags. The `FeatureFlag` methods to get feature flag values require `FlagId` enum literals, and unit tests ensure that there is a default value for all `FlagId` literals. This means there is always a value for these flags, and centralizes the default values in a JSON file on the classpath (the same format that is returned from LaunchDarkly). Because the sidecar version is needed in the device context, this also refactors how that information is kept (moving it out of the `VersionResource` and into a new `SidecarInfo` application bean with several other attributes. This bean and the various attributes can be injected into other beans, including the new `FeatureFlags`. Currently the OS information is read from the System properties, and the VS Code version and extension version information is read from either System properties (lowercase dot-delimited), such as ``` ide-sidecar-*-runner -Dvscode.version=1.93.2 -Dvscode.extension.version=v0.17.1 … ``` or if those are absent from environment variables (capital case underscore-delimited) used to start the sidecar process, such as: ``` VSCODE_VERSION=1.93.1 VSCODE_EXTENSION_VERSION=v0.17.0 ide-sidecar-*-runner … ``` These are currently optional, but we do want to change the extension to start supplying them soon, as the VS Code information is passed in the `device` contexts passed to LaunchDarkly during flag evaluation.

rhauch · 2024-10-09T22:18:18Z

Users should see very little difference in the sidecar logs. They may see errors if there are problems parsing the LD results, but otherwise the FeatureFlags class and its supporting classes are pretty silent during operation. We don't want log problems connecting to LaunchDarkly, since a user might not have internet connectivity.

There are quite a few debug messages that we can see during development, if we enable debug logs.

Anyway, here's an example startup:

$ ./target/ide-sidecar-0.26.0-runner -Dvscode.version=v1.93.2 -Dvscode.extension.version=v0.17.1

with resulting log:

2024-10-09 17:09:47,342 INFO  [io.con.ide.res.app.LifecycleBean] (main) Sidecar starting...
2024-10-09 17:09:47,400 INFO  [io.con.ide.res.app.BeanProducers] (main) Extracted templates to /var/folders/yf/kdn74cm97_z5_jdbx_cygb_m0000gq/T/12022813187248413278
2024-10-09 17:09:47,405 INFO  [io.con.ide.res.app.SidecarInfo] (main) OS: Mac OS X 13.7 (MacOS); VS Code 1.93.2, extension version 0.17.1
2024-10-09 17:09:47,405 INFO  [io.con.ide.res.fea.FeatureFlags] (main) Checking for current feature flags in projects [IDE, Confluent Cloud], using 5 defaults, 0 overrides
2024-10-09 17:09:47,409 INFO  [io.con.ide.res.fea.FeatureFlags] (main) Next evaluation of feature flags will be after 5:10:47 PM (in 60s)
2024-10-09 17:09:47,543 INFO  [io.quarkus] (main) ide-sidecar 0.26.0 native (powered by Quarkus 3.13.3) started in 0.230s. Listening on: http://127.0.0.1:26636
2024-10-09 17:09:47,543 INFO  [io.quarkus] (main) Profile prod activated. 
2024-10-09 17:09:47,543 INFO  [io.quarkus] (main) Installed features: [cdi, config-yaml, hibernate-validator, qute, qute-web, reactive-routes, rest, rest-jackson, scheduler, smallrye-context-propagation, smallrye-graphql, smallrye-health, smallrye-openapi, vertx]
2024-10-09 17:09:48,002 INFO  [io.con.ide.res.app.KnownWorkspacesBean] (executor-thread-2) Grim reaper: Will check if the sidecar should remain alive periodically.

There are just a few new log messages. The third line in the output (repeated below) is new:

2024-10-09 17:09:47,405 INFO  [io.con.ide.res.app.SidecarInfo] (main) OS: Mac OS X 13.7 (MacOS); VS Code 1.93.2, extension version 0.17.1

Note that if the startup command excludes the -Dvscode.version=v1.93.2 -Dvscode.extension.version=v0.17.1:

$ ./target/ide-sidecar-0.26.0-runner

then this log message becomes:

2024-10-09 17:11:26,240 INFO  [io.con.ide.res.app.SidecarInfo] (main) OS: Mac OS X 13.7 (MacOS); VS Code unknown, extension version unknown

and the VS Code version and extension version information is not included in the LD contexts during feature flag evaluation.

The other new log message is this:

2024-10-09 17:09:47,409 INFO  [io.con.ide.res.fea.FeatureFlags] (main) Next evaluation of feature flags will be after 5:10:47 PM (in 60s)

which will occur again roughly every 60 seconds, or whenever the CCloud connection authenticates or disconnects (since this changes the LD context used for feature flag evaluations).

flippingbits

Great work, @rhauch! I have a few questions/comments. Otherwise, your PR looks good to me.

flippingbits · 2024-10-10T11:46:10Z

src/main/java/io/confluent/idesidecar/restapi/exceptions/ExceptionMappers.java

+ null
+ );
+ return Response
+ .status(Status.NOT_FOUND)


Should this be Status.INTERNAL_SERVER_ERROR so that it's aligned with the Failure object?

If you decide to use Status.INTERNAL_SERVER_ERROR, please also update the responses in the OpenAPI spec.

This only happens if the sidecar is unable to parse the LD response, so I would consider it an internal error. But as you suggested, I will add 500 to the OpenAPI spec.

Made this change.

Using Status.INTERNAL_SERVER_ERROR makes a lot of sense here. Can you please also update the Response.status in the exception mapper function?

Actually, the FeatureFlagFailureException are logged, since that means we had trouble parsing the LD response, but that parsing and this error only occur on background threads and does not occur on REST API request threads. So I'm going to remove this exception mapping.

...main/java/io/confluent/idesidecar/restapi/exceptions/ParsingFeatureFlagsFailedException.java

src/main/java/io/confluent/idesidecar/restapi/featureflags/FeatureFlags.java

src/main/java/io/confluent/idesidecar/restapi/featureflags/HttpFlagEvaluationProvider.java

src/main/java/io/confluent/idesidecar/restapi/util/OperatingSystemType.java

Refactored the `FeatureFlagsFailedException` and how the LD error response is parsed and that exception object thrown. Also changed the `WebClientFactory` fields to use an `@Inject`-ed instance. Since there are few beans in the `featureflags` package, this change required passing the `WebClientFactory` into the other objects in a different way. In particular, the `FeatureProject` objects are not beans and are instead instantied statically, before the `WebClientFactory` field could be injected. So now the `WebClientFactory` is `@Inject`-ed into the `FeatureFlags` bean, and passed into the `FeatureProject.refresh(…)` method, which is where the client factory is used. These changes (particular the second) required quite a few simple changes to several test cases.

…at application startup With the `@Startup` annotation, the `FeatureFlags` and `SidecarInfo` beans are initialized when the sidecar starts up. The LD flags are evaluated immediately and likely ready before the flags are used in the sidecar or via the REST API. Without that annotation, those beans are initialized only when they are used. In the case of `FeatureFlags`, that delayed initialization would be too late after the first feature flag was requested, as the evaluation of the feature flags would likely not yet have completed. For example, if a REST request were for a feature flag with a default, the default would have been used. But if the request were for a feature flag that does not have a default (i.e., no `FlagId` enum literal, as is the case for all CCloud flags), the REST request response would have been a `404 Not Found` signifying the feature flag does not exist.

src/main/java/io/confluent/idesidecar/restapi/application/SidecarInfo.java

src/main/java/io/confluent/idesidecar/restapi/featureflags/FeatureFlags.java

src/main/java/io/confluent/idesidecar/restapi/util/OperatingSystemType.java

flippingbits

Thanks for the update, @rhauch! I added a few more minor comments/questions.

Update: Argh - I accidentally created two reviews. Please let me know if you can't see my comments.

Improved the handling of `FeatureFlagFailureException`. The `HttpFlagEvaluationProvider` emits it when it is unable to parse the LD evaluation response or an error response. Since the feature flags are always evaluated on a separate thread, while local checks of feature flags can occur on any thread and only hit the cached values. Therefore, `FeatureFlagFailureException` is never exposed to users, and doesn’t need to be in the `ExceptionMappers`.

rhauch · 2024-10-11T19:37:21Z

Thanks for the review and feedback, @flippingbits. I've made a few changes based upon your suggestions. Would you please take another look?

flippingbits

Excellent work, @rhauch!

rhauch · 2024-10-11T21:04:03Z

Thanks, @flippingbits. I've added the on-hold label since I'm waiting for a few more reviews before I merge this.

rhauch · 2024-10-17T12:55:48Z

Actually, still on-hold because we're waiting for some final corrections to the IDE project in Confluent's LaunchDarkly.

rhauch added 3 commits October 9, 2024 13:15

Use the new OperatingSystemType enum and SidecarInfo bean

396c7bd

Remove any unanticipated feature flags

99b5563

rhauch requested a review from a team as a code owner October 9, 2024 22:08

rhauch changed the title ~~Feature flags~~ Add initial support in the sidecar for feature flags Oct 10, 2024

flippingbits reviewed Oct 10, 2024

View reviewed changes

rhauch added 2 commits October 10, 2024 13:35

flippingbits reviewed Oct 11, 2024

View reviewed changes

flippingbits self-requested a review October 11, 2024 07:12

flippingbits reviewed Oct 11, 2024

View reviewed changes

flippingbits approved these changes Oct 11, 2024

View reviewed changes

rhauch added the on-hold label Oct 11, 2024

rhauch marked this pull request as draft October 11, 2024 21:04

rhauch marked this pull request as ready for review October 15, 2024 14:50

rhauch added on-hold and removed on-hold labels Oct 17, 2024

rhauch modified the milestones: 2024 Q4 General Improvements, Feature Flag Support via LaunchDarkly Oct 18, 2024

rhauch self-assigned this Oct 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add initial support in the sidecar for feature flags #76

Add initial support in the sidecar for feature flags #76

rhauch commented Oct 9, 2024 •

edited

Loading

rhauch commented Oct 9, 2024

flippingbits left a comment •

edited

Loading

flippingbits Oct 10, 2024

rhauch Oct 10, 2024

rhauch Oct 10, 2024

flippingbits Oct 11, 2024

rhauch Oct 11, 2024

flippingbits left a comment •

edited

Loading

rhauch commented Oct 11, 2024

flippingbits left a comment

rhauch commented Oct 11, 2024

rhauch commented Oct 17, 2024

Add initial support in the sidecar for feature flags #76

Are you sure you want to change the base?

Add initial support in the sidecar for feature flags #76

Conversation

rhauch commented Oct 9, 2024 • edited Loading

Summary of Changes

Any additional details or context that should be provided?

Pull request checklist

rhauch commented Oct 9, 2024

flippingbits left a comment • edited Loading

Choose a reason for hiding this comment

flippingbits Oct 10, 2024

Choose a reason for hiding this comment

rhauch Oct 10, 2024

Choose a reason for hiding this comment

rhauch Oct 10, 2024

Choose a reason for hiding this comment

flippingbits Oct 11, 2024

Choose a reason for hiding this comment

rhauch Oct 11, 2024

Choose a reason for hiding this comment

flippingbits left a comment • edited Loading

Choose a reason for hiding this comment

rhauch commented Oct 11, 2024

flippingbits left a comment

Choose a reason for hiding this comment

rhauch commented Oct 11, 2024

rhauch commented Oct 17, 2024

rhauch commented Oct 9, 2024 •

edited

Loading

flippingbits left a comment •

edited

Loading

flippingbits left a comment •

edited

Loading