From 086b41f06498ecb0e44c78b8e67493b24ff568d1 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Wed, 15 Sep 2021 01:26:56 +0000 Subject: [PATCH 01/12] Add open telemetry logging hook for logrus This adds valuable logging data to the open telemetry traces. When the trace is not recording we don't bother doing anything as it is relatively expensive to convert logrus data to otel just due to the nature of how logrus works. The way this works is that we now set a context on the logrus.Entry that gets passed around which the hook then uses to determine if there is an active span to forward the logs to. Signed-off-by: Brian Goff --- otel/log.go | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 otel/log.go diff --git a/otel/log.go b/otel/log.go new file mode 100644 index 0000000..a8c540b --- /dev/null +++ b/otel/log.go @@ -0,0 +1,66 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package tracing + +import ( + "github.com/sirupsen/logrus" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/trace" +) + +// NewLogrusHook creates a new logrus hook +func NewLogrusHook() *LogrusHook { + return &LogrusHook{} +} + +// LogrusHook is a logrus hook which adds logrus events to active spans. +// If the span is not recording or the span context is invalid, the hook is a no-op. +type LogrusHook struct{} + +// Levels returns the logrus levels that this hook is interested in. +func (h *LogrusHook) Levels() []logrus.Level { + return logrus.AllLevels +} + +// Fire is called when a log event occurs. +func (h *LogrusHook) Fire(entry *logrus.Entry) error { + span := trace.SpanFromContext(entry.Context) + if span == nil { + return nil + } + + if !span.SpanContext().IsValid() || !span.IsRecording() { + return nil + } + + span.AddEvent( + entry.Message, + trace.WithAttributes(logrusDataToAttrs(entry.Data)...), + trace.WithAttributes(attribute.String("level", entry.Level.String())), + trace.WithTimestamp(entry.Time), + ) + + return nil +} + +func logrusDataToAttrs(data logrus.Fields) []attribute.KeyValue { + attrs := make([]attribute.KeyValue, 0, len(data)) + for k, v := range data { + attrs = append(attrs, attribute.Any(k, v)) + } + return attrs +} From 19f157d81cee82d981c3bdf95f3d7ff19a9e8011 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Tue, 28 Sep 2021 23:08:18 +0000 Subject: [PATCH 02/12] Update go otel 1.0.1 This fixes the issue with the usage of the deprecated attribute.Any function that original caused build issues. Signed-off-by: Brian Goff --- otel/log.go | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 65 insertions(+), 1 deletion(-) diff --git a/otel/log.go b/otel/log.go index a8c540b..6c6dd6d 100644 --- a/otel/log.go +++ b/otel/log.go @@ -17,6 +17,9 @@ package tracing import ( + "encoding/json" + "fmt" + "github.com/sirupsen/logrus" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" @@ -60,7 +63,68 @@ func (h *LogrusHook) Fire(entry *logrus.Entry) error { func logrusDataToAttrs(data logrus.Fields) []attribute.KeyValue { attrs := make([]attribute.KeyValue, 0, len(data)) for k, v := range data { - attrs = append(attrs, attribute.Any(k, v)) + attrs = append(attrs, any(k, v)) } return attrs } + +func any(k string, v interface{}) attribute.KeyValue { + if v == nil { + return attribute.String(k, "") + } + + switch typed := v.(type) { + case bool: + return attribute.Bool(k, typed) + case []bool: + return attribute.BoolSlice(k, typed) + case int: + return attribute.Int(k, typed) + case []int: + return attribute.IntSlice(k, typed) + case int8: + return attribute.Int(k, int(typed)) + case []int8: + ls := make([]int, 0, len(typed)) + for _, i := range typed { + ls = append(ls, int(i)) + } + return attribute.IntSlice(k, ls) + case int16: + return attribute.Int(k, int(typed)) + case []int16: + ls := make([]int, 0, len(typed)) + for _, i := range typed { + ls = append(ls, int(i)) + } + return attribute.IntSlice(k, ls) + case int32: + return attribute.Int64(k, int64(typed)) + case []int32: + ls := make([]int64, 0, len(typed)) + for _, i := range typed { + ls = append(ls, int64(i)) + } + return attribute.Int64Slice(k, ls) + case int64: + return attribute.Int64(k, typed) + case []int64: + return attribute.Int64Slice(k, typed) + case float64: + return attribute.Float64(k, typed) + case []float64: + return attribute.Float64Slice(k, typed) + case string: + return attribute.String(k, typed) + case []string: + return attribute.StringSlice(k, typed) + } + + if stringer, ok := v.(fmt.Stringer); ok { + return attribute.String(k, stringer.String()) + } + if b, err := json.Marshal(v); b != nil && err == nil { + return attribute.String(k, string(b)) + } + return attribute.String(k, fmt.Sprintf("%v", v)) +} From 4c827d1addf7586f6b763337121b30aaf7cf08de Mon Sep 17 00:00:00 2001 From: Swagat Bora Date: Thu, 3 Nov 2022 16:50:50 +0000 Subject: [PATCH 03/12] add SpanAttribute Signed-off-by: Swagat Bora --- otel/helpers.go | 85 +++++++++++++++++++++++++++++++++++++++++++++++++ otel/log.go | 64 ------------------------------------- 2 files changed, 85 insertions(+), 64 deletions(-) create mode 100644 otel/helpers.go diff --git a/otel/helpers.go b/otel/helpers.go new file mode 100644 index 0000000..0357709 --- /dev/null +++ b/otel/helpers.go @@ -0,0 +1,85 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package tracing + +import ( + "encoding/json" + "fmt" + + "go.opentelemetry.io/otel/attribute" +) + +func any(k string, v interface{}) attribute.KeyValue { + if v == nil { + return attribute.String(k, "") + } + + switch typed := v.(type) { + case bool: + return attribute.Bool(k, typed) + case []bool: + return attribute.BoolSlice(k, typed) + case int: + return attribute.Int(k, typed) + case []int: + return attribute.IntSlice(k, typed) + case int8: + return attribute.Int(k, int(typed)) + case []int8: + ls := make([]int, 0, len(typed)) + for _, i := range typed { + ls = append(ls, int(i)) + } + return attribute.IntSlice(k, ls) + case int16: + return attribute.Int(k, int(typed)) + case []int16: + ls := make([]int, 0, len(typed)) + for _, i := range typed { + ls = append(ls, int(i)) + } + return attribute.IntSlice(k, ls) + case int32: + return attribute.Int64(k, int64(typed)) + case []int32: + ls := make([]int64, 0, len(typed)) + for _, i := range typed { + ls = append(ls, int64(i)) + } + return attribute.Int64Slice(k, ls) + case int64: + return attribute.Int64(k, typed) + case []int64: + return attribute.Int64Slice(k, typed) + case float64: + return attribute.Float64(k, typed) + case []float64: + return attribute.Float64Slice(k, typed) + case string: + return attribute.String(k, typed) + case []string: + return attribute.StringSlice(k, typed) + } + + if stringer, ok := v.(fmt.Stringer); ok { + return attribute.String(k, stringer.String()) + } + if b, err := json.Marshal(v); b != nil && err == nil { + return attribute.String(k, string(b)) + } + return attribute.String(k, fmt.Sprintf("%v", v)) +} diff --git a/otel/log.go b/otel/log.go index 6c6dd6d..98fa16f 100644 --- a/otel/log.go +++ b/otel/log.go @@ -17,9 +17,6 @@ package tracing import ( - "encoding/json" - "fmt" - "github.com/sirupsen/logrus" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" @@ -67,64 +64,3 @@ func logrusDataToAttrs(data logrus.Fields) []attribute.KeyValue { } return attrs } - -func any(k string, v interface{}) attribute.KeyValue { - if v == nil { - return attribute.String(k, "") - } - - switch typed := v.(type) { - case bool: - return attribute.Bool(k, typed) - case []bool: - return attribute.BoolSlice(k, typed) - case int: - return attribute.Int(k, typed) - case []int: - return attribute.IntSlice(k, typed) - case int8: - return attribute.Int(k, int(typed)) - case []int8: - ls := make([]int, 0, len(typed)) - for _, i := range typed { - ls = append(ls, int(i)) - } - return attribute.IntSlice(k, ls) - case int16: - return attribute.Int(k, int(typed)) - case []int16: - ls := make([]int, 0, len(typed)) - for _, i := range typed { - ls = append(ls, int(i)) - } - return attribute.IntSlice(k, ls) - case int32: - return attribute.Int64(k, int64(typed)) - case []int32: - ls := make([]int64, 0, len(typed)) - for _, i := range typed { - ls = append(ls, int64(i)) - } - return attribute.Int64Slice(k, ls) - case int64: - return attribute.Int64(k, typed) - case []int64: - return attribute.Int64Slice(k, typed) - case float64: - return attribute.Float64(k, typed) - case []float64: - return attribute.Float64Slice(k, typed) - case string: - return attribute.String(k, typed) - case []string: - return attribute.StringSlice(k, typed) - } - - if stringer, ok := v.(fmt.Stringer); ok { - return attribute.String(k, stringer.String()) - } - if b, err := json.Marshal(v); b != nil && err == nil { - return attribute.String(k, string(b)) - } - return attribute.String(k, fmt.Sprintf("%v", v)) -} From ae97774593cfc796f50c584ce112b37aaece6f6d Mon Sep 17 00:00:00 2001 From: Swagat Bora Date: Wed, 9 Nov 2022 21:47:44 +0000 Subject: [PATCH 04/12] Add a thin wrapper around otel Span object Signed-off-by: Swagat Bora --- otel/helpers.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/otel/helpers.go b/otel/helpers.go index 0357709..981da6c 100644 --- a/otel/helpers.go +++ b/otel/helpers.go @@ -19,10 +19,19 @@ package tracing import ( "encoding/json" "fmt" + "strings" "go.opentelemetry.io/otel/attribute" ) +const ( + spanDelimiter = "." +) + +func makeSpanName(names ...string) string { + return strings.Join(names, spanDelimiter) +} + func any(k string, v interface{}) attribute.KeyValue { if v == nil { return attribute.String(k, "") From 63e4572ad08ea155b6d63eb66970ef453c0c3158 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 14 Jun 2024 11:32:19 +0200 Subject: [PATCH 05/12] pkg/tracing: rename func that shadowed builtin, rm makeSpanName Signed-off-by: Sebastiaan van Stijn --- otel/helpers.go | 11 +---------- otel/log.go | 2 +- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/otel/helpers.go b/otel/helpers.go index 981da6c..ab1278e 100644 --- a/otel/helpers.go +++ b/otel/helpers.go @@ -19,20 +19,11 @@ package tracing import ( "encoding/json" "fmt" - "strings" "go.opentelemetry.io/otel/attribute" ) -const ( - spanDelimiter = "." -) - -func makeSpanName(names ...string) string { - return strings.Join(names, spanDelimiter) -} - -func any(k string, v interface{}) attribute.KeyValue { +func keyValue(k string, v any) attribute.KeyValue { if v == nil { return attribute.String(k, "") } diff --git a/otel/log.go b/otel/log.go index 98fa16f..5834d03 100644 --- a/otel/log.go +++ b/otel/log.go @@ -60,7 +60,7 @@ func (h *LogrusHook) Fire(entry *logrus.Entry) error { func logrusDataToAttrs(data logrus.Fields) []attribute.KeyValue { attrs := make([]attribute.KeyValue, 0, len(data)) for k, v := range data { - attrs = append(attrs, any(k, v)) + attrs = append(attrs, keyValue(k, v)) } return attrs } From f84d90f1622c4f2f16b78e774f484af6cf2c7446 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 14 Jun 2024 12:08:51 +0200 Subject: [PATCH 06/12] pkg/tracing: remove direct use of github.com/sirupsen/logrus While the hook is intended to be used with logrus, we don't need to have the direct import; use the aliases provided by the containerd/log module instead. Signed-off-by: Sebastiaan van Stijn --- otel/log.go | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/otel/log.go b/otel/log.go index 5834d03..f2604cd 100644 --- a/otel/log.go +++ b/otel/log.go @@ -17,27 +17,43 @@ package tracing import ( - "github.com/sirupsen/logrus" + "github.com/containerd/log" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" ) +// allLevels is the equivalent to [logrus.AllLevels]. +// +// [logrus.AllLevels]: https://github.com/sirupsen/logrus/blob/v1.9.3/logrus.go#L80-L89 +var allLevels = []log.Level{ + log.PanicLevel, + log.FatalLevel, + log.ErrorLevel, + log.WarnLevel, + log.InfoLevel, + log.DebugLevel, + log.TraceLevel, +} + // NewLogrusHook creates a new logrus hook func NewLogrusHook() *LogrusHook { return &LogrusHook{} } -// LogrusHook is a logrus hook which adds logrus events to active spans. -// If the span is not recording or the span context is invalid, the hook is a no-op. +// LogrusHook is a [logrus.Hook] which adds logrus events to active spans. +// If the span is not recording or the span context is invalid, the hook +// is a no-op. +// +// [logrus.Hook]: https://github.com/sirupsen/logrus/blob/v1.9.3/hooks.go#L3-L11 type LogrusHook struct{} // Levels returns the logrus levels that this hook is interested in. -func (h *LogrusHook) Levels() []logrus.Level { - return logrus.AllLevels +func (h *LogrusHook) Levels() []log.Level { + return allLevels } // Fire is called when a log event occurs. -func (h *LogrusHook) Fire(entry *logrus.Entry) error { +func (h *LogrusHook) Fire(entry *log.Entry) error { span := trace.SpanFromContext(entry.Context) if span == nil { return nil @@ -57,7 +73,7 @@ func (h *LogrusHook) Fire(entry *logrus.Entry) error { return nil } -func logrusDataToAttrs(data logrus.Fields) []attribute.KeyValue { +func logrusDataToAttrs(data map[string]any) []attribute.KeyValue { attrs := make([]attribute.KeyValue, 0, len(data)) for k, v := range data { attrs = append(attrs, keyValue(k, v)) From 4e78d3b5a429ac5e5a5b28dbe94d54a84000c04a Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 14 Jun 2024 12:12:22 +0200 Subject: [PATCH 07/12] pkg/tracing: LogrusHook.Fire: micro-optimisation Check span.IsRecording first, as it's a more lightweight check than span.SpanContext().IsValid() Signed-off-by: Sebastiaan van Stijn --- otel/log.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/otel/log.go b/otel/log.go index f2604cd..3af24a2 100644 --- a/otel/log.go +++ b/otel/log.go @@ -59,7 +59,7 @@ func (h *LogrusHook) Fire(entry *log.Entry) error { return nil } - if !span.SpanContext().IsValid() || !span.IsRecording() { + if !span.IsRecording() || !span.SpanContext().IsValid() { return nil } From 7aacd4a10f04def47df40246ab128d34562f8116 Mon Sep 17 00:00:00 2001 From: Hasan Siddiqui Date: Mon, 23 Mar 2026 23:00:12 +0000 Subject: [PATCH 08/12] tracing: add option to inject trace ID into logrus fields Introduce functional options to NewLogrusHook to allow optional Trace ID injection into log fields. This enables log-trace correlation via the [debug] config without breaking existing external consumers of pkg/tracing. Signed-off-by: Hasan Siddiqui --- otel/log.go | 30 +++++++++++++--- otel/log_test.go | 91 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 4 deletions(-) create mode 100644 otel/log_test.go diff --git a/otel/log.go b/otel/log.go index 3af24a2..b1c3a74 100644 --- a/otel/log.go +++ b/otel/log.go @@ -35,9 +35,21 @@ var allLevels = []log.Level{ log.TraceLevel, } +type HookOpt func(*LogrusHook) + // NewLogrusHook creates a new logrus hook -func NewLogrusHook() *LogrusHook { - return &LogrusHook{} +func NewLogrusHook(opts ...HookOpt) *LogrusHook { + hook := &LogrusHook{} + for _, opt := range opts { + opt(hook) + } + return hook +} + +func WithTraceIDField(enabled bool) HookOpt { + return func(h *LogrusHook) { + h.enableTraceIDField = enabled + } } // LogrusHook is a [logrus.Hook] which adds logrus events to active spans. @@ -45,7 +57,9 @@ func NewLogrusHook() *LogrusHook { // is a no-op. // // [logrus.Hook]: https://github.com/sirupsen/logrus/blob/v1.9.3/hooks.go#L3-L11 -type LogrusHook struct{} +type LogrusHook struct { + enableTraceIDField bool +} // Levels returns the logrus levels that this hook is interested in. func (h *LogrusHook) Levels() []log.Level { @@ -59,7 +73,15 @@ func (h *LogrusHook) Fire(entry *log.Entry) error { return nil } - if !span.IsRecording() || !span.SpanContext().IsValid() { + if !span.SpanContext().IsValid() { + return nil + } + + if h.enableTraceIDField { + entry.Data["trace_id"] = span.SpanContext().TraceID().String() + } + + if !span.IsRecording() { return nil } diff --git a/otel/log_test.go b/otel/log_test.go new file mode 100644 index 0000000..ced6d30 --- /dev/null +++ b/otel/log_test.go @@ -0,0 +1,91 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package tracing + +import ( + "context" + "testing" + + "github.com/containerd/log" + "github.com/stretchr/testify/assert" + "go.opentelemetry.io/otel/trace" +) + +const expectedTraceIDStr = "0102030405060708090a0b0c0d0e0f10" + +var ( + testTraceID = trace.TraceID{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16} + testSpanID = trace.SpanID{1, 2, 3, 4, 5, 6, 7, 8} +) + +func TestLogrusHookTraceID(t *testing.T) { + tests := []struct { + name string + enableOpt bool + withSpan bool + expectedTID string + }{ + { + name: "TraceIDInjected", + enableOpt: true, + withSpan: true, + expectedTID: expectedTraceIDStr, + }, + { + name: "TraceIDNotInjected_OptionDisabled", + enableOpt: false, + withSpan: true, + }, + { + name: "TraceIDNotInjected_NoSpan", + enableOpt: true, + withSpan: false, + }, + } + + 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, + }), + ) + } + + hook := NewLogrusHook(WithTraceIDField(tc.enableOpt)) + entry := &log.Entry{ + Context: ctx, + Data: make(log.Fields), + } + + err := hook.Fire(entry) + assert.NoError(t, err) + + traceID, ok := entry.Data["trace_id"] + if tc.expectedTID != "" { + assert.True(t, ok) + assert.Equal(t, tc.expectedTID, traceID) + } else { + assert.False(t, ok) + } + }) + } +} From ae7e6884646ade8036be13ad57c2f4dc2f60f75a Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 22 Aug 2026 19:36:03 +0200 Subject: [PATCH 09/12] otel: fix imports, remove testify, add go.mod Make the otel package a separate module, so that containerd/log does not inherit the otel dependencies. Signed-off-by: Sebastiaan van Stijn --- .github/workflows/ci.yml | 18 +++++++++++++++--- otel/go.mod | 14 ++++++++++++++ otel/go.sum | 16 ++++++++++++++++ otel/helpers.go | 2 +- otel/log.go | 6 +++++- otel/log_test.go | 22 ++++++++++++++-------- 6 files changed, 65 insertions(+), 13 deletions(-) create mode 100644 otel/go.mod create mode 100644 otel/go.sum diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5191c19..8c5056e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,11 +36,19 @@ jobs: cache: false # see actions/setup-go#368 - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - - uses: golangci/golangci-lint-action@ba0d7d2ec06a0ea1cb5fa41b2e4a3ab91d21278a # v9.3.0 + - name: Lint + uses: golangci/golangci-lint-action@ba0d7d2ec06a0ea1cb5fa41b2e4a3ab91d21278a # v9.3.0 with: version: v2.13 skip-cache: true + - name: Lint otel + uses: golangci/golangci-lint-action@ba0d7d2ec06a0ea1cb5fa41b2e4a3ab91d21278a # v9.3.0 + with: + version: v2.13 + skip-cache: true + working-directory: otel + # # Project checks # @@ -95,6 +103,10 @@ jobs: echo "GOPATH=${{ github.workspace }}" >> $GITHUB_ENV echo "${{ github.workspace }}/bin" >> $GITHUB_PATH - - run: | - go test -v -race + - name: Test + run: go test -v -race working-directory: src/github.com/containerd/log + + - name: Test otel + run: go test -v -race + working-directory: src/github.com/containerd/log/otel diff --git a/otel/go.mod b/otel/go.mod new file mode 100644 index 0000000..c198532 --- /dev/null +++ b/otel/go.mod @@ -0,0 +1,14 @@ +module github.com/containerd/log/otel + +go 1.23 + +require ( + github.com/containerd/log v0.1.0 + go.opentelemetry.io/otel v1.35.0 + go.opentelemetry.io/otel/trace v1.35.0 +) + +require ( + github.com/sirupsen/logrus v1.10.1 // indirect + golang.org/x/sys v0.13.0 // indirect +) diff --git a/otel/go.sum b/otel/go.sum new file mode 100644 index 0000000..a2c71db --- /dev/null +++ b/otel/go.sum @@ -0,0 +1,16 @@ +github.com/containerd/log v0.1.0 h1:TCJt7ioM2cr/tfR8GPbGf9/VRAX8D2B4PjzCpfX540I= +github.com/containerd/log v0.1.0/go.mod h1:VRRf09a7mHDIRezVKTRCrOq78v577GXq3bSa3EhrzVo= +github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= +github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= +github.com/sirupsen/logrus v1.10.1 h1:xi4336Zh11WpU14fXR6I67V3yaTPQYwRx2WEtHbRg4Q= +github.com/sirupsen/logrus v1.10.1/go.mod h1:vsQHnG7xzNsxk3NrwboUiWPnIC3dmbjcGPykD7+tiHk= +github.com/stretchr/testify v1.12.0 h1:K6Mr6jO9JICuend/5xzTM03ydSV3vdNRYAdPSukj8uI= +github.com/stretchr/testify v1.12.0/go.mod h1:bOYBZb5qJ00vPzWfIqBUZPaxK8jWiXc6d3ErP4Ca9Gw= +go.opentelemetry.io/otel v1.35.0 h1:xKWKPxrxB6OtMCbmMY021CqC45J+3Onta9MqjhnusiQ= +go.opentelemetry.io/otel v1.35.0/go.mod h1:UEqy8Zp11hpkUrL73gSlELM0DupHoiq72dR+Zqel/+Y= +go.opentelemetry.io/otel/trace v1.35.0 h1:dPpEfJu1sDIqruz7BHFG3c7528f6ddfSWfFDVt/xgMs= +go.opentelemetry.io/otel/trace v1.35.0/go.mod h1:WUk7DtFp1Aw2MkvqGdwiXYDZZNvA/1J8o6xRXLrIkyc= +golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE= +golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/otel/helpers.go b/otel/helpers.go index ab1278e..6ff259a 100644 --- a/otel/helpers.go +++ b/otel/helpers.go @@ -14,7 +14,7 @@ limitations under the License. */ -package tracing +package otel import ( "encoding/json" diff --git a/otel/log.go b/otel/log.go index b1c3a74..a7fc5bd 100644 --- a/otel/log.go +++ b/otel/log.go @@ -14,7 +14,11 @@ limitations under the License. */ -package tracing +// Package otel provides integration between containerd/log and OpenTelemetry. +// +// In particular, it provides a hook that records log entries as events on +// active OpenTelemetry spans. +package otel import ( "github.com/containerd/log" diff --git a/otel/log_test.go b/otel/log_test.go index ced6d30..b5b2fd8 100644 --- a/otel/log_test.go +++ b/otel/log_test.go @@ -14,14 +14,14 @@ limitations under the License. */ -package tracing +package otel_test import ( "context" "testing" "github.com/containerd/log" - "github.com/stretchr/testify/assert" + "github.com/containerd/log/otel" "go.opentelemetry.io/otel/trace" ) @@ -70,21 +70,27 @@ func TestLogrusHookTraceID(t *testing.T) { ) } - hook := NewLogrusHook(WithTraceIDField(tc.enableOpt)) + hook := otel.NewLogrusHook(otel.WithTraceIDField(tc.enableOpt)) entry := &log.Entry{ Context: ctx, Data: make(log.Fields), } err := hook.Fire(entry) - assert.NoError(t, err) + if err != nil { + t.Fatal(err) + } traceID, ok := entry.Data["trace_id"] if tc.expectedTID != "" { - assert.True(t, ok) - assert.Equal(t, tc.expectedTID, traceID) - } else { - assert.False(t, ok) + if !ok { + t.Fatal(`expected "trace_id" field`) + } + if traceID != tc.expectedTID { + t.Errorf(`"trace_id" = %v; want %q`, traceID, tc.expectedTID) + } + } else if ok { + t.Errorf(`unexpected "trace_id" field: %v`, traceID) } }) } From aee2c9b0de961ba6b7da1a3a51dfbeb8d7719cf9 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 22 Aug 2026 20:50:57 +0200 Subject: [PATCH 10/12] otel: handle error and typed-nil Stringer attributes Log fields which contain an error currently fall through to JSON marshaling, which may produce an unhelpful value such as "{}" instead of the error message. Handle error values explicitly and use fmt.Sprint when formatting error and fmt.Stringer values. Besides preserving their textual representation, fmt handles typed-nil implementations without propagating a panic from their Error or String method. Also simplify the final fmt.Sprintf("%v", v) fallback to fmt.Sprint(v). Signed-off-by: Sebastiaan van Stijn --- otel/helpers.go | 6 ++- otel/helpers_test.go | 88 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 2 deletions(-) create mode 100644 otel/helpers_test.go diff --git a/otel/helpers.go b/otel/helpers.go index 6ff259a..4bf73c8 100644 --- a/otel/helpers.go +++ b/otel/helpers.go @@ -73,13 +73,15 @@ func keyValue(k string, v any) attribute.KeyValue { return attribute.String(k, typed) case []string: return attribute.StringSlice(k, typed) + case error: + return attribute.String(k, fmt.Sprint(typed)) } if stringer, ok := v.(fmt.Stringer); ok { - return attribute.String(k, stringer.String()) + return attribute.String(k, fmt.Sprint(stringer)) } if b, err := json.Marshal(v); b != nil && err == nil { return attribute.String(k, string(b)) } - return attribute.String(k, fmt.Sprintf("%v", v)) + return attribute.String(k, fmt.Sprint(v)) } diff --git a/otel/helpers_test.go b/otel/helpers_test.go new file mode 100644 index 0000000..08265ae --- /dev/null +++ b/otel/helpers_test.go @@ -0,0 +1,88 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package otel + +import ( + "errors" + "testing" + + "go.opentelemetry.io/otel/attribute" +) + +type stringer string + +func (s stringer) String() string { return string(s) } + +type nilStringer struct{} + +func (*nilStringer) String() string { panic("should not panic") } + +type nilError struct{} + +func (*nilError) Error() string { panic("should not panic") } + +func TestKeyValue(t *testing.T) { + tests := []struct { + name string + in any + want attribute.KeyValue + }{ + { + name: "nil", + want: attribute.String("key", ""), + }, + { + name: "string", + in: "value", + want: attribute.String("key", "value"), + }, + { + name: "error", + in: errors.New("error message"), + want: attribute.String("key", "error message"), + }, + { + name: "typed nil error", + in: (*nilError)(nil), + want: attribute.String("key", ""), + }, + { + name: "stringer", + in: stringer("string value"), + want: attribute.String("key", "string value"), + }, + { + name: "typed nil stringer", + in: (*nilStringer)(nil), + want: attribute.String("key", ""), + }, + { + name: "JSON", + in: struct{ Value string }{"foo"}, + want: attribute.String("key", `{"Value":"foo"}`), + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := keyValue("key", tc.in) + if got != tc.want { + t.Errorf("keyValue() = %v; want %v", got, tc.want) + } + }) + } +} From 9abb7ae4221c5fea0ecbca44b8d42817f68cda50 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 23 Aug 2026 12:45:58 +0200 Subject: [PATCH 11/12] otel: add option to configure log level Add WithLevel to configure the minimum log level handled by the OpenTelemetry hook. This allows callers to limit which log entries are recorded as span events, reducing trace noise and overhead when lower-severity logs are not useful for tracing. By default, the hook continues to handle all log levels, preserving the existing behavior. Signed-off-by: Sebastiaan van Stijn --- otel/log.go | 19 +++++++++++++++- otel/log_test.go | 56 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/otel/log.go b/otel/log.go index a7fc5bd..6f08583 100644 --- a/otel/log.go +++ b/otel/log.go @@ -56,6 +56,19 @@ func WithTraceIDField(enabled bool) HookOpt { } } +// WithLevel configures the minimum log level handled by the hook. +// Entries below this level are ignored. +func WithLevel(level log.Level) HookOpt { + return func(h *LogrusHook) { + for i, l := range allLevels { + if l == level { + h.levels = allLevels[:i+1] + return + } + } + } +} + // LogrusHook is a [logrus.Hook] which adds logrus events to active spans. // If the span is not recording or the span context is invalid, the hook // is a no-op. @@ -63,11 +76,15 @@ func WithTraceIDField(enabled bool) HookOpt { // [logrus.Hook]: https://github.com/sirupsen/logrus/blob/v1.9.3/hooks.go#L3-L11 type LogrusHook struct { enableTraceIDField bool + levels []log.Level } // Levels returns the logrus levels that this hook is interested in. func (h *LogrusHook) Levels() []log.Level { - return allLevels + if h.levels == nil { + return allLevels + } + return h.levels } // Fire is called when a log event occurs. diff --git a/otel/log_test.go b/otel/log_test.go index b5b2fd8..a4f5080 100644 --- a/otel/log_test.go +++ b/otel/log_test.go @@ -18,6 +18,7 @@ package otel_test import ( "context" + "slices" "testing" "github.com/containerd/log" @@ -95,3 +96,58 @@ func TestLogrusHookTraceID(t *testing.T) { }) } } + +// TestLogrusHookLevels verifies that [WithLevel] limits the levels handled by +// the hook while preserving all levels by default. +func TestLogrusHookLevels(t *testing.T) { + tests := []struct { + name string + opts []otel.HookOpt + want []log.Level + }{ + { + name: "default", + want: []log.Level{ + log.PanicLevel, + log.FatalLevel, + log.ErrorLevel, + log.WarnLevel, + log.InfoLevel, + log.DebugLevel, + log.TraceLevel, + }, + }, + { + name: "warn", + opts: []otel.HookOpt{ + otel.WithLevel(log.WarnLevel), + }, + want: []log.Level{ + log.PanicLevel, + log.FatalLevel, + log.ErrorLevel, + log.WarnLevel, + }, + }, + { + name: "error", + opts: []otel.HookOpt{ + otel.WithLevel(log.ErrorLevel), + }, + want: []log.Level{ + log.PanicLevel, + log.FatalLevel, + log.ErrorLevel, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + hook := otel.NewLogrusHook(tc.opts...) + if got := hook.Levels(); !slices.Equal(got, tc.want) { + t.Errorf("Levels() = %v; want %v", got, tc.want) + } + }) + } +} From 15f4ac3d51b38c5e4c851af09adbe9d6d29d29ee Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 23 Aug 2026 12:52:36 +0200 Subject: [PATCH 12/12] otel: add option to set span error status Add WithErrorStatusLevel to configure the minimum log level that marks an active span with an error status. This allows callers to reflect sufficiently severe log entries in the span status while keeping the behavior independent from attached error fields. By default, log entries continue to leave the span status unchanged. Signed-off-by: Sebastiaan van Stijn --- otel/log.go | 17 +++++++++ otel/log_test.go | 93 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+) diff --git a/otel/log.go b/otel/log.go index 6f08583..86b7ae6 100644 --- a/otel/log.go +++ b/otel/log.go @@ -23,6 +23,7 @@ package otel import ( "github.com/containerd/log" "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/trace" ) @@ -69,6 +70,14 @@ func WithLevel(level log.Level) HookOpt { } } +// WithErrorStatusLevel configures the minimum log level that marks the +// active span with an error status. +func WithErrorStatusLevel(level log.Level) HookOpt { + return func(h *LogrusHook) { + h.errorStatusLevel = &level + } +} + // LogrusHook is a [logrus.Hook] which adds logrus events to active spans. // If the span is not recording or the span context is invalid, the hook // is a no-op. @@ -76,6 +85,7 @@ func WithLevel(level log.Level) HookOpt { // [logrus.Hook]: https://github.com/sirupsen/logrus/blob/v1.9.3/hooks.go#L3-L11 type LogrusHook struct { enableTraceIDField bool + errorStatusLevel *log.Level levels []log.Level } @@ -113,6 +123,13 @@ func (h *LogrusHook) Fire(entry *log.Entry) error { trace.WithTimestamp(entry.Time), ) + // Set the span status based on the log level, rather than the presence of + // an error field. Error values may be attached to lower-severity log entries + // without indicating that the operation represented by the span failed. + if h.errorStatusLevel != nil && entry.Level <= *h.errorStatusLevel { + span.SetStatus(codes.Error, entry.Message) + } + return nil } diff --git a/otel/log_test.go b/otel/log_test.go index a4f5080..20eb35c 100644 --- a/otel/log_test.go +++ b/otel/log_test.go @@ -18,11 +18,14 @@ package otel_test import ( "context" + "errors" "slices" "testing" + "time" "github.com/containerd/log" "github.com/containerd/log/otel" + "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/trace" ) @@ -33,6 +36,21 @@ var ( testSpanID = trace.SpanID{1, 2, 3, 4, 5, 6, 7, 8} ) +// testSpan is a minimal recording span used to test hook behavior without +// depending on the OpenTelemetry SDK. +type testSpan struct { + trace.Span + status codes.Code +} + +func (s *testSpan) SpanContext() trace.SpanContext { + return trace.NewSpanContext(trace.SpanContextConfig{TraceID: testTraceID, SpanID: testSpanID}) +} + +func (s *testSpan) IsRecording() bool { return true } +func (s *testSpan) AddEvent(string, ...trace.EventOption) {} +func (s *testSpan) SetStatus(code codes.Code, _ string) { s.status = code } + func TestLogrusHookTraceID(t *testing.T) { tests := []struct { name string @@ -151,3 +169,78 @@ func TestLogrusHookLevels(t *testing.T) { }) } } + +// TestLogrusHookErrorStatusLevel verifies that [WithErrorStatusLevel] marks +// spans as errors based on log severity and leaves span status unchanged by +// default. +func TestLogrusHookErrorStatusLevel(t *testing.T) { + tests := []struct { + name string + opts []otel.HookOpt + level log.Level + fields log.Fields + wantError bool + }{ + { + name: "default", + level: log.ErrorLevel, + }, + { + name: "below threshold", + opts: []otel.HookOpt{ + otel.WithErrorStatusLevel(log.ErrorLevel), + }, + level: log.WarnLevel, + }, + { + name: "at threshold", + opts: []otel.HookOpt{ + otel.WithErrorStatusLevel(log.ErrorLevel), + }, + level: log.ErrorLevel, + wantError: true, + }, + { + name: "above threshold", + opts: []otel.HookOpt{ + otel.WithErrorStatusLevel(log.ErrorLevel), + }, + level: log.FatalLevel, + wantError: true, + }, + { + name: "error field below threshold", + opts: []otel.HookOpt{ + otel.WithErrorStatusLevel(log.ErrorLevel), + }, + level: log.DebugLevel, + fields: log.Fields{ + "error": errors.New("ignored"), + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + span := &testSpan{} + ctx := trace.ContextWithSpan(context.Background(), span) + + hook := otel.NewLogrusHook(tc.opts...) + err := hook.Fire(&log.Entry{ + Context: ctx, + Data: tc.fields, + Level: tc.level, + Message: "message", + Time: time.Now(), + }) + if err != nil { + t.Fatal(err) + } + + gotError := span.status == codes.Error + if gotError != tc.wantError { + t.Errorf("span error status = %v; want %v", gotError, tc.wantError) + } + }) + } +}