From cb75dab1f631394a99308c354b272c4520454d4e Mon Sep 17 00:00:00 2001 From: Florian Forster Date: Fri, 10 Oct 2025 08:21:09 +0200 Subject: [PATCH 1/2] fix(tracing): Add the `service.name` default attribute in the Stackdriver tracer. The `service.name` attribute is used by the GCP Trace Explorer to allow filtering by service. Users would reasonably expect that when they use the `WithServiceName` option, that this attribute would be set in traces. --- tracing/impl/stackdriver_tracer.go | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/tracing/impl/stackdriver_tracer.go b/tracing/impl/stackdriver_tracer.go index b5266fc..5f1ceb6 100644 --- a/tracing/impl/stackdriver_tracer.go +++ b/tracing/impl/stackdriver_tracer.go @@ -19,6 +19,10 @@ import ( "go.opencensus.io/trace/propagation" ) +// ServiceNameKey is the attribute name used to configure the service name. +// Follows OpenTelemetry convention. +const ServiceNameKey = "service.name" + // to play nice with grpc_opentracing tagsCarrier // https://github.com/grpc-ecosystem/go-grpc-middleware/blob/a77ba4df9c270ec918ed6a6d506309078e3e4c4d/tracing/opentracing/id_extract.go#L30 // https://github.com/grpc-ecosystem/go-grpc-middleware/blob/a77ba4df9c270ec918ed6a6d506309078e3e4c4d/tracing/opentracing/options.go#L48 @@ -272,8 +276,9 @@ func (c stackdriverCloser) Close() error { } type stackdriverConfig struct { - options *stackdriver.Options - sampler trace.Sampler + serviceName string + options *stackdriver.Options + sampler trace.Sampler } // https://pkg.go.dev/contrib.go.opencensus.io/exporter/stackdriver#Options @@ -282,6 +287,10 @@ var stackdriverConfigMapper = map[string]func(traceCfg *stackdriverConfig, value stackdriverCfg.options.ProjectID = value return nil }, + "service_name": func(stackdriverCfg *stackdriverConfig, value string) error { + stackdriverCfg.serviceName = value + return nil + }, "location": func(stackdriverCfg *stackdriverConfig, value string) error { stackdriverCfg.options.Location = value return nil @@ -354,6 +363,15 @@ func stackdriverTracerFactory(config map[string]string) (opentracing.Tracer, io. } } + // If serviceName is configured in options, add it to the default trace attributes. + if stackdriverCfg.serviceName != "" { + if stackdriverCfg.options.DefaultTraceAttributes == nil { + stackdriverCfg.options.DefaultTraceAttributes = make(map[string]any) + } + + stackdriverCfg.options.DefaultTraceAttributes[ServiceNameKey] = stackdriverCfg.serviceName + } + exporter, err := stackdriver.NewExporter(*stackdriverCfg.options) if err != nil { return nil, nil, err -- GitLab From 6c1b6a4ece71f5b97d3ce0043ff2a563a3145be3 Mon Sep 17 00:00:00 2001 From: Florian Forster Date: Fri, 10 Oct 2025 08:56:25 +0200 Subject: [PATCH 2/2] test(tracing): Add test for Stackdriver config handling. --- tracing/impl/stackdriver_tracer.go | 13 +- tracing/impl/stackdriver_tracer_test.go | 242 +++++++++++++++++++++++- 2 files changed, 251 insertions(+), 4 deletions(-) diff --git a/tracing/impl/stackdriver_tracer.go b/tracing/impl/stackdriver_tracer.go index 5f1ceb6..c705ba4 100644 --- a/tracing/impl/stackdriver_tracer.go +++ b/tracing/impl/stackdriver_tracer.go @@ -345,7 +345,7 @@ var stackdriverConfigMapper = map[string]func(traceCfg *stackdriverConfig, value }, } -func stackdriverTracerFactory(config map[string]string) (opentracing.Tracer, io.Closer, error) { +func newStackdriverConfig(config map[string]string) (*stackdriverConfig, error) { stackdriverCfg := &stackdriverConfig{ options: &stackdriver.Options{}, sampler: trace.NeverSample(), @@ -356,7 +356,7 @@ func stackdriverTracerFactory(config map[string]string) (opentracing.Tracer, io. if mapper != nil { err := mapper(stackdriverCfg, v) if err != nil { - return nil, nil, err + return nil, err } } else { log.Printf("stackdriver tracer: warning: ignoring unknown configuration option: %s", k) @@ -372,6 +372,15 @@ func stackdriverTracerFactory(config map[string]string) (opentracing.Tracer, io. stackdriverCfg.options.DefaultTraceAttributes[ServiceNameKey] = stackdriverCfg.serviceName } + return stackdriverCfg, nil +} + +func stackdriverTracerFactory(config map[string]string) (opentracing.Tracer, io.Closer, error) { + stackdriverCfg, err := newStackdriverConfig(config) + if err != nil { + return nil, nil, err + } + exporter, err := stackdriver.NewExporter(*stackdriverCfg.options) if err != nil { return nil, nil, err diff --git a/tracing/impl/stackdriver_tracer_test.go b/tracing/impl/stackdriver_tracer_test.go index 3be26bd..6cb9dc3 100644 --- a/tracing/impl/stackdriver_tracer_test.go +++ b/tracing/impl/stackdriver_tracer_test.go @@ -7,8 +7,11 @@ import ( "reflect" "strings" "testing" + "time" + "contrib.go.opencensus.io/exporter/stackdriver" "github.com/opentracing/opentracing-go" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.opencensus.io/trace" @@ -20,7 +23,7 @@ func TestOcSpanAdapterCastToAttribute(t *testing.T) { tests := []struct { name string key string - value interface{} + value any want []trace.Attribute }{ { @@ -158,10 +161,245 @@ func TestIsSampled_stackdriver(t *testing.T) { } span := stackdriverTracer.StartSpan("rootSpan") - for i := 0; i < 10; i++ { + for i := range 10 { require.Equal(t, tc.sampled, IsSampled(span)) span = stackdriverTracer.StartSpan(fmt.Sprintf("span%d", i), opentracing.ChildOf(span.Context())) } }) } } + +func TestNewStackdriverConfig(t *testing.T) { + cases := []struct { + name string + config map[string]string + want *stackdriverConfig + wantErr string + }{ + { + name: "with project ID", + config: map[string]string{ + "project_id": "unittest", + }, + want: &stackdriverConfig{ + options: &stackdriver.Options{ + ProjectID: "unittest", + }, + }, + }, + { + name: "with service name", + config: map[string]string{ + "service_name": "unittest", + }, + want: &stackdriverConfig{ + serviceName: "unittest", + options: &stackdriver.Options{ + DefaultTraceAttributes: map[string]any{ + ServiceNameKey: "unittest", + }, + }, + }, + }, + { + name: "with location", + config: map[string]string{ + "location": "us-central1", + }, + want: &stackdriverConfig{ + options: &stackdriver.Options{ + Location: "us-central1", + }, + }, + }, + { + name: "with bundle delay threshold", + config: map[string]string{ + "bundle_delay_threshold": "5s", + }, + want: &stackdriverConfig{ + options: &stackdriver.Options{ + BundleDelayThreshold: 5 * time.Second, + }, + }, + }, + { + name: "with invalid bundle delay threshold", + config: map[string]string{ + "bundle_delay_threshold": "invalid", + }, + wantErr: "time: invalid duration", + }, + { + name: "with bundle count threshold", + config: map[string]string{ + "bundle_count_threshold": "100", + }, + want: &stackdriverConfig{ + options: &stackdriver.Options{ + BundleCountThreshold: 100, + }, + }, + }, + { + name: "with invalid bundle count threshold", + config: map[string]string{ + "bundle_count_threshold": "not-a-number", + }, + wantErr: "not-a-number", + }, + { + name: "with trace spans buffer max bytes", + config: map[string]string{ + "trace_spans_buffer_max_bytes": "1048576", + }, + want: &stackdriverConfig{ + options: &stackdriver.Options{ + TraceSpansBufferMaxBytes: 1048576, + }, + }, + }, + { + name: "with invalid trace spans buffer max bytes", + config: map[string]string{ + "trace_spans_buffer_max_bytes": "not-a-number", + }, + wantErr: "not-a-number", + }, + { + name: "with timeout", + config: map[string]string{ + "timeout": "30s", + }, + want: &stackdriverConfig{ + options: &stackdriver.Options{ + Timeout: 30 * time.Second, + }, + }, + }, + { + name: "with invalid timeout", + config: map[string]string{ + "timeout": "not-a-duration", + }, + wantErr: "time: invalid duration", + }, + { + name: "with number of workers", + config: map[string]string{ + "number_of_workers": "10", + }, + want: &stackdriverConfig{ + options: &stackdriver.Options{ + NumberOfWorkers: 10, + }, + }, + }, + { + name: "with invalid number of workers", + config: map[string]string{ + "number_of_workers": "not-a-number", + }, + wantErr: "not-a-number", + }, + { + name: "with sampler probability", + config: map[string]string{ + "sampler_probability": "0.5", + }, + want: &stackdriverConfig{ + options: &stackdriver.Options{}, + }, + }, + { + name: "with invalid sampler probability", + config: map[string]string{ + "sampler_probability": "not-a-float", + }, + wantErr: "not-a-float", + }, + { + name: "with all valid options", + config: map[string]string{ + "project_id": "test-project", + "service_name": "test-service", + "location": "us-west1", + "bundle_delay_threshold": "10s", + "bundle_count_threshold": "50", + "trace_spans_buffer_max_bytes": "2097152", + "timeout": "1m", + "number_of_workers": "5", + "sampler_probability": "0.75", + }, + want: &stackdriverConfig{ + serviceName: "test-service", + options: &stackdriver.Options{ + ProjectID: "test-project", + Location: "us-west1", + BundleDelayThreshold: 10 * time.Second, + BundleCountThreshold: 50, + TraceSpansBufferMaxBytes: 2097152, + Timeout: time.Minute, + NumberOfWorkers: 5, + DefaultTraceAttributes: map[string]any{ + ServiceNameKey: "test-service", + }, + }, + }, + }, + { + name: "with unknown option (should be ignored with log)", + config: map[string]string{ + "project_id": "test", + "unknown_option": "value", + }, + want: &stackdriverConfig{ + options: &stackdriver.Options{ + ProjectID: "test", + }, + }, + }, + { + name: "empty config", + config: map[string]string{}, + want: &stackdriverConfig{ + options: &stackdriver.Options{}, + }, + }, + { + name: "service name with existing default attributes", + config: map[string]string{ + "service_name": "test-service", + "project_id": "test-project", + }, + want: &stackdriverConfig{ + serviceName: "test-service", + options: &stackdriver.Options{ + ProjectID: "test-project", + DefaultTraceAttributes: map[string]any{ + ServiceNameKey: "test-service", + }, + }, + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, err := newStackdriverConfig(tc.config) + + if tc.wantErr != "" { + require.ErrorContains(t, err, tc.wantErr) + return + } + + require.NoError(t, err) + + // trace.Sampler is a function pointer and therefore not something we can compare. + assert.NotNil(t, got.sampler) + got.sampler = nil + + require.Equal(t, tc.want, got) + }) + } +} -- GitLab