feat(flagd): add new context enrichment approach for in-process provider #730
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aepfli
force-pushed
the
feat/contextValues
branch
2 times, most recently
from
fa7a42d to
30a0194
Compare
aepfli
commented
Jul 30, 2025
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Summary of ChangesThis pull request enhances the in-process flagd provider by introducing a flexible context enrichment mechanism. It allows users to define custom functions to modify or extend the evaluation context dynamically, ensuring that flag evaluations can incorporate additional, derived, or external data. This is achieved through a new "ContextEnricher" configuration option and its integration via a "SyncContextHook", providing a powerful way to customize flag evaluation behavior. Highlights
Changelog
Activity
|
|
/gemini review |
Signed-off-by: Simon Schrottner <[email protected]> diff --git c/providers/flagd/pkg/provider.go i/providers/flagd/pkg/provider.go index 2b0084c..cf52f01 100644 --- c/providers/flagd/pkg/provider.go +++ i/providers/flagd/pkg/provider.go @@ -22,8 +22,8 @@ type Provider struct { service IService status of.State mtx parallel.RWMutex - - eventStream chan of.Event + hooks []of.Hook + eventStream chan of.Event } func NewProvider(opts ...ProviderOption) (*Provider, error) { @@ -38,6 +38,7 @@ func NewProvider(opts ...ProviderOption) (*Provider, error) { eventStream: make(chan of.Event), providerConfiguration: providerConfiguration, status: of.NotReadyState, + hooks: []of.Hook{}, } cacheService := cache.NewCacheService( @@ -60,7 +61,7 @@ func NewProvider(opts ...ProviderOption) (*Provider, error) { provider.providerConfiguration.log, provider.providerConfiguration.EventStreamConnectionMaxAttempts) } else if provider.providerConfiguration.Resolver == inProcess { - service = process.NewInProcessService(process.Configuration{ + inprocess_service := process.NewInProcessService(process.Configuration{ Host: provider.providerConfiguration.Host, Port: provider.providerConfiguration.Port, ProviderID: provider.providerConfiguration.ProviderId, @@ -73,6 +74,13 @@ func NewProvider(opts ...ProviderOption) (*Provider, error) { CustomSyncProviderUri: provider.providerConfiguration.CustomSyncProviderUri, GrpcDialOptionsOverride: provider.providerConfiguration.GrpcDialOptionsOverride, }) + provider.hooks = append(provider.hooks, NewSyncContextHook( + func() *of.EvaluationContext { + evaluationContext := of.NewTargetlessEvaluationContext(inprocess_service.ContextValues) + return &evaluationContext + })) + service = inprocess_service + } else { service = process.NewInProcessService(process.Configuration{ OfflineFlagSource: provider.providerConfiguration.OfflineFlagSourcePath, @@ -145,7 +153,7 @@ func (p *Provider) EventChannel() <-chan of.Event { // Hooks flagd provider does not have any hooks, returns empty slice func (p *Provider) Hooks() []of.Hook { - return []of.Hook{} + return p.hooks } // Metadata returns value of Metadata (name of current service, exposed to openfeature sdk) diff --git c/providers/flagd/pkg/service/in_process/service.go i/providers/flagd/pkg/service/in_process/service.go index 468b1c6..4f1c2ee 100644 --- c/providers/flagd/pkg/service/in_process/service.go +++ i/providers/flagd/pkg/service/in_process/service.go @@ -3,7 +3,6 @@ package process import ( "context" "fmt" - "regexp" parallel "sync" @@ -33,6 +32,7 @@ type InProcess struct { sync sync.ISync syncEnd context.CancelFunc wg parallel.WaitGroup + ContextValues map[string]any } type Configuration struct { @@ -113,6 +113,10 @@ func (i *InProcess) Init() error { case data := <-syncChan: // re-syncs are ignored as we only support single flag sync source changes, _, err := i.evaluator.SetState(data) + if data.SyncContext != nil { + i.ContextValues = data.SyncContext.AsMap() + } + if err != nil { i.events <- of.Event{ ProviderName: "flagd", EventType: of.ProviderError, diff --git c/providers/flagd/pkg/service/in_process/service_grpc_test.go i/providers/flagd/pkg/service/in_process/service_grpc_test.go index a1e7c1c..c271161 100644 --- c/providers/flagd/pkg/service/in_process/service_grpc_test.go +++ i/providers/flagd/pkg/service/in_process/service_grpc_test.go @@ -7,6 +7,7 @@ import ( "fmt" "github.com/open-feature/go-sdk/openfeature" "google.golang.org/grpc" + "google.golang.org/protobuf/types/known/structpb" "log" "net" "testing" @@ -38,11 +39,20 @@ func TestInProcessProviderEvaluation(t *testing.T) { t.Fatal(err) } + m := make(map[string]any) + + m["context"] = "set" + syncContext, err := structpb.NewStruct(m) + if err != nil { + t.Fatal(err) + } + bufServ := &bufferedServer{ listener: listen, mockResponses: []*v1.SyncFlagsResponse{ { FlagConfiguration: flagRsp, + SyncContext: syncContext, }, }, fetchAllFlagsResponse: nil, @@ -112,6 +122,10 @@ func TestInProcessProviderEvaluation(t *testing.T) { if scope != detail.FlagMetadata["scope"] { t.Fatalf("Wrong scope value. Expected %s, but got %s", scope, detail.FlagMetadata["scope"]) } + + if len(inProcessService.ContextValues) == 0 { + t.Fatal("Expected context_values to be present, but got none") + } } // custom name resolver @@ -138,7 +152,7 @@ func TestInProcessProviderEvaluationEnvoy(t *testing.T) { } inProcessService := NewInProcessService(Configuration{ - TargetUri: "envoy://localhost:9211/foo.service", + TargetUri: "envoy://localhost:9211/foo.service", Selector: scope, TLSEnabled: false, }) @@ -201,7 +215,6 @@ func TestInProcessProviderEvaluationEnvoy(t *testing.T) { } } - // bufferedServer - a mock grpc service backed by buffered connection type bufferedServer struct { listener net.Listener diff --git c/providers/flagd/pkg/sync_context_hook.go i/providers/flagd/pkg/sync_context_hook.go new file mode 100644 index 0000000..ebb6590 --- /dev/null +++ i/providers/flagd/pkg/sync_context_hook.go @@ -0,0 +1,21 @@ +package flagd + +import ( + "context" + "github.com/open-feature/go-sdk/openfeature" +) + +type Supplier func() *openfeature.EvaluationContext + +type SyncContextHook struct { + openfeature.UnimplementedHook + contextSupplier Supplier +} + +func NewSyncContextHook(contextSupplier Supplier) SyncContextHook { + return SyncContextHook{contextSupplier: contextSupplier} +} + +func (hook SyncContextHook) Before(ctx context.Context, hookContext openfeature.HookContext, hookHints openfeature.HookHints) (*openfeature.EvaluationContext, error) { + return hook.contextSupplier(), nil +}
Signed-off-by: Simon Schrottner <[email protected]>
Signed-off-by: Simon Schrottner <[email protected]>
aepfli
force-pushed
the
feat/contextValues
branch
from
2106a06 to
b6c9664
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces a context enrichment feature for the in-process provider, allowing static context from a sync source to be merged into the evaluation context. The implementation correctly uses OpenFeature hooks. However, I've found a potential race condition in the in-process service that needs to be addressed. I also have a few suggestions to improve code clarity and stick to Go idioms.
Signed-off-by: Simon Schrottner <[email protected]>
aepfli
force-pushed
the
feat/contextValues
branch
from
81e621d to
0286738
Compare
Signed-off-by: Simon Schrottner <[email protected]>
aepfli
force-pushed
the
feat/contextValues
branch
from
ab45615 to
38fd36c
Compare
chrfwow
reviewed
Oct 6, 2025
| // re-syncs are ignored as we only support single flag sync source | ||
| changes, _, err := i.evaluator.SetState(data) | ||
| if data.SyncContext != nil { | ||
| i.mtx.Lock() |
There was a problem hiding this comment.
Is it possible to add a defer i.mtx.Unlock() here?
There was a problem hiding this comment.
where do you see the benefit?
There was a problem hiding this comment.
Mostly for consistency. But you are right, it's just the Java dev in me that sees lock/unlock outside of a try/finally context and becomes uneasy
toddbaert
changed the title
toddbaert
reviewed
Oct 7, 2025
| changes, _, err := i.evaluator.SetState(data) | ||
| if data.SyncContext != nil { | ||
| i.mtx.Lock() | ||
| i.contextValues = data.SyncContext.AsMap() |
There was a problem hiding this comment.
In java we have an optional lambda to manipulate this... I think it should be simple to add it here.
There was a problem hiding this comment.
Oh, I think you added this in the hook itself.
In the Java impl, we only run this transform function when we actually get the sync data. I think there's a reason to do it in the hook so many times, when we can do it before we store the context.
chrfwow
approved these changes
Oct 14, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
trying to add context value enrichment approach for in-process mode.
Long time no golang, so this might be a little bit of sub optimal approach open for feedback/insights