Skip to content

feat: add evaluation details to finally hook #328

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Mar 5, 2025
Merged

feat: add evaluation details to finally hook #328

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2f0edc0
feat: add evaluation details to finally hook
bbland1 Feb 26, 2025
87d3541
fixed spacing issue openfeature/hooks/logging_hook.go
bbland1 Feb 27, 2025
dca32b5
Merge branch 'main' into update-finally-hook
toddbaert Feb 27, 2025
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
6 changes: 3 additions & 3 deletions openfeature/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,7 @@ func (c *Client) evaluate(
}

defer func() {
c.finallyHooks(ctx, hookCtx, providerInvocationClientApiHooks, options)
c.finallyHooks(ctx, hookCtx, providerInvocationClientApiHooks, evalDetails, options)
}()

// bypass short-circuit logic for the Noop provider; it is essentially stateless and a "special case"
Expand Down Expand Up @@ -828,9 +828,9 @@ func (c *Client) errorHooks(ctx context.Context, hookCtx HookContext, hooks []Ho
}
}

func (c *Client) finallyHooks(ctx context.Context, hookCtx HookContext, hooks []Hook, options EvaluationOptions) {
func (c *Client) finallyHooks(ctx context.Context, hookCtx HookContext, hooks []Hook, evalDetails InterfaceEvaluationDetails, options EvaluationOptions) {
for _, hook := range hooks {
hook.Finally(ctx, hookCtx, options.hookHints)
hook.Finally(ctx, hookCtx,evalDetails, options.hookHints)
}
}

Expand Down
4 changes: 2 additions & 2 deletions openfeature/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1314,7 +1314,7 @@ func TestRequirement_1_7_6(t *testing.T) {
ctrl := gomock.NewController(t)
mockHook := NewMockHook(ctrl)
mockHook.EXPECT().Error(gomock.Any(), gomock.Any(), ProviderNotReadyError, gomock.Any())
mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any())
mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())

notReadyEventingProvider := struct {
FeatureProvider
Expand Down Expand Up @@ -1377,7 +1377,7 @@ func TestRequirement_1_7_7(t *testing.T) {
ctrl := gomock.NewController(t)
mockHook := NewMockHook(ctrl)
mockHook.EXPECT().Error(gomock.Any(), gomock.Any(), ProviderFatalError, gomock.Any())
mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any())
mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())

client := GetApiInstance().GetNamedClient(t.Name())
client.AddHooks(mockHook)
Expand Down
4 changes: 2 additions & 2 deletions openfeature/hooks.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ type Hook interface {
Before(ctx context.Context, hookContext HookContext, hookHints HookHints) (*EvaluationContext, error)
After(ctx context.Context, hookContext HookContext, flagEvaluationDetails InterfaceEvaluationDetails, hookHints HookHints) error
Error(ctx context.Context, hookContext HookContext, err error, hookHints HookHints)
Finally(ctx context.Context, hookContext HookContext, hookHints HookHints)
Finally(ctx context.Context, hookContext HookContext, flagEvaluationDetails InterfaceEvaluationDetails, hookHints HookHints)
}

// HookHints contains a map of hints for hooks
Expand Down Expand Up @@ -108,4 +108,4 @@ func (UnimplementedHook) After(context.Context, HookContext, InterfaceEvaluation
return nil
}
func (UnimplementedHook) Error(context.Context, HookContext, error, HookHints) {}
func (UnimplementedHook) Finally(context.Context, HookContext, HookHints) {}
func (UnimplementedHook) Finally(context.Context, HookContext, InterfaceEvaluationDetails, HookHints) {}
2 changes: 1 addition & 1 deletion openfeature/hooks/logging_hook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,6 @@ func (h *LoggingHook) Error(ctx context.Context, hookContext of.HookContext, err
h.logger.Error("Error stage", args...)
}

func (h *LoggingHook) Finally(ctx context.Context, hCtx of.HookContext, hint of.HookHints) {
func (h *LoggingHook) Finally(ctx context.Context, hCtx of.HookContext, flagEvaluationDetails of.InterfaceEvaluationDetails, hint of.HookHints) {

}
4 changes: 2 additions & 2 deletions openfeature/hooks_mock_test.go
View file
Open in desktop

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

52 changes: 26 additions & 26 deletions openfeature/hooks_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ func TestRequirement_4_3_2(t *testing.T) {
mockProvider.EXPECT().StringEvaluation(gomock.Any(), flagKey, defaultValue, flatCtx).
After(mockHook.EXPECT().Before(gomock.Any(), gomock.Any(), gomock.Any()))
mockHook.EXPECT().After(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any())
mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())

_, err = GetApiInstance().GetNamedClient(t.Name()).StringValueDetails(context.Background(), flagKey, defaultValue, evalCtx, WithHooks(mockHook))
if err != nil {
Expand Down Expand Up @@ -244,9 +244,9 @@ func TestRequirement_4_3_3(t *testing.T) {
mockHook2.EXPECT().Before(gomock.Any(), hook2Ctx, gomock.Any())

mockHook1.EXPECT().After(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
mockHook1.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any())
mockHook1.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
mockHook2.EXPECT().After(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
mockHook2.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any())
mockHook2.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())

_, err = client.StringValueDetails(context.Background(), flagKey, defaultValue, evalCtx, WithHooks(mockHook1, mockHook2))
if err != nil {
Expand Down Expand Up @@ -321,7 +321,7 @@ func TestRequirement_4_3_4(t *testing.T) {
}
mockProvider.EXPECT().StringEvaluation(gomock.Any(), flagKey, defaultValue, flattenContext(expectedMergedContext))
mockHook.EXPECT().After(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any())
mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())

_, err = client.StringValueDetails(context.Background(), flagKey, defaultValue, invEvalCtx, WithHooks(mockHook))
if err != nil {
Expand Down Expand Up @@ -357,7 +357,7 @@ func TestRequirement_4_3_5(t *testing.T) {
// assert that the After hooks are executed after the flag evaluation
mockHook.EXPECT().After(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
After(mockProvider.EXPECT().StringEvaluation(gomock.Any(), flagKey, defaultValue, flatCtx))
mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any())
mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())

_, err = GetApiInstance().GetNamedClient(t.Name()).StringValueDetails(context.Background(), flagKey, defaultValue, evalCtx, WithHooks(mockHook))
if err != nil {
Expand Down Expand Up @@ -407,7 +407,7 @@ func TestRequirement_4_3_6(t *testing.T) {
// assert that the Error hooks are executed after the failed Before hooks
mockHook.EXPECT().Error(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
After(mockHook.EXPECT().Before(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, errors.New("forced")))
mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any())
mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())

_, err = GetApiInstance().GetNamedClient(t.Name()).StringValueDetails(context.Background(), flagKey, defaultValue, evalCtx, WithHooks(mockHook))
if err == nil {
Expand Down Expand Up @@ -440,7 +440,7 @@ func TestRequirement_4_3_6(t *testing.T) {
},
}),
)
mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any())
mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())

_, err = GetApiInstance().GetNamedClient(t.Name()).StringValueDetails(context.Background(), flagKey, defaultValue, evalCtx, WithHooks(mockHook))
if err == nil {
Expand All @@ -467,7 +467,7 @@ func TestRequirement_4_3_6(t *testing.T) {
// assert that the Error hooks are executed after the failed After hooks
mockHook.EXPECT().Error(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
After(mockHook.EXPECT().After(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(errors.New("forced")))
mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any())
mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())

_, err = GetApiInstance().GetNamedClient(t.Name()).StringValueDetails(context.Background(), flagKey, defaultValue, evalCtx, WithHooks(mockHook))
if err == nil {
Expand Down Expand Up @@ -514,7 +514,7 @@ func TestRequirement_4_3_7(t *testing.T) {
mockProvider.EXPECT().Hooks().AnyTimes()

// assert that the Finally hook runs after the Before & After stages
mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any()).
mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
After(mockHook.EXPECT().After(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())).
After(mockHook.EXPECT().Before(gomock.Any(), gomock.Any(), gomock.Any()))
mockProvider.EXPECT().StringEvaluation(context.Background(), flagKey, defaultValue, flatCtx)
Expand All @@ -541,7 +541,7 @@ func TestRequirement_4_3_7(t *testing.T) {

mockHook.EXPECT().Before(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, errors.New("forced"))
// assert that the Finally hook runs after the Error stage
mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any()).
mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
After(mockHook.EXPECT().Error(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()))

_, err = GetApiInstance().GetNamedClient(t.Name()).StringValueDetails(context.Background(), flagKey, defaultValue, evalCtx, WithHooks(mockHook))
Expand All @@ -554,7 +554,7 @@ func TestRequirement_4_3_7(t *testing.T) {
mockHook := NewMockHook(ctrl)

type requirement interface {
Finally(ctx context.Context, hookContext HookContext, hookHints HookHints)
Finally(ctx context.Context, hookContext HookContext, flagEvaluationDetails InterfaceEvaluationDetails, hookHints HookHints)
}

var hookI interface{} = mockHook
Expand Down Expand Up @@ -673,10 +673,10 @@ func TestRequirement_4_4_2(t *testing.T) {
After(mockProviderHook.EXPECT().After(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()))

// finally: Invocation, Client, API
mockAPIHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any()).
After(mockClientHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any())).
After(mockInvocationHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any())).
After(mockProviderHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any()))
mockAPIHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
After(mockClientHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())).
After(mockInvocationHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())).
After(mockProviderHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()))

mockProvider.EXPECT().StringEvaluation(context.Background(), flagKey, defaultValue, flatCtx)

Expand Down Expand Up @@ -717,10 +717,10 @@ func TestRequirement_4_4_2(t *testing.T) {
After(mockInvocationHook.EXPECT().Error(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())).
After(mockProviderHook.EXPECT().Error(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()))

mockProviderHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any())
mockInvocationHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any())
mockClientHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any())
mockAPIHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any())
mockProviderHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
mockInvocationHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
mockClientHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
mockAPIHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())

_, err = client.StringValueDetails(context.Background(), flagKey, defaultValue, evalCtx, WithHooks(mockInvocationHook))
if err == nil {
Expand Down Expand Up @@ -779,9 +779,9 @@ func TestRequirement_4_4_6(t *testing.T) {
mockHook1.EXPECT().Before(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, errors.New("forced"))
// the lack of mockHook2.EXPECT().Before() asserts that remaining hooks aren't invoked after an error
mockHook1.EXPECT().Error(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
mockHook1.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any())
mockHook1.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
mockHook2.EXPECT().Error(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
mockHook2.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any())
mockHook2.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())

_, err = client.StringValueDetails(context.Background(), flagKey, defaultValue, evalCtx, WithHooks(mockHook1, mockHook2))
if err == nil {
Expand Down Expand Up @@ -814,9 +814,9 @@ func TestRequirement_4_4_6(t *testing.T) {
mockHook1.EXPECT().After(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(errors.New("forced"))
// the lack of mockHook2.EXPECT().After() asserts that remaining hooks aren't invoked after an error
mockHook1.EXPECT().Error(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
mockHook1.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any())
mockHook1.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
mockHook2.EXPECT().Error(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
mockHook2.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any())
mockHook2.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())

mockProvider.EXPECT().StringEvaluation(context.Background(), flagKey, defaultValue, flatCtx)

Expand Down Expand Up @@ -850,7 +850,7 @@ func TestRequirement_4_4_7(t *testing.T) {

mockHook.EXPECT().Before(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, errors.New("forced"))
mockHook.EXPECT().Error(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())
mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any())
mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())

res, err := GetApiInstance().GetNamedClient(t.Name()).StringValueDetails(context.Background(), flagKey, defaultValue, evalCtx, WithHooks(mockHook))
if err == nil {
Expand Down Expand Up @@ -900,7 +900,7 @@ func TestRequirement_4_5_2(t *testing.T) {

mockHook.EXPECT().Before(gomock.Any(), gomock.Any(), hookHints)
mockHook.EXPECT().After(gomock.Any(), gomock.Any(), gomock.Any(), hookHints)
mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), hookHints)
mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), hookHints)

mockProvider.EXPECT().StringEvaluation(context.Background(), flagKey, defaultValue, flatCtx)

Expand Down Expand Up @@ -933,7 +933,7 @@ func TestRequirement_4_5_2(t *testing.T) {

mockHook.EXPECT().Before(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, errors.New("forced"))
mockHook.EXPECT().Error(gomock.Any(), gomock.Any(), gomock.Any(), hookHints)
mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any())
mockHook.EXPECT().Finally(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any())

_, err = client.StringValueDetails(context.Background(), flagKey, defaultValue, evalCtx, WithHooks(mockHook), WithHookHints(hookHints))
if err == nil {
Expand Down
Loading