Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: context enrichment via contextEnricher, not from init #991

Merged
merged 6 commits into from
Oct 3, 2024

Conversation

toddbaert
Copy link
Member

@toddbaert toddbaert commented Sep 30, 2024

  • ⚠️ provider no longer merges context passed at init into each evaluation (this was non-standard behavior
  • contextEnricher option added which does the above, and more
    • runs with every (re)connection
    • updates per-evaluation context, optionally with data from sync-metadata response.

Continuation of #967.

@toddbaert toddbaert requested a review from a team as a code owner September 30, 2024 20:31
@toddbaert toddbaert changed the title feat!: context enrichment via contextEnricher func feat!: context enrichment via contextEnricher, not from init Sep 30, 2024
* SyncMetadata.
* Set on initial connection and updated with every reconnection.
* see:
* https://buf.build/open-feature/flagd/docs/main:flagd.sync.v1#flagd.sync.v1.FlagSyncService.GetMetadata
*
* @return Object map representing sync metadata
*/
protected Map<String, Object> getSyncMetadata() {
return Collections.unmodifiableMap(syncMetadata);
protected Structure getSyncMetadata() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a much safer type which we already use for this sort of thing.

@@ -149,6 +158,7 @@ private void onConnectionEvent(ConnectionEvent connectionEvent) {
boolean previous = connected;
boolean current = connected = connectionEvent.isConnected();
syncMetadata = connectionEvent.getSyncMetadata();
enrichedContext = contextEnricher.apply(connectionEvent.getSyncMetadata());
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Every time the resolver connects, we update the cached context by re-running the passed enricher.

@@ -177,6 +177,12 @@ By default, the provider is configured to
use [least recently used (lru)](https://commons.apache.org/proper/commons-collections/apidocs/org/apache/commons/collections4/map/LRUMap.html)
caching with up to 1000 entries.

##### Context enrichment
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be used to select a subset of fields from the getMetadata response. By default, all of it is injected into the context.

providers/flagd/README.md Outdated Show resolved Hide resolved
providers/flagd/README.md Outdated Show resolved Hide resolved
@toddbaert toddbaert requested a review from aepfli October 1, 2024 13:26
@toddbaert toddbaert force-pushed the feat/sync-metadata branch 2 times, most recently from ffa1893 to faa9e80 Compare October 1, 2024 21:33
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
…s/flagd/SyncMetadataHook.java

Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
@toddbaert toddbaert merged commit 1c2e11b into main Oct 3, 2024
4 checks passed
@toddbaert toddbaert deleted the feat/sync-metadata branch October 3, 2024 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants