Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions providers/flagd/internal/mock/service_mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 13 additions & 0 deletions providers/flagd/pkg/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package flagd
import (
"errors"
"fmt"
of "github.com/open-feature/go-sdk/openfeature"
"os"
"strconv"
"strings"
Expand Down Expand Up @@ -64,6 +65,7 @@ type ProviderConfiguration struct {
CustomSyncProvider sync.ISync
CustomSyncProviderUri string
GrpcDialOptionsOverride []grpc.DialOption
ContextEnricher ContextEnricher

log logr.Logger
}
Expand All @@ -77,6 +79,10 @@ func newDefaultConfiguration(log logr.Logger) *ProviderConfiguration {
MaxCacheSize: defaultMaxCacheSize,
Resolver: defaultResolver,
Tls: defaultTLS,
ContextEnricher: func(contextValues map[string]any) *of.EvaluationContext {
evaluationContext := of.NewTargetlessEvaluationContext(contextValues)
return &evaluationContext
},
}

p.updateFromEnvVar()
Expand Down Expand Up @@ -397,3 +403,10 @@ func WithGrpcDialOptionsOverride(grpcDialOptionsOverride []grpc.DialOption) Prov
p.GrpcDialOptionsOverride = grpcDialOptionsOverride
}
}

// WithContextEnricher allows to add a custom context enricher (BeforeHook)
func WithContextEnricher(contextEnricher ContextEnricher) ProviderOption {
return func(p *ProviderConfiguration) {
p.ContextEnricher = contextEnricher
}
}
1 change: 1 addition & 0 deletions providers/flagd/pkg/iservice.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,5 @@ type IService interface {
ResolveObject(ctx context.Context, key string, defaultValue interface{},
evalCtx map[string]interface{}) of.InterfaceResolutionDetail
EventChannel() <-chan of.Event
ContextValues() map[string]any
}
13 changes: 10 additions & 3 deletions providers/flagd/pkg/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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(
Expand Down Expand Up @@ -73,12 +74,18 @@ func NewProvider(opts ...ProviderOption) (*Provider, error) {
CustomSyncProviderUri: provider.providerConfiguration.CustomSyncProviderUri,
GrpcDialOptionsOverride: provider.providerConfiguration.GrpcDialOptionsOverride,
})

} else {
service = process.NewInProcessService(process.Configuration{
OfflineFlagSource: provider.providerConfiguration.OfflineFlagSourcePath,
})
}

if provider.providerConfiguration.Resolver == inProcess {
provider.hooks = append(provider.hooks, NewSyncContextHook(func() *of.EvaluationContext {
return provider.providerConfiguration.ContextEnricher(service.ContextValues())
}))
}
provider.service = service

return provider, nil
Expand Down Expand Up @@ -145,7 +152,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)
Expand Down
53 changes: 53 additions & 0 deletions providers/flagd/pkg/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ func TestNewProvider(t *testing.T) {
grpc.WithAuthority("test-authority"),
}

enrichment := make(map[string]any)
enrichment["test"] = "test"

expectedEnrichment := make(map[string]any)
expectedEnrichment["test"] = "test"
expectedEnrichment["test2"] = "test2"
tests := []struct {
name string
expectedResolver ResolverType
Expand All @@ -41,6 +47,7 @@ func TestNewProvider(t *testing.T) {
expectCustomSyncProviderUri string
expectOfflineFlagSourcePath string
expectGrpcDialOptionsOverride []grpc.DialOption
expectContextEnrichment map[string]any
options []ProviderOption
}{
{
Expand Down Expand Up @@ -269,6 +276,45 @@ func TestNewProvider(t *testing.T) {
WithOfflineFilePath("offlineFilePath"),
},
},
{
name: "with ContextEnricher inprocess resolver",
expectedResolver: inProcess,
expectHost: defaultHost,
expectPort: defaultInProcessPort,
expectCacheType: defaultCache,
expectCacheSize: defaultMaxCacheSize,
expectMaxRetries: defaultMaxEventStreamRetries,
expectContextEnrichment: enrichment,
options: []ProviderOption{
WithInProcessResolver(),
WithContextEnricher(func(m map[string]any) *of.EvaluationContext {
context := of.NewTargetlessEvaluationContext(m)
return &context
}),
},
},
{
name: "with ContextEnricher inprocess resolver adding values",
expectedResolver: inProcess,
expectHost: defaultHost,
expectPort: defaultInProcessPort,
expectCacheType: defaultCache,
expectCacheSize: defaultMaxCacheSize,
expectMaxRetries: defaultMaxEventStreamRetries,
expectContextEnrichment: expectedEnrichment,
options: []ProviderOption{
WithInProcessResolver(),
WithContextEnricher(func(m map[string]any) *of.EvaluationContext {
newMap := make(map[string]any, len(m)+1)
for k, v := range m {
newMap[k] = v
}
newMap["test2"] = "test2"
context := of.NewTargetlessEvaluationContext(newMap)
return &context
}),
},
},
}

for _, test := range tests {
Expand Down Expand Up @@ -369,6 +415,13 @@ func TestNewProvider(t *testing.T) {
}
}

if test.expectContextEnrichment != nil {
enriched := config.ContextEnricher(enrichment).Attributes()
if !reflect.DeepEqual(test.expectContextEnrichment, enriched) {
t.Errorf("incorrect context_enrichment attribute, expected %v, got %v", test.expectContextEnrichment, enriched)
}
}

// this line will fail linting if this provider is no longer compatible with the openfeature sdk
var _ of.FeatureProvider = flagdProvider
})
Expand Down
25 changes: 24 additions & 1 deletion providers/flagd/pkg/service/in_process/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package process
import (
"context"
"fmt"

"regexp"
parallel "sync"

Expand Down Expand Up @@ -33,6 +32,8 @@ type InProcess struct {
sync sync.ISync
syncEnd context.CancelFunc
wg parallel.WaitGroup
contextValues map[string]any
mtx parallel.RWMutex
}

type Configuration struct {
Expand Down Expand Up @@ -113,6 +114,12 @@ 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.mtx.Lock()
Copy link

Choose a reason for hiding this comment

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

Is it possible to add a defer i.mtx.Unlock() here?

Copy link
Member Author

Choose a reason for hiding this comment

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

where do you see the benefit?

Copy link

Choose a reason for hiding this comment

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

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

i.contextValues = data.SyncContext.AsMap()
Copy link
Member

Choose a reason for hiding this comment

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

In java we have an optional lambda to manipulate this... I think it should be simple to add it here.

Copy link
Member

Choose a reason for hiding this comment

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

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.

i.mtx.Unlock()
}

if err != nil {
i.events <- of.Event{
ProviderName: "flagd", EventType: of.ProviderError,
Expand Down Expand Up @@ -294,6 +301,22 @@ func (i *InProcess) appendMetadata(evalMetadata model.Metadata) {
}
}

func (i *InProcess) ContextValues() map[string]any {
i.mtx.RLock()
defer i.mtx.RUnlock()

if i.contextValues == nil {
return nil
}

// Return a copy to prevent mutation of internal state
contextValuesCopy := make(map[string]any, len(i.contextValues))
for k, v := range i.contextValues {
contextValuesCopy[k] = v
}
return contextValuesCopy
}

// makeSyncProvider is a helper to create sync.ISync and return the underlying uri used by it to the caller
func makeSyncProvider(cfg Configuration, log *logger.Logger) (sync.ISync, string) {
if cfg.CustomSyncProvider != nil {
Expand Down
17 changes: 15 additions & 2 deletions providers/flagd/pkg/service/in_process/service_grpc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"fmt"
"github.com/open-feature/go-sdk/openfeature"
"google.golang.org/grpc"
"google.golang.org/protobuf/types/known/structpb"
"log"
"net"
"testing"
Expand Down Expand Up @@ -38,11 +39,20 @@
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,
Expand Down Expand Up @@ -112,6 +122,10 @@
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
Expand All @@ -138,7 +152,7 @@
}

inProcessService := NewInProcessService(Configuration{
TargetUri: "envoy://localhost:9211/foo.service",
TargetUri: "envoy://localhost:9211/foo.service",
Selector: scope,
TLSEnabled: false,
})
Expand Down Expand Up @@ -201,7 +215,6 @@
}
}


// bufferedServer - a mock grpc service backed by buffered connection
type bufferedServer struct {
listener net.Listener
Expand Down Expand Up @@ -229,8 +242,8 @@
return b.fetchAllFlagsResponse, b.fetchAllFlagsError
}

func (b *bufferedServer) GetMetadata(_ context.Context, _ *v1.GetMetadataRequest) (*v1.GetMetadataResponse, error) {

Check failure on line 245 in providers/flagd/pkg/service/in_process/service_grpc_test.go

View workflow job for this annotation

GitHub Actions / lint

SA1019: v1.GetMetadataRequest is deprecated: Marked as deprecated in flagd/sync/v1/sync.proto. (staticcheck)
return &v1.GetMetadataResponse{}, nil

Check failure on line 246 in providers/flagd/pkg/service/in_process/service_grpc_test.go

View workflow job for this annotation

GitHub Actions / lint

SA1019: v1.GetMetadataResponse is deprecated: Marked as deprecated in flagd/sync/v1/sync.proto. (staticcheck)
}

// serve serves a bufferedServer. This is a blocking call
Expand Down
4 changes: 4 additions & 0 deletions providers/flagd/pkg/service/rpc/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,10 @@ func handleError(err error) openfeature.ResolutionError {
return openfeature.NewGeneralResolutionError(err.Error())
}

func (s *Service) ContextValues() map[string]any {
return nil
}

func (s *Service) EventChannel() <-chan of.Event {
return s.events
}
Expand Down
21 changes: 21 additions & 0 deletions providers/flagd/pkg/sync_context_hook.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package flagd

import (
"context"
"github.com/open-feature/go-sdk/openfeature"
)

type ContextEnricher func(map[string]any) *openfeature.EvaluationContext

type SyncContextHook struct {
openfeature.UnimplementedHook
contextEnricher func() *openfeature.EvaluationContext
}

func NewSyncContextHook(contextEnricher func() *openfeature.EvaluationContext) SyncContextHook {
return SyncContextHook{contextEnricher: contextEnricher}
}

func (hook SyncContextHook) Before(ctx context.Context, hookContext openfeature.HookContext, hookHints openfeature.HookHints) (*openfeature.EvaluationContext, error) {
return hook.contextEnricher(), nil
}
2 changes: 1 addition & 1 deletion tests/flagd/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ toolchain go1.24.4

require (
github.com/cucumber/godog v0.15.1
github.com/docker/go-connections v0.5.0
github.com/open-feature/go-sdk v1.11.0
github.com/open-feature/go-sdk-contrib/providers/flagd v0.3.0
github.com/testcontainers/testcontainers-go v0.38.0
Expand Down Expand Up @@ -70,7 +71,6 @@ require (
github.com/docker/docker v28.2.2+incompatible // indirect
github.com/docker/docker-credential-helpers v0.8.2 // indirect
github.com/docker/go v1.5.1-1.0.20160303222718-d30aec9fd63c // indirect
github.com/docker/go-connections v0.5.0 // indirect
github.com/docker/go-metrics v0.0.1 // indirect
github.com/docker/go-units v0.5.0 // indirect
github.com/ebitengine/purego v0.8.4 // indirect
Expand Down
Loading