Skip to content
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.

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

feat: introduce metrics for failed evaluations #584

Merged
merged 3 commits into from
Apr 13, 2023
Merged
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: 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:
odubajDT marked this conversation as resolved.
Show resolved Hide resolved
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