diff --git a/core/pkg/telemetry/builder.go b/core/pkg/telemetry/builder.go index 951cbd41a..2883e180e 100644 --- a/core/pkg/telemetry/builder.go +++ b/core/pkg/telemetry/builder.go @@ -43,7 +43,7 @@ func RegisterErrorHandling(log *logger.Logger) { // BuildMetricsRecorder is a helper to build telemetry.MetricsRecorder based on configurations func BuildMetricsRecorder( ctx context.Context, svcName string, svcVersion string, config Config, -) (*MetricsRecorder, error) { +) (IMetricsRecorder, error) { // Build metric reader based on configurations mReader, err := buildMetricReader(ctx, config) if err != nil { diff --git a/core/pkg/telemetry/metrics.go b/core/pkg/telemetry/metrics.go index 76d077a66..dcb3c7ea8 100644 --- a/core/pkg/telemetry/metrics.go +++ b/core/pkg/telemetry/metrics.go @@ -26,6 +26,40 @@ const ( reasonMetric = "feature_flag." + ProviderName + ".evaluation.reason" ) +type IMetricsRecorder interface { + HTTPAttributes(svcName, url, method, code string) []attribute.KeyValue + HTTPRequestDuration(ctx context.Context, duration time.Duration, attrs []attribute.KeyValue) + HTTPResponseSize(ctx context.Context, sizeBytes int64, attrs []attribute.KeyValue) + InFlightRequestStart(ctx context.Context, attrs []attribute.KeyValue) + InFlightRequestEnd(ctx context.Context, attrs []attribute.KeyValue) + RecordEvaluation(ctx context.Context, err error, reason, variant, key string) + Impressions(ctx context.Context, reason, variant, key string) +} + +type NoopMetricsRecorder struct{} + +func (NoopMetricsRecorder) HTTPAttributes(_, _, _, _ string) []attribute.KeyValue { + return []attribute.KeyValue{} +} + +func (NoopMetricsRecorder) HTTPRequestDuration(_ context.Context, _ time.Duration, _ []attribute.KeyValue) { +} + +func (NoopMetricsRecorder) HTTPResponseSize(_ context.Context, _ int64, _ []attribute.KeyValue) { +} + +func (NoopMetricsRecorder) InFlightRequestStart(_ context.Context, _ []attribute.KeyValue) { +} + +func (NoopMetricsRecorder) InFlightRequestEnd(_ context.Context, _ []attribute.KeyValue) { +} + +func (NoopMetricsRecorder) RecordEvaluation(_ context.Context, _ error, _, _, _ string) { +} + +func (NoopMetricsRecorder) Impressions(_ context.Context, _, _, _ string) { +} + type MetricsRecorder struct { httpRequestDurHistogram metric.Float64Histogram httpResponseSizeHistogram metric.Float64Histogram diff --git a/core/pkg/telemetry/metrics_test.go b/core/pkg/telemetry/metrics_test.go index 36ba79dca..20d556996 100644 --- a/core/pkg/telemetry/metrics_test.go +++ b/core/pkg/telemetry/metrics_test.go @@ -204,3 +204,35 @@ func TestMetrics(t *testing.T) { }) } } + +// some really simple tests just to make sure all methods are actually implemented and nothing panics +func TestNoopMetricsRecorder_HTTPAttributes(t *testing.T) { + no := NoopMetricsRecorder{} + got := no.HTTPAttributes("", "", "", "") + require.Empty(t, got) +} + +func TestNoopMetricsRecorder_HTTPRequestDuration(_ *testing.T) { + no := NoopMetricsRecorder{} + no.HTTPRequestDuration(context.TODO(), 0, nil) +} + +func TestNoopMetricsRecorder_InFlightRequestStart(_ *testing.T) { + no := NoopMetricsRecorder{} + no.InFlightRequestStart(context.TODO(), nil) +} + +func TestNoopMetricsRecorder_InFlightRequestEnd(_ *testing.T) { + no := NoopMetricsRecorder{} + no.InFlightRequestEnd(context.TODO(), nil) +} + +func TestNoopMetricsRecorder_RecordEvaluation(_ *testing.T) { + no := NoopMetricsRecorder{} + no.RecordEvaluation(context.TODO(), nil, "", "", "") +} + +func TestNoopMetricsRecorder_Impressions(_ *testing.T) { + no := NoopMetricsRecorder{} + no.Impressions(context.TODO(), "", "", "") +} diff --git a/flagd/pkg/runtime/from_config.go b/flagd/pkg/runtime/from_config.go index 28fe43075..0ec0cfd1a 100644 --- a/flagd/pkg/runtime/from_config.go +++ b/flagd/pkg/runtime/from_config.go @@ -51,13 +51,15 @@ func FromConfig(logger *logger.Logger, version string, config Config) (*Runtime, // register trace provider for the runtime err := telemetry.BuildTraceProvider(context.Background(), logger, svcName, version, telCfg) if err != nil { - return nil, fmt.Errorf("error building trace provider: %w", err) + // log the error but continue + logger.Error(fmt.Sprintf("error building trace provider: %v", err)) } // build metrics recorder with startup configurations recorder, err := telemetry.BuildMetricsRecorder(context.Background(), svcName, version, telCfg) if err != nil { - return nil, fmt.Errorf("error building metrics recorder: %w", err) + // log the error but continue + logger.Error(fmt.Sprintf("error building metrics recorder: %v", err)) } // build flag store, collect flag sources & fill sources details @@ -113,7 +115,8 @@ func FromConfig(logger *logger.Logger, version string, config Config) (*Runtime, options, err := telemetry.BuildConnectOptions(telCfg) if err != nil { - return nil, fmt.Errorf("failed to build connect options, %w", err) + // log the error but continue + logger.Error(fmt.Sprintf("failed to build connect options, %v", err)) } return &Runtime{ diff --git a/flagd/pkg/service/flag-evaluation/connect_service.go b/flagd/pkg/service/flag-evaluation/connect_service.go index d66c9c7f5..799d2546c 100644 --- a/flagd/pkg/service/flag-evaluation/connect_service.go +++ b/flagd/pkg/service/flag-evaluation/connect_service.go @@ -57,7 +57,7 @@ func (b bufSwitchHandler) ServeHTTP(writer http.ResponseWriter, request *http.Re type ConnectService struct { logger *logger.Logger eval evaluator.IEvaluator - metrics *telemetry.MetricsRecorder + metrics telemetry.IMetricsRecorder eventingConfiguration IEvents server *http.Server @@ -71,17 +71,21 @@ type ConnectService struct { // NewConnectService creates a ConnectService with provided parameters func NewConnectService( - logger *logger.Logger, evaluator evaluator.IEvaluator, mRecorder *telemetry.MetricsRecorder, + logger *logger.Logger, evaluator evaluator.IEvaluator, mRecorder telemetry.IMetricsRecorder, ) *ConnectService { - return &ConnectService{ + cs := &ConnectService{ logger: logger, eval: evaluator, - metrics: mRecorder, + metrics: &telemetry.NoopMetricsRecorder{}, eventingConfiguration: &eventingConfiguration{ subs: make(map[interface{}]chan service.Notification), mu: &sync.RWMutex{}, }, } + if mRecorder != nil { + cs.metrics = mRecorder + } + return cs } // Serve serves services with provided configuration options @@ -242,8 +246,8 @@ func (s *ConnectService) startServer(svcConf service.Configuration) error { func (s *ConnectService) startMetricsServer(svcConf service.Configuration) error { s.logger.Info(fmt.Sprintf("metrics and probes listening at %d", svcConf.ManagementPort)) - grpc := grpc.NewServer() - grpc_health_v1.RegisterHealthServer(grpc, health.NewServer()) + srv := grpc.NewServer() + grpc_health_v1.RegisterHealthServer(srv, health.NewServer()) mux := http.NewServeMux() mux.Handle("/healthz", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -261,7 +265,7 @@ func (s *ConnectService) startMetricsServer(svcConf service.Configuration) error handler := http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { // if this is 'application/grpc' and HTTP2, handle with gRPC, otherwise HTTP. if request.ProtoMajor == 2 && strings.HasPrefix(request.Header.Get("Content-Type"), "application/grpc") { - grpc.ServeHTTP(writer, request) + srv.ServeHTTP(writer, request) } else { mux.ServeHTTP(writer, request) return diff --git a/flagd/pkg/service/flag-evaluation/flag_evaluator.go b/flagd/pkg/service/flag-evaluation/flag_evaluator.go index 96a0a60b8..9e553b53a 100644 --- a/flagd/pkg/service/flag-evaluation/flag_evaluator.go +++ b/flagd/pkg/service/flag-evaluation/flag_evaluator.go @@ -29,25 +29,31 @@ type resolverSignature[T constraints] func(context context.Context, reqID, flagK type OldFlagEvaluationService struct { logger *logger.Logger eval evaluator.IEvaluator - metrics *telemetry.MetricsRecorder + metrics telemetry.IMetricsRecorder eventingConfiguration IEvents flagEvalTracer trace.Tracer } // NewOldFlagEvaluationService creates a OldFlagEvaluationService with provided parameters func NewOldFlagEvaluationService(log *logger.Logger, - eval evaluator.IEvaluator, eventingCfg IEvents, metricsRecorder *telemetry.MetricsRecorder, + eval evaluator.IEvaluator, eventingCfg IEvents, metricsRecorder telemetry.IMetricsRecorder, ) *OldFlagEvaluationService { - return &OldFlagEvaluationService{ + svc := &OldFlagEvaluationService{ logger: log, eval: eval, - metrics: metricsRecorder, + metrics: &telemetry.NoopMetricsRecorder{}, eventingConfiguration: eventingCfg, flagEvalTracer: otel.Tracer("flagEvaluationService"), } + + if metricsRecorder != nil { + svc.metrics = metricsRecorder + } + + return svc } -// nolint:dupl +// nolint:dupl,funlen func (s *OldFlagEvaluationService) ResolveAll( ctx context.Context, req *connect.Request[schemaV1.ResolveAllRequest], @@ -68,6 +74,7 @@ func (s *OldFlagEvaluationService) ResolveAll( for _, value := range values { // register the impression and reason for each flag evaluated s.metrics.RecordEvaluation(sCtx, value.Error, value.Reason, value.Variant, value.FlagKey) + switch v := value.Value.(type) { case bool: res.Flags[value.FlagKey] = &schemaV1.AnyFlag{ @@ -111,6 +118,7 @@ func (s *OldFlagEvaluationService) ResolveAll( return connect.NewResponse(res), nil } +// nolint:dupl func (s *OldFlagEvaluationService) EventStream( ctx context.Context, req *connect.Request[schemaV1.EventStreamRequest], @@ -276,7 +284,7 @@ func (s *OldFlagEvaluationService) ResolveObject( // resolve is a generic flag resolver func resolve[T constraints](ctx context.Context, logger *logger.Logger, resolver resolverSignature[T], flagKey string, - evaluationContext *structpb.Struct, resp response[T], metrics *telemetry.MetricsRecorder, + evaluationContext *structpb.Struct, resp response[T], metrics telemetry.IMetricsRecorder, ) error { reqID := xid.New().String() defer logger.ClearFields(reqID) @@ -295,7 +303,9 @@ func resolve[T constraints](ctx context.Context, logger *logger.Logger, resolver evalErrFormatted = errFormat(evalErr) } - metrics.RecordEvaluation(ctx, evalErr, reason, variant, flagKey) + if metrics != nil { + metrics.RecordEvaluation(ctx, evalErr, reason, variant, flagKey) + } spanFromContext := trace.SpanFromContext(ctx) spanFromContext.SetAttributes(telemetry.SemConvFeatureFlagAttributes(flagKey, variant)...) diff --git a/flagd/pkg/service/flag-evaluation/flag_evaluator_v2.go b/flagd/pkg/service/flag-evaluation/flag_evaluator_v2.go index 1e78734f2..b6eea6324 100644 --- a/flagd/pkg/service/flag-evaluation/flag_evaluator_v2.go +++ b/flagd/pkg/service/flag-evaluation/flag_evaluator_v2.go @@ -22,7 +22,7 @@ import ( type FlagEvaluationService struct { logger *logger.Logger eval evaluator.IEvaluator - metrics *telemetry.MetricsRecorder + metrics telemetry.IMetricsRecorder eventingConfiguration IEvents flagEvalTracer trace.Tracer } @@ -31,15 +31,21 @@ type FlagEvaluationService struct { func NewFlagEvaluationService(log *logger.Logger, eval evaluator.IEvaluator, eventingCfg IEvents, - metricsRecorder *telemetry.MetricsRecorder, + metricsRecorder telemetry.IMetricsRecorder, ) *FlagEvaluationService { - return &FlagEvaluationService{ + svc := &FlagEvaluationService{ logger: log, eval: eval, - metrics: metricsRecorder, + metrics: &telemetry.NoopMetricsRecorder{}, eventingConfiguration: eventingCfg, flagEvalTracer: otel.Tracer("flagd.evaluation.v1"), } + + if metricsRecorder != nil { + svc.metrics = metricsRecorder + } + + return svc } // nolint:dupl,funlen @@ -110,6 +116,7 @@ func (s *FlagEvaluationService) ResolveAll( return connect.NewResponse(res), nil } +// nolint: dupl func (s *FlagEvaluationService) EventStream( ctx context.Context, req *connect.Request[evalV1.EventStreamRequest], diff --git a/flagd/pkg/service/middleware/metrics/http_metrics.go b/flagd/pkg/service/middleware/metrics/http_metrics.go index 0a8034852..661b9d965 100644 --- a/flagd/pkg/service/middleware/metrics/http_metrics.go +++ b/flagd/pkg/service/middleware/metrics/http_metrics.go @@ -16,7 +16,7 @@ import ( ) type Config struct { - MetricRecorder *telemetry.MetricsRecorder + MetricRecorder telemetry.IMetricsRecorder Logger *logger.Logger Service string GroupedStatus bool @@ -41,7 +41,7 @@ func (cfg *Config) defaults() { log.Fatal("missing logger") } if cfg.MetricRecorder == nil { - cfg.Logger.Fatal("missing OpenTelemetry metric recorder") + cfg.MetricRecorder = &telemetry.NoopMetricsRecorder{} } }