Skip to content

Commit 4115185

Browse files
suthar26suthar26
committed
feat: return disabled reason as a successful evaluation
Signed-off-by: Parth Suthar <parth.suthar@dynatrace.com>
1 parent 5042473 commit 4115185

11 files changed

Lines changed: 59 additions & 52 deletions

File tree

‎core/pkg/evaluator/json.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -175,11 +175,6 @@ func (je *Resolver) ResolveAllValues(ctx context.Context, reqID string, context
175175
var metadata map[string]interface{}
176176

177177
for _, flag := range allFlags {
178-
if flag.State == Disabled {
179-
// ignore evaluation of disabled flag
180-
continue
181-
}
182-
183178
defaultValue := flag.Variants[flag.DefaultVariant]
184179
switch defaultValue.(type) {
185180
case bool:
@@ -313,6 +308,11 @@ func resolve[T constraints](ctx context.Context, reqID string, key string, conte
313308
return zero, variant, model.FallbackReason, metadata, nil
314309
}
315310

311+
if reason == model.DisabledReason {
312+
var zero T
313+
return zero, "", model.DisabledReason, metadata, nil
314+
}
315+
316316
var ok bool
317317
value, ok = variants[variant].(T)
318318
if !ok {
@@ -348,7 +348,7 @@ func (je *Resolver) evaluateVariant(ctx context.Context, reqID string, flagKey s
348348

349349
if flag.State == Disabled {
350350
je.Logger.DebugWithID(reqID, fmt.Sprintf("requested flag is disabled: %s", flagKey))
351-
return "", flag.Variants, model.ErrorReason, metadata, errors.New(model.FlagDisabledErrorCode)
351+
return "", nil, model.DisabledReason, metadata, nil
352352
}
353353

354354
// get the targeting logic, if any

‎core/pkg/evaluator/json_test.go‎

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -467,9 +467,11 @@ func TestResolveAllValues(t *testing.T) {
467467
}
468468

469469
for _, val := range vals {
470-
// disabled flag must be ignored from bulk evaluation
470+
// disabled flag must not be ignored from bulk evaluation
471471
if val.FlagKey == DisabledFlag {
472-
t.Errorf("disabled flag '%s' is present in evaluation results", DisabledFlag)
472+
assert.Equal(t, model.DisabledReason, val.Reason)
473+
assert.Nil(t, val.Error)
474+
continue
473475
}
474476

475477
switch vT := val.Value.(type) {
@@ -510,7 +512,7 @@ func TestResolveBooleanValue(t *testing.T) {
510512
{DynamicBoolFlag, map[string]interface{}{ColorProp: ColorValue}, StaticBoolValue, model.TargetingMatchReason, ""},
511513
{StaticObjectFlag, nil, StaticBoolValue, model.ErrorReason, model.TypeMismatchErrorCode},
512514
{MissingFlag, nil, StaticBoolValue, model.ErrorReason, model.FlagNotFoundErrorCode},
513-
{DisabledFlag, nil, StaticBoolValue, model.ErrorReason, model.FlagDisabledErrorCode},
515+
{DisabledFlag, nil, false, model.DisabledReason, ""},
514516
}
515517
const reqID = "default"
516518
evaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
@@ -545,7 +547,7 @@ func BenchmarkResolveBooleanValue(b *testing.B) {
545547
{DynamicBoolFlag, map[string]interface{}{ColorProp: ColorValue}, StaticBoolValue, model.TargetingMatchReason, ""},
546548
{StaticObjectFlag, nil, StaticBoolValue, model.ErrorReason, model.TypeMismatchErrorCode},
547549
{MissingFlag, nil, StaticBoolValue, model.ErrorReason, model.FlagNotFoundErrorCode},
548-
{DisabledFlag, nil, StaticBoolValue, model.ErrorReason, model.FlagDisabledErrorCode},
550+
{DisabledFlag, nil, false, model.DisabledReason, ""},
549551
}
550552

551553
evaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
@@ -585,7 +587,7 @@ func TestResolveStringValue(t *testing.T) {
585587
{DynamicStringFlag, map[string]interface{}{ColorProp: ColorValue}, DynamicStringValue, model.TargetingMatchReason, ""},
586588
{StaticObjectFlag, nil, "", model.ErrorReason, model.TypeMismatchErrorCode},
587589
{MissingFlag, nil, "", model.ErrorReason, model.FlagNotFoundErrorCode},
588-
{DisabledFlag, nil, "", model.ErrorReason, model.FlagDisabledErrorCode},
590+
{DisabledFlag, nil, "", model.DisabledReason, ""},
589591
}
590592
const reqID = "default"
591593
evaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
@@ -621,7 +623,7 @@ func BenchmarkResolveStringValue(b *testing.B) {
621623
{DynamicStringFlag, map[string]interface{}{ColorProp: ColorValue}, DynamicStringValue, model.TargetingMatchReason, ""},
622624
{StaticObjectFlag, nil, "", model.ErrorReason, model.TypeMismatchErrorCode},
623625
{MissingFlag, nil, "", model.ErrorReason, model.FlagNotFoundErrorCode},
624-
{DisabledFlag, nil, "", model.ErrorReason, model.FlagDisabledErrorCode},
626+
{DisabledFlag, nil, "", model.DisabledReason, ""},
625627
}
626628

627629
evaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
@@ -661,7 +663,7 @@ func TestResolveFloatValue(t *testing.T) {
661663
{DynamicFloatFlag, map[string]interface{}{ColorProp: ColorValue}, DynamicFloatValue, model.TargetingMatchReason, ""},
662664
{StaticObjectFlag, nil, 13, model.ErrorReason, model.TypeMismatchErrorCode},
663665
{MissingFlag, nil, 13, model.ErrorReason, model.FlagNotFoundErrorCode},
664-
{DisabledFlag, nil, 0, model.ErrorReason, model.FlagDisabledErrorCode},
666+
{DisabledFlag, nil, 0, model.DisabledReason, ""},
665667
}
666668
const reqID = "default"
667669
evaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
@@ -697,7 +699,7 @@ func BenchmarkResolveFloatValue(b *testing.B) {
697699
{DynamicFloatFlag, map[string]interface{}{ColorProp: ColorValue}, DynamicFloatValue, model.TargetingMatchReason, ""},
698700
{StaticObjectFlag, nil, 13, model.ErrorReason, model.TypeMismatchErrorCode},
699701
{MissingFlag, nil, 13, model.ErrorReason, model.FlagNotFoundErrorCode},
700-
{DisabledFlag, nil, 0, model.ErrorReason, model.FlagDisabledErrorCode},
702+
{DisabledFlag, nil, 0, model.DisabledReason, ""},
701703
}
702704

703705
evaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
@@ -737,7 +739,7 @@ func TestResolveIntValue(t *testing.T) {
737739
{DynamicIntFlag, map[string]interface{}{ColorProp: ColorValue}, DynamicIntValue, model.TargetingMatchReason, ""},
738740
{StaticObjectFlag, nil, 13, model.ErrorReason, model.TypeMismatchErrorCode},
739741
{MissingFlag, nil, 13, model.ErrorReason, model.FlagNotFoundErrorCode},
740-
{DisabledFlag, nil, 0, model.ErrorReason, model.FlagDisabledErrorCode},
742+
{DisabledFlag, nil, 0, model.DisabledReason, ""},
741743
}
742744
const reqID = "default"
743745
evaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
@@ -773,7 +775,7 @@ func BenchmarkResolveIntValue(b *testing.B) {
773775
{DynamicIntFlag, map[string]interface{}{ColorProp: ColorValue}, DynamicIntValue, model.TargetingMatchReason, ""},
774776
{StaticObjectFlag, nil, 13, model.ErrorReason, model.TypeMismatchErrorCode},
775777
{MissingFlag, nil, 13, model.ErrorReason, model.FlagNotFoundErrorCode},
776-
{DisabledFlag, nil, 0, model.ErrorReason, model.FlagDisabledErrorCode},
778+
{DisabledFlag, nil, 0, model.DisabledReason, ""},
777779
}
778780

779781
evaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
@@ -813,7 +815,7 @@ func TestResolveObjectValue(t *testing.T) {
813815
{DynamicObjectFlag, map[string]interface{}{ColorProp: ColorValue}, DynamicObjectValue, model.TargetingMatchReason, ""},
814816
{StaticBoolFlag, nil, "{}", model.ErrorReason, model.TypeMismatchErrorCode},
815817
{MissingFlag, nil, "{}", model.ErrorReason, model.FlagNotFoundErrorCode},
816-
{DisabledFlag, nil, "{}", model.ErrorReason, model.FlagDisabledErrorCode},
818+
{DisabledFlag, nil, "", model.DisabledReason, ""},
817819
}
818820
const reqID = "default"
819821
evaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
@@ -827,11 +829,15 @@ func TestResolveObjectValue(t *testing.T) {
827829

828830
if test.errorCode == "" {
829831
if assert.NoError(t, err) {
830-
marshalled, err := json.Marshal(val)
831-
if assert.NoError(t, err) {
832-
assert.JSONEq(t, test.val, string(marshalled))
833-
assert.Equal(t, test.reason, reason)
832+
if test.flagKey == DisabledFlag {
833+
assert.Nil(t, val)
834+
} else {
835+
marshalled, err := json.Marshal(val)
836+
if assert.NoError(t, err) {
837+
assert.JSONEq(t, test.val, string(marshalled))
838+
}
834839
}
840+
assert.Equal(t, test.reason, reason)
835841
}
836842
} else {
837843
assert.Equal(t, model.ErrorReason, reason)
@@ -852,7 +858,7 @@ func BenchmarkResolveObjectValue(b *testing.B) {
852858
{DynamicObjectFlag, map[string]interface{}{ColorProp: ColorValue}, DynamicObjectValue, model.TargetingMatchReason, ""},
853859
{StaticBoolFlag, nil, "{}", model.ErrorReason, model.TypeMismatchErrorCode},
854860
{MissingFlag, nil, "{}", model.ErrorReason, model.FlagNotFoundErrorCode},
855-
{DisabledFlag, nil, "{}", model.ErrorReason, model.FlagDisabledErrorCode},
861+
{DisabledFlag, nil, "{}", model.DisabledReason, ""},
856862
}
857863

858864
evaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())
@@ -897,7 +903,7 @@ func TestResolveAsAnyValue(t *testing.T) {
897903
{DynamicObjectFlag, map[string]interface{}{ColorProp: ColorValue}, DynamicObjectValue, model.TargetingMatchReason, ""},
898904
// errors
899905
{MissingFlag, nil, "{}", model.ErrorReason, model.FlagNotFoundErrorCode},
900-
{DisabledFlag, nil, "{}", model.ErrorReason, model.FlagDisabledErrorCode},
906+
{DisabledFlag, nil, "{}", model.DisabledReason, ""},
901907
}
902908

903909
evaluator := flagdEvaluator.NewJSON(logger.NewLogger(nil, false), store.NewFlags())

‎core/pkg/model/error.go‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ const (
77
ParseErrorCode = "PARSE_ERROR"
88
TypeMismatchErrorCode = "TYPE_MISMATCH"
99
GeneralErrorCode = "GENERAL"
10-
FlagDisabledErrorCode = "FLAG_DISABLED"
1110
InvalidContextCode = "INVALID_CONTEXT"
1211
)
1312

@@ -16,7 +15,6 @@ var ReadableErrorMessage = map[string]string{
1615
ParseErrorCode: "Error parsing input or configuration",
1716
TypeMismatchErrorCode: "Type mismatch error",
1817
GeneralErrorCode: "General error",
19-
FlagDisabledErrorCode: "Flag is disabled",
2018
InvalidContextCode: "Invalid context provided",
2119
}
2220

‎core/pkg/service/ofrep/models.go‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,13 @@ func SuccessResponseFrom(result evaluator.AnyValue) EvaluationSuccess {
7070
Metadata: result.Metadata,
7171
}
7272
}
73+
if result.Reason == model.DisabledReason {
74+
return EvaluationSuccess{
75+
Key: result.FlagKey,
76+
Reason: model.DisabledReason,
77+
Metadata: result.Metadata,
78+
}
79+
}
7380
return EvaluationSuccess{
7481
Value: result.Value,
7582
Key: result.FlagKey,
@@ -114,10 +121,6 @@ func EvaluationErrorResponseFrom(result evaluator.AnyValue) (int, EvaluationErro
114121
status = 404
115122
payload.ErrorCode = model.FlagNotFoundErrorCode
116123
payload.ErrorDetails = fmt.Sprintf("flag `%s` does not exist", result.FlagKey)
117-
case model.FlagDisabledErrorCode:
118-
status = 404
119-
payload.ErrorCode = model.FlagNotFoundErrorCode
120-
payload.ErrorDetails = fmt.Sprintf("flag `%s` is disabled", result.FlagKey)
121124
case model.ParseErrorCode:
122125
payload.ErrorCode = model.ParseErrorCode
123126
payload.ErrorDetails = fmt.Sprintf("error parsing the flag `%s`", result.FlagKey)

‎core/pkg/service/ofrep/models_test.go‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -327,12 +327,6 @@ func TestErrorStatus(t *testing.T) {
327327
expectedStatus: 400,
328328
expectedCode: model.ParseErrorCode,
329329
},
330-
{
331-
name: "flag disabled",
332-
modelError: model.FlagDisabledErrorCode,
333-
expectedStatus: 404,
334-
expectedCode: model.FlagNotFoundErrorCode,
335-
},
336330
{
337331
name: "general error",
338332
modelError: model.GeneralErrorCode,

‎flagd/pkg/service/flag-evaluation/flag_evaluator.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,13 @@ func (s *OldFlagEvaluationService) ResolveAll(
9090
// register the impression and reason for each flag evaluated
9191
s.metrics.RecordEvaluation(ctx, value.Error, value.Reason, value.Variant, value.FlagKey)
9292

93+
if value.Reason == model.DisabledReason {
94+
res.Flags[value.FlagKey] = &schemaV1.AnyFlag{
95+
Reason: value.Reason,
96+
}
97+
continue
98+
}
99+
93100
switch v := value.Value.(type) {
94101
case bool:
95102
res.Flags[value.FlagKey] = &schemaV1.AnyFlag{
@@ -395,7 +402,7 @@ func formatContextKeys(context map[string]any) []string {
395402
func errFormat(err error) error {
396403
ReadableErrorMsg := model.GetErrorMessage(err.Error())
397404
switch err.Error() {
398-
case model.FlagNotFoundErrorCode, model.FlagDisabledErrorCode:
405+
case model.FlagNotFoundErrorCode:
399406
return connect.NewError(connect.CodeNotFound, fmt.Errorf("%s", ReadableErrorMsg))
400407
case model.TypeMismatchErrorCode:
401408
return connect.NewError(connect.CodeInvalidArgument, fmt.Errorf("%s", ReadableErrorMsg))

‎flagd/pkg/service/flag-evaluation/flag_evaluator_test.go‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -974,10 +974,6 @@ func TestFlag_Evaluation_ErrorCodes(t *testing.T) {
974974
err: errors.New(model.ParseErrorCode),
975975
code: connect.CodeDataLoss,
976976
},
977-
{
978-
err: errors.New(model.FlagDisabledErrorCode),
979-
code: connect.CodeNotFound,
980-
},
981977
{
982978
err: errors.New(model.GeneralErrorCode),
983979
code: connect.CodeUnknown,
@@ -1027,11 +1023,6 @@ func Test_Readable_ErrorMessage(t *testing.T) {
10271023
code: model.GeneralErrorCode,
10281024
want: model.ReadableErrorMessage[model.GeneralErrorCode],
10291025
},
1030-
{
1031-
name: "Testing flag disabled error",
1032-
code: model.FlagDisabledErrorCode,
1033-
want: model.ReadableErrorMessage[model.FlagDisabledErrorCode],
1034-
},
10351026
{
10361027
name: "Testing invalid context error",
10371028
code: model.InvalidContextCode,

‎flagd/pkg/service/flag-evaluation/flag_evaluator_v1.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"connectrpc.com/connect"
1111
"github.com/open-feature/flagd/core/pkg/evaluator"
1212
"github.com/open-feature/flagd/core/pkg/logger"
13+
"github.com/open-feature/flagd/core/pkg/model"
1314
"github.com/open-feature/flagd/core/pkg/service"
1415
"github.com/open-feature/flagd/core/pkg/store"
1516
"github.com/open-feature/flagd/core/pkg/telemetry"
@@ -91,6 +92,12 @@ func (s *FlagEvaluationService) ResolveAll(
9192
for _, resolved := range resolutions {
9293
// register the impression and reason for each flag evaluated
9394
s.metrics.RecordEvaluation(ctx, resolved.Error, resolved.Reason, resolved.Variant, resolved.FlagKey)
95+
if resolved.Reason == model.DisabledReason {
96+
res.Flags[resolved.FlagKey] = &evalV1.AnyFlag{
97+
Reason: resolved.Reason,
98+
}
99+
continue
100+
}
94101
switch v := resolved.Value.(type) {
95102
case bool:
96103
res.Flags[resolved.FlagKey] = &evalV1.AnyFlag{

‎flagd/pkg/service/flag-evaluation/flag_evaluator_v1_test.go‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -971,10 +971,6 @@ func TestFlag_EvaluationV2_ErrorCodes(t *testing.T) {
971971
err: errors.New(model.ParseErrorCode),
972972
code: connect.CodeDataLoss,
973973
},
974-
{
975-
err: errors.New(model.FlagDisabledErrorCode),
976-
code: connect.CodeNotFound,
977-
},
978974
{
979975
err: errors.New(model.GeneralErrorCode),
980976
code: connect.CodeUnknown,

‎flagd/pkg/service/flag-evaluation/flag_evaluator_v2.go‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,13 @@ func resolveV2[T constraints](ctx context.Context, logger *logger.Logger, resolv
251251
return fmt.Errorf("error setting response result: %w", err)
252252
}
253253
}
254+
} else if reason == model.DisabledReason {
255+
if respV2, ok := resp.(responseV2[T]); ok {
256+
if err := respV2.SetReasonOnly(model.DisabledReason, metadata); err != nil {
257+
logger.ErrorWithID(reqID, err.Error())
258+
return fmt.Errorf("error setting response result: %w", err)
259+
}
260+
}
254261
} else {
255262
if err := resp.SetResult(result, variant, reason, metadata); err != nil && evalErr == nil {
256263
logger.ErrorWithID(reqID, err.Error())
@@ -286,8 +293,6 @@ func recordResolveErrorV2(span trace.Span, err error, flagKey string) {
286293
func errFormatV2(err error) error {
287294
ReadableErrorMsg := model.GetErrorMessage(err.Error())
288295
switch err.Error() {
289-
case model.FlagDisabledErrorCode:
290-
return connect.NewError(connect.CodeNotFound, fmt.Errorf("%s", ReadableErrorMsg))
291296
case model.TypeMismatchErrorCode:
292297
return connect.NewError(connect.CodeInvalidArgument, fmt.Errorf("%s", ReadableErrorMsg))
293298
case model.GeneralErrorCode:

0 commit comments

Comments
 (0)