From 9389666230fc6c5c83eb1d0708663d2c11b97e15 Mon Sep 17 00:00:00 2001 From: Guangya Liu Date: Mon, 31 Aug 2026 12:31:50 -0400 Subject: [PATCH] tracing: support OTEL_TRACES_EXPORTER=none|otlp|console Signed-off-by: Guangya Liu --- pkg/common/observability/tracing/telemetry.go | 83 +++++++---- .../observability/tracing/telemetry_test.go | 131 ++++++++++++++++++ 2 files changed, 190 insertions(+), 24 deletions(-) create mode 100644 pkg/common/observability/tracing/telemetry_test.go diff --git a/pkg/common/observability/tracing/telemetry.go b/pkg/common/observability/tracing/telemetry.go index fe41e150..e1bcdca2 100644 --- a/pkg/common/observability/tracing/telemetry.go +++ b/pkg/common/observability/tracing/telemetry.go @@ -52,17 +52,13 @@ func InitTracing(ctx context.Context, logger logr.Logger, defaultServiceName str os.Setenv("OTEL_SERVICE_NAME", defaultServiceName) } - _, ok = os.LookupEnv("OTEL_EXPORTER_OTLP_ENDPOINT") - if !ok { - os.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://localhost:4317") - } - - traceExporter, err := initTraceExporter(ctx, logger) + exporterType, err := traceExporterType() if err != nil { - loggerWrap.Handle(fmt.Errorf("%s: %v", "init trace exporter failed", err)) - return err + loggerWrap.Handle(fmt.Errorf("trace exporter configuration degraded: %w", err)) } + logger.Info("init OTel trace exporter", "type", exporterType) + // Go SDK doesn't have an automatic sampler, handle manually samplerType, ok := os.LookupEnv("OTEL_TRACES_SAMPLER") if !ok { @@ -86,7 +82,6 @@ func InitTracing(ctx context.Context, logger logr.Logger, defaultServiceName str } opt := []sdktrace.TracerProviderOption{ - sdktrace.WithBatcher(traceExporter), sdktrace.WithSampler(sampler), sdktrace.WithResource(resource.NewWithAttributes( semconv.SchemaURL, @@ -94,6 +89,17 @@ func InitTracing(ctx context.Context, logger logr.Logger, defaultServiceName str )), } + // "none" registers no span processor at all. Spans are still created and + // propagated, so instrumented code and context propagation are unaffected. + if exporterType != exporterTypeNone { + traceExporter, err := newTraceExporter(ctx, exporterType) + if err != nil { + loggerWrap.Handle(fmt.Errorf("%s: %v", "init trace exporter failed", err)) + return err + } + opt = append(opt, sdktrace.WithBatcher(traceExporter)) + } + tracerProvider := sdktrace.NewTracerProvider(opt...) otel.SetTracerProvider(tracerProvider) otel.SetTextMapPropagator(propagation.NewCompositeTextMapPropagator(propagation.TraceContext{}, propagation.Baggage{})) @@ -112,29 +118,58 @@ func InitTracing(ctx context.Context, logger logr.Logger, defaultServiceName str return nil } -// initTraceExporter create a SpanExporter -// support exporter type -// - console: export spans in console for development use case -// - otlp: export spans through gRPC to an opentelemetry collector -func initTraceExporter(ctx context.Context, logger logr.Logger) (sdktrace.SpanExporter, error) { - var traceExporter sdktrace.SpanExporter - traceExporter, err := stdouttrace.New(stdouttrace.WithPrettyPrint()) - if err != nil { - return nil, fmt.Errorf("failed to create stdouttrace exporter: %w", err) - } +// The exporter types OTEL_TRACES_EXPORTER selects between. +const ( + exporterTypeOTLP = "otlp" + exporterTypeConsole = "console" + exporterTypeNone = "none" + defaultExporterType = exporterTypeOTLP +) + +// traceExporterType resolves OTEL_TRACES_EXPORTER to one of the types +// newTraceExporter builds: +// +// - otlp: export spans through gRPC to an opentelemetry collector +// - console: pretty print spans on stdout, for development +// - none: create spans but export nothing +// +// An unrecognised value is returned as an error alongside the default type, so a +// typo is reported rather than quietly selecting an exporter the operator did not +// ask for. The exporter is not worth failing startup over. +func traceExporterType() (string, error) { exporterType, ok := os.LookupEnv("OTEL_TRACES_EXPORTER") if !ok { - exporterType = "console" + return defaultExporterType, nil } - logger.Info("init OTel trace exporter", "type", exporterType) - if exporterType == "otlp" { - traceExporter, err = otlptracegrpc.New(ctx, otlptracegrpc.WithInsecure()) + switch exporterType { + case exporterTypeOTLP, exporterTypeConsole, exporterTypeNone: + return exporterType, nil + default: + return defaultExporterType, fmt.Errorf("unsupported OTEL_TRACES_EXPORTER %q, falling back to %s", exporterType, defaultExporterType) + } +} + +// newTraceExporter builds the exporter named by exporterType, which traceExporterType +// has already narrowed. Exactly one exporter is constructed; exporterTypeNone builds +// none and is handled by the caller. +func newTraceExporter(ctx context.Context, exporterType string) (sdktrace.SpanExporter, error) { + if exporterType == exporterTypeConsole { + traceExporter, err := stdouttrace.New(stdouttrace.WithPrettyPrint()) if err != nil { - return nil, fmt.Errorf("failed to create otlp-grcp exporter: %w", err) + return nil, fmt.Errorf("failed to create stdouttrace exporter: %w", err) } + return traceExporter, nil } + if _, ok := os.LookupEnv("OTEL_EXPORTER_OTLP_ENDPOINT"); !ok { + os.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://localhost:4317") + } + + traceExporter, err := otlptracegrpc.New(ctx, otlptracegrpc.WithInsecure()) + if err != nil { + return nil, fmt.Errorf("failed to create otlp-grpc exporter: %w", err) + } return traceExporter, nil } diff --git a/pkg/common/observability/tracing/telemetry_test.go b/pkg/common/observability/tracing/telemetry_test.go new file mode 100644 index 00000000..638cb569 --- /dev/null +++ b/pkg/common/observability/tracing/telemetry_test.go @@ -0,0 +1,131 @@ +/* +Copyright 2026 The llm-d 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" + "fmt" + "os" + "testing" +) + +func clearEnv(t *testing.T, keys ...string) { + t.Helper() + for _, key := range keys { + if _, ok := os.LookupEnv(key); ok { + orig := os.Getenv(key) + if err := os.Unsetenv(key); err != nil { + t.Fatalf("Unsetenv(%q) error = %v", key, err) + } + t.Cleanup(func() { os.Setenv(key, orig) }) + } + } +} + +func ptr(s string) *string { return &s } + +func TestTraceExporterType(t *testing.T) { + tests := []struct { + name string + env *string + want string + wantErr bool + }{ + {name: "unset defaults to otlp", env: nil, want: "otlp"}, + {name: "otlp", env: ptr("otlp"), want: "otlp"}, + {name: "console", env: ptr("console"), want: "console"}, + {name: "none", env: ptr("none"), want: "none"}, + {name: "an unrecognised value is reported", env: ptr("jaeger"), want: "otlp", wantErr: true}, + {name: "an empty value is reported rather than treated as unset", env: ptr(""), want: "otlp", wantErr: true}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + clearEnv(t, "OTEL_TRACES_EXPORTER") + if tc.env != nil { + t.Setenv("OTEL_TRACES_EXPORTER", *tc.env) + } + + got, err := traceExporterType() + if (err != nil) != tc.wantErr { + t.Fatalf("traceExporterType() error = %v, wantErr %v", err, tc.wantErr) + } + if got != tc.want { + t.Errorf("traceExporterType() = %q, want %q", got, tc.want) + } + }) + } +} + +// newTraceExporter must build exactly the exporter it was asked for. The stdout +// exporter in particular must not be constructed for the otlp type. +func TestNewTraceExporter(t *testing.T) { + clearEnv(t, "OTEL_EXPORTER_OTLP_ENDPOINT") + + tests := []struct { + exporterType string + wantType string + }{ + {exporterType: exporterTypeOTLP, wantType: "*otlptrace.Exporter"}, + {exporterType: exporterTypeConsole, wantType: "*stdouttrace.Exporter"}, + } + + for _, tc := range tests { + t.Run(tc.exporterType, func(t *testing.T) { + exporter, err := newTraceExporter(context.Background(), tc.exporterType) + if err != nil { + t.Fatalf("newTraceExporter(%q) error = %v", tc.exporterType, err) + } + t.Cleanup(func() { _ = exporter.Shutdown(context.Background()) }) + + if got := fmt.Sprintf("%T", exporter); got != tc.wantType { + t.Errorf("newTraceExporter(%q) = %s, want %s", tc.exporterType, got, tc.wantType) + } + }) + } +} + +// The otlp exporter falls back to a loopback endpoint only when the operator has +// not configured one; "none" and "console" must not touch the environment at all. +func TestNewTraceExporterOTLPDefaultsEndpointOnlyWhenUnset(t *testing.T) { + clearEnv(t, "OTEL_EXPORTER_OTLP_ENDPOINT") + t.Cleanup(func() { os.Unsetenv("OTEL_EXPORTER_OTLP_ENDPOINT") }) + + exporter, err := newTraceExporter(context.Background(), exporterTypeOTLP) + if err != nil { + t.Fatalf("newTraceExporter(otlp) error = %v", err) + } + t.Cleanup(func() { _ = exporter.Shutdown(context.Background()) }) + + if got := os.Getenv("OTEL_EXPORTER_OTLP_ENDPOINT"); got != "http://localhost:4317" { + t.Errorf("OTEL_EXPORTER_OTLP_ENDPOINT = %q, want the loopback default", got) + } +} + +func TestNewTraceExporterConsoleDoesNotSetEndpoint(t *testing.T) { + clearEnv(t, "OTEL_EXPORTER_OTLP_ENDPOINT") + + exporter, err := newTraceExporter(context.Background(), exporterTypeConsole) + if err != nil { + t.Fatalf("newTraceExporter(console) error = %v", err) + } + t.Cleanup(func() { _ = exporter.Shutdown(context.Background()) }) + + if _, ok := os.LookupEnv("OTEL_EXPORTER_OTLP_ENDPOINT"); ok { + t.Error("OTEL_EXPORTER_OTLP_ENDPOINT was set for the console exporter, want it left untouched") + } +}