Handle the error in SpanLogger.Log if log level is Error (#3016)

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## master / unreleased
 
 * [CHANGE] Querier / ruler: deprecated `-store.query-chunk-limit` CLI flag (and its respective YAML config option `max_chunks_per_query`) in favour of `-querier.max-fetched-chunks-per-query` (and its respective YAML config option `max_fetched_chunks_per_query`). The new limit specifies the maximum number of chunks that can be fetched in a single query from ingesters and long-term storage: the total number of actual fetched chunks could be 2x the limit, being independently applied when querying ingesters and long-term storage. #4125
+* [ENHANCEMENT] Tracing: flag a span with error if its log level is set to Error. #3995
 
 ## 1.9.0 in progress
 

pkg/chunk/cache/memcached.go

@@ -158,7 +158,6 @@ func (c *Memcached) fetch(ctx context.Context, keys []string) (found []string, b
 
 		// Memcached returns partial results even on error.
 		if err != nil {
-			log.Error(err)
 			level.Error(log).Log("msg", "Failed to get keys from memcached", "err", err)
 		}
 		return err

pkg/querier/queryrange/results_cache.go

@@ -570,7 +570,6 @@ func (s resultsCache) get(ctx context.Context, key string) ([]Extent, bool) {
 
 	if err := proto.Unmarshal(bufs[0], &resp); err != nil {
 		level.Error(log).Log("msg", "error unmarshalling cached value", "err", err)
-		log.Error(err)
 		return nil, false
 	}
 

pkg/testexporter/correctness/simple.go

@@ -176,8 +176,7 @@ func verifySamples(log *spanlogger.SpanLogger, tc Case, pairs []model.SamplePair
 	} else {
 		expectedNumSamples := int(duration / cfg.ScrapeInterval)
 		if math.Abs(float64(expectedNumSamples-len(pairs))) > 2 {
-			level.Error(log).Log("msg", "wrong number of samples", "expected", expectedNumSamples, "actual", len(pairs))
-			log.Error(fmt.Errorf("wrong number of samples"))
+			level.Error(log).Log("msg", "wrong number of samples", "expected", expectedNumSamples, "actual", len(pairs), "err", fmt.Errorf("wrong number of samples"))
 			return false
 		}
 	}

pkg/util/spanlogger/spanlogger.go

@@ -77,6 +77,31 @@ func FromContextWithFallback(ctx context.Context, fallback log.Logger) *SpanLogg
 // also puts the on the spans.
 func (s *SpanLogger) Log(kvps ...interface{}) error {
 	s.Logger.Log(kvps...)
+
+	var logAsError = false
+	errorIndex := -1
+	for i := 0; i < len(kvps)-1; i += 2 {
+		// Find out whether to log as error
+		if kvps[i] == level.Key() {
+			logAsError = kvps[i+1] == level.ErrorValue()
+			if !logAsError {
+				break
+			}
+			ext.Error.Set(s.Span, true)
+		} else if errorIndex == -1 {
+			// Check if this is the error we want to log
+			if _, ok := kvps[i+1].(error); ok && (kvps[i] == "err" || kvps[i] == "error") {
+				errorIndex = i
+			}
+		}
+		if logAsError && errorIndex != -1 {
+			s.Span.LogFields(otlog.Error(kvps[i+1].(error)))
+			// Remove the already logged error
+			kvps = append(kvps[:i], kvps[i+2:]...)
+			break
+		}
+	}
+
 	fields, err := otlog.InterleavedKVToFields(kvps...)
 	if err != nil {
 		return err

pkg/util/spanlogger/spanlogger_test.go

@@ -14,11 +14,10 @@ func TestSpanLogger_Log(t *testing.T) {
 	_ = span.Log("foo")
 	newSpan := FromContext(ctx)
 	require.Equal(t, span.Span, newSpan.Span)
-	_ = newSpan.Log("bar")
+	require.Error(t, newSpan.Log("bar", "err", errors.New("err"), "metric2", 2))
 	noSpan := FromContext(context.Background())
 	_ = noSpan.Log("foo")
-	require.Error(t, noSpan.Error(errors.New("err")))
-	require.NoError(t, noSpan.Error(nil))
+	require.NoError(t, noSpan.Log("metric1", 1, "err", errors.New("err"), "metric2", 2))
 }
 
 func TestSpanLogger_CustomLogger(t *testing.T) {