From 544ab8e726a2b3a7e33ce9c9471b39a4e4a882a4 Mon Sep 17 00:00:00 2001 From: Goutham Annem Date: Tue, 8 Sep 2026 19:13:02 -0700 Subject: [PATCH] fix(spanlogger): set span error tag when log level is error When SpanLogger.Log() is called with level=error in the key-value pairs, the associated span's error tag was not being set, so errors logged through SpanLogger were invisible as errors in tracing UIs. Scan the kvps in Log() for a level=error pair and call ext.Error.Set when found, consistent with the existing Error() method behaviour. Fixes #3016 Signed-off-by: Goutham Annem --- CHANGELOG.md | 1 + pkg/util/spanlogger/spanlogger.go | 9 +++++++++ pkg/util/spanlogger/spanlogger_test.go | 16 ++++++++++++++++ 3 files changed, 26 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 28e50e86458..a048eb1edf3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -73,6 +73,7 @@ * [ENHANCEMENT] Distributor: Deduplicate metric metadata when converting PRW 2.0 requests. PRW 2.0 attaches metadata to every series, so a metric family was previously expanded into one `MetricMetadata` per series. #7760 * [ENHANCEMENT] Querier: Use non-pointer HistogramBucket slice in response codec. #7809 * [ENHANCEMENT] Update build image and Go version to 1.27.0. #7814 +* [BUGFIX] SpanLogger: Set span error tag when `SpanLogger.Log()` is called with `level=error`, so errors logged through SpanLogger are visible as errors in tracing UIs. #3016 * [BUGFIX] Querier: Fix queryWithRetry and labelsWithRetry returning (nil, nil) on cancelled context by propagating ctx.Err(). #7370 * [BUGFIX] Metrics Helper: Fix non-deterministic bucket order in merged histograms by sorting buckets after map iteration, matching Prometheus client library behavior. #7380 * [BUGFIX] Distributor: Return HTTP 401 Unauthorized when tenant ID resolution fails in the Prometheus Remote Write 2.0 path. #7389 diff --git a/pkg/util/spanlogger/spanlogger.go b/pkg/util/spanlogger/spanlogger.go index aa78dbea7f3..5be9387b607 100644 --- a/pkg/util/spanlogger/spanlogger.go +++ b/pkg/util/spanlogger/spanlogger.go @@ -90,6 +90,15 @@ func (s *SpanLogger) Log(kvps ...any) error { return err } s.LogFields(fields...) + // Mirror level=error onto the span's error tag so tracing UIs surface it. + for i := 0; i+1 < len(kvps); i += 2 { + if k, ok := kvps[i].(string); ok && k == "level" { + if v, ok := kvps[i+1].(string); ok && v == "error" { + ext.Error.Set(s.Span, true) + } + break + } + } return nil } diff --git a/pkg/util/spanlogger/spanlogger_test.go b/pkg/util/spanlogger/spanlogger_test.go index f522fa6f9f5..e13f3569a7e 100644 --- a/pkg/util/spanlogger/spanlogger_test.go +++ b/pkg/util/spanlogger/spanlogger_test.go @@ -60,6 +60,22 @@ func TestSpanCreatedWithoutTenantTag(t *testing.T) { require.False(t, exist) } +func TestSpanLogger_Log_SetsErrorTagOnErrorLevel(t *testing.T) { + mockTracer := mocktracer.New() + opentracing.SetGlobalTracer(mockTracer) + + logger, _ := New(context.Background(), "test") + mockSpan := logger.Span.(*mocktracer.MockSpan) + + // Non-error level should NOT set the error tag. + _ = logger.Log("level", "info", "msg", "hello") + require.Nil(t, mockSpan.Tag("error")) + + // level=error SHOULD set the span error tag. + _ = logger.Log("level", "error", "msg", "something failed") + require.Equal(t, true, mockSpan.Tag("error")) +} + func createSpan(ctx context.Context) *mocktracer.MockSpan { mockTracer := mocktracer.New() opentracing.SetGlobalTracer(mockTracer)