From e2e09b384a98b918979bb8544e104f2fc2f3ea34 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 14 Jun 2024 11:32:19 +0200 Subject: [PATCH 1/4] pkg/tracing: rename func that shadowed builtin, rm makeSpanName Signed-off-by: Sebastiaan van Stijn --- pkg/tracing/helpers.go | 11 +---------- pkg/tracing/log.go | 2 +- pkg/tracing/tracing.go | 9 ++++++--- 3 files changed, 8 insertions(+), 14 deletions(-) diff --git a/pkg/tracing/helpers.go b/pkg/tracing/helpers.go index 981da6c79..ab1278ef1 100644 --- a/pkg/tracing/helpers.go +++ b/pkg/tracing/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/pkg/tracing/log.go b/pkg/tracing/log.go index 98fa16f93..5834d03b0 100644 --- a/pkg/tracing/log.go +++ b/pkg/tracing/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 } diff --git a/pkg/tracing/tracing.go b/pkg/tracing/tracing.go index 1d2738606..c6c6b1264 100644 --- a/pkg/tracing/tracing.go +++ b/pkg/tracing/tracing.go @@ -19,6 +19,7 @@ package tracing import ( "context" "net/http" + "strings" "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" "go.opentelemetry.io/otel" @@ -99,14 +100,16 @@ func (s *Span) SetAttributes(kv ...attribute.KeyValue) { s.otelSpan.SetAttributes(kv...) } +const spanDelimiter = "." + // Name sets the span name by joining a list of strings in dot separated format. func Name(names ...string) string { - return makeSpanName(names...) + return strings.Join(names, spanDelimiter) } // Attribute takes a key value pair and returns attribute.KeyValue type. -func Attribute(k string, v interface{}) attribute.KeyValue { - return any(k, v) +func Attribute(k string, v any) attribute.KeyValue { + return keyValue(k, v) } // HTTPStatusCodeAttributes generates attributes of the HTTP namespace as specified by the OpenTelemetry From 4203e2de8d30a5f0bd5b3d030562db9e37018119 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 14 Jun 2024 11:33:46 +0200 Subject: [PATCH 2/4] pkg/tracing/plugin: rename var that collided with import Signed-off-by: Sebastiaan van Stijn --- pkg/tracing/plugin/otlp.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/tracing/plugin/otlp.go b/pkg/tracing/plugin/otlp.go index 52586913e..04303eff0 100644 --- a/pkg/tracing/plugin/otlp.go +++ b/pkg/tracing/plugin/otlp.go @@ -96,14 +96,14 @@ func init() { return nil, err } - //get TracingProcessorPlugin which is a dependency - plugins, err := ic.GetByType(plugins.TracingProcessorPlugin) + // get TracingProcessorPlugin which is a dependency + tracingProcessors, err := ic.GetByType(plugins.TracingProcessorPlugin) if err != nil { return nil, fmt.Errorf("failed to get tracing processors: %w", err) } - procs := make([]trace.SpanProcessor, 0, len(plugins)) - for _, p := range plugins { + procs := make([]trace.SpanProcessor, 0, len(tracingProcessors)) + for _, p := range tracingProcessors { procs = append(procs, p.(trace.SpanProcessor)) } From ccf7938126e0d7a52b3ef8a1d45468732ba866de Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 14 Jun 2024 12:08:51 +0200 Subject: [PATCH 3/4] 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 --- pkg/tracing/log.go | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/pkg/tracing/log.go b/pkg/tracing/log.go index 5834d03b0..f2604cd60 100644 --- a/pkg/tracing/log.go +++ b/pkg/tracing/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 587ee80f61de6ea368cfe1eaa02f67e85d0fa3f7 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 14 Jun 2024 12:12:22 +0200 Subject: [PATCH 4/4] 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 --- pkg/tracing/log.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/tracing/log.go b/pkg/tracing/log.go index f2604cd60..3af24a294 100644 --- a/pkg/tracing/log.go +++ b/pkg/tracing/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 }