From 2b5e7c3f050362b5b397ec5320ec36fd5d8837b5 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 27 Aug 2026 13:43:28 +0200 Subject: [PATCH] otel: avoid span lookup for entries without context Logrus entries are not required to have a context, so return early when one is not set instead of performing an unnecessary OpenTelemetry span lookup. Also reuse the span context when validating it and extracting the trace ID, and update the test to exercise the hook through Logrus' normal logging path, including entries with a nil context. Signed-off-by: Sebastiaan van Stijn --- otel/log.go | 9 +++++---- otel/log_test.go | 47 +++++++++++++++++++++++++++++------------------ 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/otel/log.go b/otel/log.go index a7fc5bd..8b41f37 100644 --- a/otel/log.go +++ b/otel/log.go @@ -72,17 +72,18 @@ func (h *LogrusHook) Levels() []log.Level { // Fire is called when a log event occurs. func (h *LogrusHook) Fire(entry *log.Entry) error { - span := trace.SpanFromContext(entry.Context) - if span == nil { + if entry.Context == nil { return nil } - if !span.SpanContext().IsValid() { + span := trace.SpanFromContext(entry.Context) + spanCtx := span.SpanContext() + if !spanCtx.IsValid() { return nil } if h.enableTraceIDField { - entry.Data["trace_id"] = span.SpanContext().TraceID().String() + entry.Data["trace_id"] = spanCtx.TraceID().String() } if !span.IsRecording() { diff --git a/otel/log_test.go b/otel/log_test.go index b5b2fd8..d68dfa8 100644 --- a/otel/log_test.go +++ b/otel/log_test.go @@ -18,10 +18,12 @@ package otel_test import ( "context" + "io" "testing" - "github.com/containerd/log" "github.com/containerd/log/otel" + "github.com/sirupsen/logrus" + "github.com/sirupsen/logrus/hooks/test" "go.opentelemetry.io/otel/trace" ) @@ -36,6 +38,7 @@ func TestLogrusHookTraceID(t *testing.T) { tests := []struct { name string enableOpt bool + nilContext bool withSpan bool expectedTID string }{ @@ -55,30 +58,38 @@ func TestLogrusHookTraceID(t *testing.T) { enableOpt: true, withSpan: false, }, + { + name: "TraceIDNotInjected_NoContext", + enableOpt: true, + nilContext: true, + }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - ctx := context.Background() - if tc.withSpan { - ctx = trace.ContextWithSpanContext( - ctx, - trace.NewSpanContext(trace.SpanContextConfig{ - TraceID: testTraceID, - SpanID: testSpanID, - }), - ) - } + logger := logrus.New() + logger.SetOutput(io.Discard) + logger.AddHook(otel.NewLogrusHook(otel.WithTraceIDField(tc.enableOpt))) + testHook := test.NewLocal(logger) + + switch { + case tc.withSpan: + ctx := trace.ContextWithSpanContext(context.Background(), trace.NewSpanContext(trace.SpanContextConfig{ + TraceID: testTraceID, + SpanID: testSpanID, + })) + logger.WithContext(ctx).Info("test") + + case tc.nilContext: + logger.Info("test") - hook := otel.NewLogrusHook(otel.WithTraceIDField(tc.enableOpt)) - entry := &log.Entry{ - Context: ctx, - Data: make(log.Fields), + default: + logger.WithContext(context.Background()).Info("test") } - err := hook.Fire(entry) - if err != nil { - t.Fatal(err) + entry := testHook.LastEntry() + if entry == nil { + t.Fatal("expected log entry") } traceID, ok := entry.Data["trace_id"]