Skip to content

Commit

Permalink
feat: introduce metrics for failed evaluations (#584)
Browse files Browse the repository at this point in the history
<!-- Please use this template for your pull request. -->
<!-- Please use the sections that you need and delete other sections -->

## This PR
<!-- add the description of the PR here -->

- introduced a new metric which buckets evaluations by reason
- small refactoring
- unit tests

### Related Issues
<!-- add here the GitHub issue that this PR resolves if applicable -->

Fixes #559 

### How to test

Run flagd via 
```
make run
```
Run
```
curl -X POST "localhost:8013/schema.v1.Service/ResolveString" -d '{"flagKey":"fibAlgo","context":{}}' -H "Content-Type: application/json"
curl -X POST "localhost:8013/schema.v1.Service/ResolveString" -d '{"flagKey":"myStringFlag","context":{}}' -H "Content-Type: application/json"
curl -X POST "localhost:8013/schema.v1.Service/ResolveString" -d '{"flagKey":"myStringFlagNonExisting","context":{}}' -H "Content-Type: application/json"
curl -X POST "localhost:8013/schema.v1.Service/ResolveString" -d '{"flagKey":"myStringFlagNonExisting2","context":{}}' -H "Content-Type: application/json"
curl -X POST "localhost:8013/schema.v1.Service/ResolveInt" -d '{"flagKey":"myStringFlag","context":{}}' -H "Content-Type: application/json"
```

Open http://localhost:8014/metrics and see something like the following

```
# HELP impressions_total The number of evaluation for a given flag
# TYPE impressions_total counter
impressions_total{feature_flag_key="fibAlgo",feature_flag_provider_name="flagd",feature_flag_reason="DEFAULT",feature_flag_variant="recursive",otel_scope_name="openfeature/flagd",otel_scope_version=""} 1
impressions_total{feature_flag_key="myStringFlag",feature_flag_provider_name="flagd",feature_flag_reason="STATIC",feature_flag_variant="key1",otel_scope_name="openfeature/flagd",otel_scope_version=""} 1
```
and

```
# HELP reasons_total The number of evaluations for a given reason
# TYPE reasons_total counter
reasons_total{feature_flag_provider_name="flagd",feature_flag_reason="DEFAULT",otel_scope_name="openfeature/flagd",otel_scope_version=""} 1
reasons_total{feature_flag_provider_name="flagd",feature_flag_reason="STATIC",otel_scope_name="openfeature/flagd",otel_scope_version=""} 1
reasons_total{exception_type="FLAG_NOT_FOUND",feature_flag_provider_name="flagd",feature_flag_reason="ERROR",otel_scope_name="openfeature/flagd",otel_scope_version=""} 2
reasons_total{exception_type="TYPE_MISMATCH",feature_flag_provider_name="flagd",feature_flag_reason="ERROR",otel_scope_name="openfeature/flagd",otel_scope_version=""} 1
```

Signed-off-by: odubajDT <ondrej.dubaj@dynatrace.com>
Co-authored-by: Michael Beemer <beeme1mr@users.noreply.github.com>
Co-authored-by: James Milligan <75740990+james-milligan@users.noreply.github.com>
  • Loading branch information
3 people authored Apr 13, 2023
1 parent c25d0ef commit 77664cd
Show file tree
Hide file tree
Showing 7 changed files with 155 additions and 50 deletions.
4 changes: 3 additions & 1 deletion core/pkg/eval/ievaluator.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,16 @@ type AnyValue struct {
Variant string
Reason string
FlagKey string
Error error
}

func NewAnyValue(value interface{}, variant string, reason string, flagKey string) AnyValue {
func NewAnyValue(value interface{}, variant string, reason string, flagKey string, err error) AnyValue {
return AnyValue{
Value: value,
Variant: variant,
Reason: reason,
FlagKey: flagKey,
Error: err,
}
}

Expand Down
20 changes: 20 additions & 0 deletions core/pkg/eval/ievaluator_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package eval

import (
"fmt"
"testing"

"github.com/stretchr/testify/require"
)

func TestAnyValue(t *testing.T) {
obj := AnyValue{
Value: "val",
Variant: "variant",
Reason: "reason",
FlagKey: "key",
Error: fmt.Errorf("err"),
}

require.Equal(t, obj, NewAnyValue("val", "variant", "reason", "key", fmt.Errorf("err")))
}
7 changes: 3 additions & 4 deletions core/pkg/eval/json_evaluator.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,8 @@ func (je *JSONEvaluator) ResolveAllValues(reqID string, context *structpb.Struct
}
if err != nil {
je.Logger.ErrorWithID(reqID, fmt.Sprintf("bulk evaluation: key: %s returned error: %s", flagKey, err.Error()))
continue
}
values = append(values, NewAnyValue(value, variant, reason, flagKey))
values = append(values, NewAnyValue(value, variant, reason, flagKey, err))
}
return values
}
Expand Down Expand Up @@ -238,7 +237,7 @@ func (je *JSONEvaluator) evaluateVariant(
targetingBytes, err := targeting.MarshalJSON()
if err != nil {
je.Logger.ErrorWithID(reqID, fmt.Sprintf("Error parsing rules for flag: %s, %s", flagKey, err))
return "", model.ErrorReason, err
return "", model.ErrorReason, errors.New(model.ParseErrorCode)
}

b, err := json.Marshal(context)
Expand All @@ -252,7 +251,7 @@ func (je *JSONEvaluator) evaluateVariant(
err = jsonlogic.Apply(bytes.NewReader(targetingBytes), bytes.NewReader(b), &result)
if err != nil {
je.Logger.ErrorWithID(reqID, fmt.Sprintf("error applying rules: %s", err))
return "", model.ErrorReason, err
return "", model.ErrorReason, errors.New(model.ParseErrorCode)
}
// strip whitespace and quotes from the variant
variant = strings.ReplaceAll(strings.TrimSpace(result.String()), "\"", "")
Expand Down
17 changes: 10 additions & 7 deletions core/pkg/service/flag-evaluation/flag_evaluator.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ func (s *FlagEvaluationService) ResolveAll(
}
values := s.eval.ResolveAllValues(reqID, req.Msg.GetContext())
for _, value := range values {
// register the impression for each flag evaluated
s.metrics.Impressions(ctx, value.FlagKey, value.Variant)
// register the impression and reason for each flag evaluated
s.metrics.RecordEvaluation(ctx, value.Error, value.Reason, value.Variant, value.FlagKey)
switch v := value.Value.(type) {
case bool:
res.Flags[value.FlagKey] = &schemaV1.AnyFlag{
Expand Down Expand Up @@ -162,21 +162,22 @@ func resolve[T constraints](
zap.Strings("context-keys", formatContextKeys(ctx)),
)

var evalErrFormatted error
result, variant, reason, evalErr := resolver(reqID, flagKey, ctx)
if evalErr != nil {
logger.WarnWithID(reqID, fmt.Sprintf("returning error response, reason: %v", evalErr))
reason = model.ErrorReason
evalErr = errFormat(evalErr)
} else {
metrics.Impressions(goCtx, flagKey, variant)
evalErrFormatted = errFormat(evalErr)
}

metrics.RecordEvaluation(goCtx, evalErr, reason, variant, flagKey)

if err := resp.SetResult(result, variant, reason); err != nil && evalErr == nil {
logger.ErrorWithID(reqID, err.Error())
return err
}

return evalErr
return evalErrFormatted
}

func (s *FlagEvaluationService) ResolveBoolean(
Expand Down Expand Up @@ -283,10 +284,12 @@ func errFormat(err error) error {
return connect.NewError(connect.CodeNotFound, fmt.Errorf("%s, %s", ErrorPrefix, err.Error()))
case model.TypeMismatchErrorCode:
return connect.NewError(connect.CodeInvalidArgument, fmt.Errorf("%s, %s", ErrorPrefix, err.Error()))
case model.DisabledReason:
case model.FlagDisabledErrorCode:
return connect.NewError(connect.CodeUnavailable, fmt.Errorf("%s, %s", ErrorPrefix, err.Error()))
case model.ParseErrorCode:
return connect.NewError(connect.CodeDataLoss, fmt.Errorf("%s, %s", ErrorPrefix, err.Error()))
case model.GeneralErrorCode:
return connect.NewError(connect.CodeUnknown, fmt.Errorf("%s, %s", ErrorPrefix, err.Error()))
}

return err
Expand Down
10 changes: 5 additions & 5 deletions core/pkg/service/flag-evaluation/flag_evaluator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ func TestFlag_Evaluation_ResolveBoolean(t *testing.T) {
wantErr: nil,
},
"eval returns error": {
mCount: 0,
mCount: 1,
evalFields: resolveBooleanEvalFields{
result: true,
variant: ":(",
Expand Down Expand Up @@ -314,7 +314,7 @@ func TestFlag_Evaluation_ResolveString(t *testing.T) {
wantErr: nil,
},
"eval returns error": {
mCount: 0,
mCount: 1,
evalFields: resolveStringEvalFields{
result: "true",
variant: ":(",
Expand Down Expand Up @@ -463,7 +463,7 @@ func TestFlag_Evaluation_ResolveFloat(t *testing.T) {
wantErr: nil,
},
"eval returns error": {
mCount: 0,
mCount: 1,
evalFields: resolveFloatEvalFields{
result: 12,
variant: ":(",
Expand Down Expand Up @@ -612,7 +612,7 @@ func TestFlag_Evaluation_ResolveInt(t *testing.T) {
wantErr: nil,
},
"eval returns error": {
mCount: 0,
mCount: 1,
evalFields: resolveIntEvalFields{
result: 12,
variant: ":(",
Expand Down Expand Up @@ -763,7 +763,7 @@ func TestFlag_Evaluation_ResolveObject(t *testing.T) {
wantErr: nil,
},
"eval returns error": {
mCount: 0,
mCount: 1,
evalFields: resolveObjectEvalFields{
result: map[string]interface{}{
"food": "bars",
Expand Down
48 changes: 43 additions & 5 deletions core/pkg/telemetry/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,21 @@ import (
)

const (
requestDurationName = "http_request_duration_seconds"
responseSizeName = "http_response_size_bytes"
requestDurationName = "http_request_duration_seconds"
responseSizeName = "http_response_size_bytes"
FlagdProviderName = "flagd"
FeatureFlagReasonKeyName = "feature_flag.reason"
ExceptionTypeKeyName = "exception.type"
FeatureFlagReasonKey = attribute.Key(FeatureFlagReasonKeyName)
ExceptionTypeKey = attribute.Key(ExceptionTypeKeyName)
)

type MetricsRecorder struct {
httpRequestDurHistogram instrument.Float64Histogram
httpResponseSizeHistogram instrument.Float64Histogram
httpRequestsInflight instrument.Int64UpDownCounter
impressions instrument.Int64Counter
reasons instrument.Int64Counter
}

func (r MetricsRecorder) HTTPAttributes(svcName, url, method, code string) []attribute.KeyValue {
Expand Down Expand Up @@ -53,14 +59,33 @@ func (r MetricsRecorder) InFlightRequestEnd(ctx context.Context, attrs []attribu
r.httpRequestsInflight.Add(ctx, -1, attrs...)
}

func (r MetricsRecorder) Impressions(ctx context.Context, key, variant string) {
func (r MetricsRecorder) Impressions(ctx context.Context, reason, variant, key string) {
r.impressions.Add(ctx, 1, []attribute.KeyValue{
semconv.FeatureFlagProviderName(FlagdProviderName),
semconv.FeatureFlagKey(key),
semconv.FeatureFlagVariant(variant),
semconv.FeatureFlagProviderName("flagd"),
FeatureFlagReason(reason),
}...)
}

func (r MetricsRecorder) Reasons(ctx context.Context, reason string, err error) {
attrs := []attribute.KeyValue{
semconv.FeatureFlagProviderName(FlagdProviderName),
FeatureFlagReason(reason),
}
if err != nil {
attrs = append(attrs, ExceptionType(err.Error()))
}
r.reasons.Add(ctx, 1, attrs...)
}

func (r MetricsRecorder) RecordEvaluation(ctx context.Context, err error, reason, variant, key string) {
if err == nil {
r.Impressions(ctx, reason, variant, key)
}
r.Reasons(ctx, reason, err)
}

func getDurationView(svcName, viewName string, bucket []float64) metric.View {
return metric.NewView(
metric.Instrument{
Expand All @@ -76,6 +101,14 @@ func getDurationView(svcName, viewName string, bucket []float64) metric.View {
)
}

func FeatureFlagReason(val string) attribute.KeyValue {
return FeatureFlagReasonKey.String(val)
}

func ExceptionType(val string) attribute.KeyValue {
return ExceptionTypeKey.String(val)
}

// NewOTelRecorder creates a MetricsRecorder based on the provided metric.Reader. Note that, metric.NewMeterProvider is
// created here but not registered globally as this is the only place we derive a metric.Meter. Consider global provider
// registration if we need more meters
Expand Down Expand Up @@ -109,12 +142,17 @@ func NewOTelRecorder(exporter metric.Reader, resource *resource.Resource, servic
)
impressions, _ := meter.Int64Counter(
"impressions",
instrument.WithDescription("The number of evaluation for a given flag"),
instrument.WithDescription("The number of evaluations for a given flag"),
)
reasons, _ := meter.Int64Counter(
"reasons",
instrument.WithDescription("The number of evaluations for a given reason"),
)
return &MetricsRecorder{
httpRequestDurHistogram: hduration,
httpResponseSizeHistogram: hsize,
httpRequestsInflight: reqCounter,
impressions: impressions,
reasons: reasons,
}
}
Loading

0 comments on commit 77664cd

Please sign in to comment.