open-feature / java-sdk-contrib Goto Github PK

https://github.com/open-feature/java-sdk-contrib/tree/main/providers/flagd

https://github.com/open-feature/java-sdk-contrib/blob/main/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdProvider.java#L56
It's not always guaranteed that FeatureProvider.shutdown() method is called at the end of application lifecycle. The user can call the shutdown method at any point in time of application lifetime. So it's better to reset the state. may be NOT_READY or new state and publish the event with the new state.

Background

Consider OFEP [1] - open-feature/ofep#62

Go-SDK hook implementation is ready[2] and this issue focuses on Java-sdk implementation of the hook.

Outcome

OTel hooks for metrics as define by OFEP

[1] - open-feature/ofep#62
[2] - open-feature/go-sdk-contrib#217

See #244 (comment)

Minor comment (potential improvement)

We could make this an interface (contract) and have a default implementation based on System.getenv(name)

That would allow having custom env variable handling (ex:- prefixes, casing and even sources like spring configurations which come from different sources [1] ) and still use the same provider

Could be a follow up work :)

[1] - https://docs.spring.io/spring-boot/docs/1.0.1.RELEASE/reference/html/boot-features-external-config.html

See here, and the linked issue for details and background. This blog post is also helpful, specifically for library maintainers.

In each module in this project, we should add this maven step (we could also add it in the parent POM and somehow grab the name dynamically). We should be careful that the module name matches the "highest level" package in the module.

Once the java-sdk has events and init/shutdown support, we need to update the flagd-provider to emit events and do proper init/shutdown. We already have a stream implemented for cache-busing, so this should be pretty simple.

Definition of done:

• flagd does does startup logic (like initializing its stream) in initialize function
• flagd shuts down stream in close/shutdown function
• flagd emits appropriate events on flag changes, and errors
• associated testing

Blocked by:

• java-sdk implementation: open-feature/java-sdk#437

Resources:

• flag provider guide

This issue lists Renovate updates and detected dependencies. Read the Dependency Dashboard docs to learn more.

Open

These updates have all been created already. Click a checkbox below to force a retry/rebase of any.

• chore(deps): update actions/checkout digest to 0ad4b8f
• chore(deps): update dependency org.apache.maven.plugins:maven-pmd-plugin to v3.22.0

Ignored or Blocked

These are blocked by an existing closed PR and will not be recreated unless you click a checkbox below.

• chore(deps): update dependency org.honton.chas:exists-maven-plugin to v0.13.0
• chore(deps): update dependency uk.org.webcompere:system-stubs-core to v2.1.6
• chore(deps): update dependency com.github.tomakehurst:wiremock-jre8 to v3
• chore(deps): update dependency com.puppycrawl.tools:checkstyle to v10
• chore(deps): update mockito monorepo to v5 (major) (org.mockito:mockito-inline, org.mockito:mockito-junit-jupiter, org.mockito:mockito-core)

Detected dependencies

• Check this box to trigger a request for Renovate to run again on this repository

We have a null pointer when the field anonymous is not set in the GO Feature Flag provider.

This issue is here for me to remember to work on it.

We need to configure renovate.

The testing harness has introduced a caching suite, tests for this should be implemented.

The flagd provider should start an OTel span that will be passed to flagd to get cross-service tracing for evaluation.

Add OTel hook implementation plus tests.

See https://github.com/open-feature/node-sdk-contrib/tree/main/libs/hooks/open-telemetry

Moved from: open-feature/java-sdk#339

JS SDK already has a simple env var provider implementation here. It may be used as an example.

• Add env var reading for configuration (see: https://github.com/open-feature/golang-sdk-contrib/blob/eabb362ad36cf032ff277ecfd91e584d096b00dd/providers/flagd/pkg/provider.go#L92)
• Add unix socket support
• Do NOT add HTTP support (GRPC only)

Problems

• Everything is bundled together into a single namespace which makes it hard to understand the responsibilities of each class.
• The current main commit allows for external entities to change the internal state of the communication of Flagd Provider. IMHO we should remove this from the API surface.
• Unit and Integration tests are hard to write
• Review opportunities to add log.debug statements

Suggestion

Refactor into modules as proposed:

Summary

Add a contributing document to the root of the repo. People interested in contributing for the first time can use this document to understand how they should get started. For a basic example, please refer to the contributing docs in the js contrib repo.

Similar to go sdk hook, the java hook can be improved with the Span error state

Full context - open-feature/go-sdk-contrib#204

The current implementation of OpenFeature's error hook in otel does not set the span status to error, despite it being recommended in the OTEL specification to do so

Also, consider providing an option to override default behavior as highlighted at open-feature/go-sdk-contrib#208

Currently it's possible to map flag metadata values to custom OTel attributes (see here). However this approach has limitations, as it can only map values 1:1.

We should implement something similar to the JS implementation, which allows consumers to provide a function that takes the flag metadata and returns OTel attributes.

We should add the same functionality to the Tracing hook.

Implement caching based on the strategy defined here.

Go implementation, for reference: open-feature/go-sdk-contrib#48

We need to absorb the (minor) changes from: open-feature/java-sdk#61

Add a CODEOWNERS file with appropriate persons per artifact, as well as root-level CODEOWNERS.

We don't yet have e2e tests for flag changes or zero-value flags.

These are both defined in the flagd.feature test suite, and supported by the sync-testbed and flagd-testbed containers.

Tasks:

• add matching steps defined in the feature file to the steps
  • note the Flag change event should fire automatically from both testbed containers, so there's no need to manually change the flags
• load the feature file in both cucumber runs (RPC and in-process)

flagd includes scope as a flag evaluation metadata ¹ which correlates to grpc proxy selector ². Given in-process evaluator can works with grpc proxy, we can include scope in in-process implementation.

Acceptance Criteria

In-process provider with grpc flag syncs includes selector as scope in flag evaluation metadata similar to flagd implementation.

We can probably use generics/lambdas to reduce some duplication across the resolvers.

The flagd in-process evaluator needs to have $flagd.timestamp added, so that users can write time-based JsonLogic rules.

See here for doc, and here for the flagd-standalone implementation.

You'll need to add a standard unix timestamp (seconds since 1970) to the evaluation context, similar to the flagKey, here.

When evaluating a flag via the client, provider should handle cases like:

• without evaluation context.
• empty evaluation context, without targeting key.

if not supported, handle accordingly with a proper error code and documentation.

Implement this.

Self-assigning.

Currently the flagd provider doesn't default to unlimited retries, and doesn't throw immediately if it can't start.

See: https://github.com/open-feature/flagd/blob/main/docs/other_resources/creating_providers.md#environment-variables and https://github.com/open-feature/flagd/blob/main/docs/other_resources/creating_providers.md#provider-lifecycle-initialization-and-shutdown

Definition of done:

• update the flagd provider to default to 0 (infinite) retries
• verify other defaults

Similar to go-sdk provider, we should allow to configure java provider's offline flag source path from an environment variable.

For consistency use same variable name - FLAGD_OFFLINE_FLAG_SOURCE_PATH

AC

Env variable is available and documentation updated accordingly

See - Originally posted by @Kavindu-Dodan in open-feature/go-sdk-contrib#421 (comment)

Flagsmith provider class and option class was created using package restricted access, so examples doesn't work.

For each artifact, we should include and SBOM. It should be published similar to the java-sdk's SBOM.

As per the recommendation from netty, we should add a guard before loading the Epoll classes. It seems like they will revert the change that caused our issue, but using this guard will also help us to give helpful error messages if this native lib is missing.

cc @Kavindu-Dodan

With the addition of the new protos (flagd.evaluation.v1, flagd.sync.v1), we must update all clients (flagd providers) to use these endpoints.

In Java the associated code is already being generated by the build process, we must simply use the new classes generated with the new protos.

new protos in buf: https://buf.build/open-feature/flagd
new proto source: https://github.com/open-feature/flagd-schemas/tree/main/protobuf/flagd 

Flagd sends empty values in flag response payloads when the value is equivalent to the zero-value of the associated go type. We should test that this works as expected and still returns an equivalent zero-value (ie: false, 0).

DoD:

• manual test confirming zero values are returned from the flagd provider when they are missing from the gRPC message (boolean set to false or numeric set to 0 in flagd config)
• new tests that assert correct values for zero-value flags

see: open-feature/flagd#218

The flagd in-process provider should be able to resolve ints as floats, and vice-versa.

To fix:

• modify this area of the code to support conversion from int/float, float/int
• add relevant tests here
• un-comment float tests here

Background:

Originally posted by @toddbaert in #468 (comment)

As suggested by this comment, our concurrency logic needs to be validated for race conditions. Though we expect initialization to be called once, there could be edge cases such as multiple provider registrations which could invoke unwanted initialization attempts.

Acceptance Criteria

In-process provider implementation is validated for potential race conditions and fixed.

This should be similar to what's in the java-sdk now.

Definition of done:

• run e2e tests based on test-harness ghekin suite and flagd test image. A lot of the code can be taken from the java SDK impl.
• run in CI

flagd's convertPrimitive(Value) results in an NPE as we explicitly set null [1] for context null values.

This must be corrected by using NullValue [2], which allows the representation to accept null values.

[1] - https://github.com/open-feature/java-sdk-contrib/blob/dev.openfeature.contrib.providers.flagd-v0.5.7/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdProvider.java#L389
[2] - https://protobuf.dev/reference/java/api-docs/com/google/protobuf/NullValue.html

As mentioned in flagd documentation of Creating an in-process flagd provider ¹, in-process provider comes with unique advantages. This issue focuses on creating the first version of flagd in-process provider in Java.

Requirements

• Fulfill requirements outlined by flagd in-process guideline ¹
• Minimal external dependencies
• Updated/New ReadMe & usage guide
• Unit & Integration tests

Technical requirements

• Use the same repository directory structure (providers/flagd)
• Build both remote and in-process providers into a single dependency (To be decided after the POC)
• Reuse existing interfaces and implementations where possible

Tasks

• POC implementation and build to detect packaging requirements (no evaluations needed)
• In-process provider with gRPC flag sync capability on basic flags
• Support for JsonLogic evaluators (complex flagd flag configurations)
• Support for flagd custom JsonLogic evaluators

We can consider implementing https://github.com/apps/sonarcloud, which is free for OSS projects. The CNCF doesn't seem to provide any free SAST tools.

Now that OTel 2529 is merged, we should implement the Open Telemetry hook (currently exists as a stub).

Basically, this would entail creating a hook which:

• in the after stage: gets the current span and adds an appropriately named event (feature_flag), with the attributes (feature_flag.key, feature_flag.provider_name, and feature_flag.variant)
• in the error stage, records the exception

In pseudocode, this looks something like:

class OTelHook implements Hook {
  after(context, details) {
    var currentTrace = trace.getActiveSpan();
    currentTrace.addEvent("feature_flag", { "feature_flag.key": details.key, /*  other props */ } );
  }

  error(err) {
    var currentTrace = trace.getActiveSpan();
    currentTrace.recordException(err);
  }
}

See: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/feature-flags.md

The JS and Go version options look like this:

| Option name | Environment variable name | Type    | Default   |
| ----------- | ------------------------- | ------- | --------- |
| host        | FLAGD_HOST                | string  | localhost |
| port        | FLAGD_PORT                | number  | 8013      |
| tls         | FLAGD_TLS                 | boolean | false     |
| socketPath  | FLAGD_SOCKET_PATH         | string  | -         |
| certPath    | FLAGD_SERVER_CERT_PATH    | string  | -         |

We need to change the config to look like this, and add certPath and socketPath support.

This will also fix a bug with an unexported Protocol enum.

Should include:

• basic overview
• link to flagd repo
• installation instructions (maven xml and gradle line)
• create issue in https://github.com/open-feature/docs.openfeature.dev/issues to add link to provider catalog.

See #244 (comment):

One thing we've done in other ENV VAR providers is to map SCREAMING_SNAKE_CASE env vars to hyphen-case-keys. This may be desirable, since generally people like to use SCREAMIN_SNAKE for env vars, but that's generally not true for feature flag keys.

You could also create some transformer interface to allow users to control this translation.... and it doesn't have to be done in this PR.

Goal

Remove the methods annotated with @Deprecated in providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdProvider.java

Technical Details

With #294, several methods are deprecated because we moved to a builder pattern. We should remove them and the respective test code.

Dependencies

• #294
• #268