From 82e5b81a30b9cc1e84313441cefa9262ad68d4c6 Mon Sep 17 00:00:00 2001 From: Tania <10127682+undef1nd@users.noreply.github.com> Date: Thu, 23 Oct 2025 12:09:41 +0200 Subject: [PATCH] OpenFeature: Fix instrumentation in OFREP API (#112845) OpenFeature: Fix instrumentation in ofrep api --- pkg/registry/apis/ofrep/proxy.go | 4 ++-- pkg/registry/apis/ofrep/register.go | 26 +++++++++++--------------- pkg/registry/apis/ofrep/static.go | 4 ++-- 3 files changed, 15 insertions(+), 19 deletions(-) diff --git a/pkg/registry/apis/ofrep/proxy.go b/pkg/registry/apis/ofrep/proxy.go index b5791925b26..c61a31bfc59 100644 --- a/pkg/registry/apis/ofrep/proxy.go +++ b/pkg/registry/apis/ofrep/proxy.go @@ -22,7 +22,7 @@ import ( ) func (b *APIBuilder) proxyAllFlagReq(ctx context.Context, isAuthedUser bool, w http.ResponseWriter, r *http.Request) { - ctx, span := tracer.Start(ctx, "ofrep.proxy.evalAllFlags") + ctx, span := tracing.Start(ctx, "ofrep.proxy.evalAllFlags") defer span.End() r = r.WithContext(ctx) @@ -70,7 +70,7 @@ func (b *APIBuilder) proxyAllFlagReq(ctx context.Context, isAuthedUser bool, w h } func (b *APIBuilder) proxyFlagReq(ctx context.Context, flagKey string, isAuthedUser bool, w http.ResponseWriter, r *http.Request) { - ctx, span := tracer.Start(ctx, "ofrep.proxy.evalFlag") + ctx, span := tracing.Start(ctx, "ofrep.proxy.evalFlag") defer span.End() r = r.WithContext(ctx) diff --git a/pkg/registry/apis/ofrep/register.go b/pkg/registry/apis/ofrep/register.go index 0876baeea2e..59da5877250 100644 --- a/pkg/registry/apis/ofrep/register.go +++ b/pkg/registry/apis/ofrep/register.go @@ -11,7 +11,6 @@ import ( "github.com/gorilla/mux" "github.com/grafana/grafana/pkg/infra/tracing" - "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" semconv "go.opentelemetry.io/otel/semconv/v1.21.0" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -35,8 +34,6 @@ var _ builder.APIGroupBuilder = (*APIBuilder)(nil) var _ builder.APIGroupRouteProvider = (*APIBuilder)(nil) var _ builder.APIGroupVersionProvider = (*APIBuilder)(nil) -var tracer = otel.Tracer("github.com/grafana/grafana/pkg/registry/apis/ofrep") - const ofrepPath = "/ofrep/v1/evaluate/flags" const namespaceMismatchMsg = "rejecting request with namespace mismatch" @@ -246,13 +243,14 @@ func (b *APIBuilder) GetAPIRoutes(gv schema.GroupVersion) *builder.APIRoutes { } func (b *APIBuilder) oneFlagHandler(w http.ResponseWriter, r *http.Request) { - ctx, span := tracer.Start(r.Context(), "ofrep.handler.evalFlag") + ctx, span := tracing.Start(r.Context(), "ofrep.handler.evalFlag") defer span.End() r = r.WithContext(ctx) - b.logger.Debug("validating namespace in oneFlagHandler handler") - if !b.validateNamespace(r) { + valid := b.validateNamespace(r) + b.logger.Debug("validating namespace in oneFlagHandler handler", "valid", valid) + if !valid { _ = tracing.Errorf(span, namespaceMismatchMsg) span.SetAttributes(semconv.HTTPStatusCode(http.StatusUnauthorized)) b.logger.Error(namespaceMismatchMsg) @@ -291,13 +289,15 @@ func (b *APIBuilder) oneFlagHandler(w http.ResponseWriter, r *http.Request) { } func (b *APIBuilder) allFlagsHandler(w http.ResponseWriter, r *http.Request) { - ctx, span := tracer.Start(r.Context(), "ofrep.handler.evalAllFlags") + ctx, span := tracing.Start(r.Context(), "ofrep.handler.evalAllFlags") defer span.End() r = r.WithContext(ctx) - b.logger.Debug("validating namespace in allFlagsHandler handler") - if !b.validateNamespace(r) { + valid := b.validateNamespace(r) + b.logger.Debug("validating namespace in allFlagsHandler handler", "valid", valid) + + if !valid { _ = tracing.Errorf(span, namespaceMismatchMsg) span.SetAttributes(semconv.HTTPStatusCode(http.StatusUnauthorized)) b.logger.Error(namespaceMismatchMsg) @@ -327,8 +327,7 @@ func writeResponse(statusCode int, result any, logger log.Logger, w http.Respons func (b *APIBuilder) namespaceFromEvalCtx(body []byte) string { // TODO: eval ctx should be added to span attributes, not log - // Adding it temporary for debugging - b.logger.Debug("evaluation context from request", "ctx", body) + b.logger.Debug("evaluation context from request", "ctx", string(body)) var evalCtx struct { // Extract namespace from request body without consuming it @@ -342,9 +341,6 @@ func (b *APIBuilder) namespaceFromEvalCtx(body []byte) string { return "" } - // Adding it temporary for debugging - b.logger.Debug("evaluation context decoded", "namespace", evalCtx.Context.Namespace) - if evalCtx.Context.Namespace == "" { b.logger.Debug("namespace missing from evaluation context", "namespace", evalCtx.Context.Namespace) return "" @@ -364,7 +360,7 @@ func (b *APIBuilder) isAuthenticatedRequest(r *http.Request) bool { // validateNamespace checks if the namespace in the evaluation context matches the namespace in the request func (b *APIBuilder) validateNamespace(r *http.Request) bool { - _, span := tracer.Start(r.Context(), "ofrep.validateNamespace") + _, span := tracing.Start(r.Context(), "ofrep.validateNamespace") defer span.End() var namespace string diff --git a/pkg/registry/apis/ofrep/static.go b/pkg/registry/apis/ofrep/static.go index 18ff794349d..824effb3382 100644 --- a/pkg/registry/apis/ofrep/static.go +++ b/pkg/registry/apis/ofrep/static.go @@ -10,7 +10,7 @@ import ( ) func (b *APIBuilder) evalAllFlagsStatic(ctx context.Context, isAuthedUser bool, w http.ResponseWriter) { - _, span := tracer.Start(ctx, "ofrep.static.evalAllFlags") + _, span := tracing.Start(ctx, "ofrep.static.evalAllFlags") defer span.End() result, err := b.staticEvaluator.EvalAllFlags(ctx) @@ -40,7 +40,7 @@ func (b *APIBuilder) evalAllFlagsStatic(ctx context.Context, isAuthedUser bool, } func (b *APIBuilder) evalFlagStatic(ctx context.Context, flagKey string, w http.ResponseWriter) { - _, span := tracer.Start(ctx, "ofrep.static.evalFlag") + _, span := tracing.Start(ctx, "ofrep.static.evalFlag") defer span.End() span.SetAttributes(attribute.String("flag_key", flagKey))