tracing: remove deprecated tracing config options

Remove support for service_name and sampling_ratio in the
io.containerd.internal.v1.tracing plugin configuration, which was
previously deprecated in v1.6.29. Users should use standard OTel
environment variables instead (OTEL_SERVICE_NAME, OTEL_TRACES_SAMPLER,
etc.).

Re-add unit tests in pkg/tracing/plugin/otlp_test.go to verify
OTel environment variable configuration for service name, sampling,
exporters, and SDK disabling.

Assisted-by: Antigravity
Signed-off-by: Samuel Karp <samuelkarp@google.com>

RELEASES.md

@@ -637,7 +637,7 @@ The deprecated properties in [`config.toml`](./docs/cri/config.md) are shown in
 |`[plugins."io.containerd.grpc.v1.cri".registry]`                      | `mirrors`                    | containerd v1.5     | containerd v2.7            | Use [`config_path`](./docs/hosts.md)                 |
 |`[plugins."io.containerd.tracing.processor.v1.otlp"]`                 | `endpoint`, `protocol`, `insecure` | containerd v1.6.29 | containerd v2.4 ✅    | Use [OTLP environment variables](https://opentelemetry.io/docs/specs/otel/protocol/exporter/), e.g. OTEL_EXPORTER_OTLP_TRACES_ENDPOINT, OTEL_EXPORTER_OTLP_PROTOCOL, OTEL_SDK_DISABLED    |
 |`[plugins."io.containerd.cri.v1.runtime".cni]`                        | `bin_dir`                    | containerd v2.1     | containerd v2.4 ✅         | Use `bin_dirs`, which supports a list of directories |
-|`[plugins."io.containerd.internal.v1.tracing"]`                       | `service_name`, `sampling_ratio`   | containerd v1.6.29 | containerd v2.4       | Instead use [OTel environment variables](https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/), e.g. OTEL_SERVICE_NAME, OTEL_TRACES_SAMPLER*  |
+|`[plugins."io.containerd.internal.v1.tracing"]`                       | `service_name`, `sampling_ratio`   | containerd v1.6.29 | containerd v2.4 ✅    | Instead use [OTel environment variables](https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/), e.g. OTEL_SERVICE_NAME, OTEL_TRACES_SAMPLER*  |
 |`[plugins."io.containerd.cri.v1.runtime"]`                            | `enable_cdi`                 | containerd v2.2     | containerd v2.4 ✅         | CDI support will always be enabled                   |
 
 

pkg/deprecation/deprecation.go

@@ -27,8 +27,6 @@ const (
 	CRIRegistryAuths Warning = Prefix + "cri-registry-auths"
 	// CRIRegistryConfigs is a warning for the use of the `configs` property
 	CRIRegistryConfigs Warning = Prefix + "cri-registry-configs"
-	// TracingServiceConfig is a warning for the use of the `tracing` property
-	TracingServiceConfig Warning = Prefix + "tracing-service-config"
 	// NRIV010Plugin is a warning for the use of NRI 0.1.0-style plugins
 	NRIV010Plugin Warning = Prefix + "nri-v010-plugin"
 	// NRIPluginInterface is a warning for the use of a deprecated NRI interface.
@@ -53,8 +51,6 @@ var messages = map[Warning]string{
 	CRIRegistryConfigs: "The `configs` property of `[plugins.\"io.containerd.grpc.v1.cri\".registry]` is deprecated since containerd v1.5 and will be removed in containerd v2.7. " +
 		"Use `config_path` instead.",
 
-	TracingServiceConfig: "The `tracing` property of `[plugins.\"io.containerd.internal.v1\".tracing]` is deprecated since containerd v1.6 and will be removed in containerd v2.4. " +
-		"Use OTEL environment variables instead: https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/",
 	NRIV010Plugin:      "NRI 0.1.0-style plugins are deprecated since containerd 2.2 and should only be used through the v010-adapter plugin.",
 	NRIPluginInterface: "NRI plugin uses a deprecated interface.",
 	CgroupV1:           "The support for cgroup v1 is deprecated since containerd v2.2 and will be removed by no later than May 2029. Upgrade the host to use cgroup v2.",

pkg/tracing/plugin/otlp.go

@@ -24,9 +24,7 @@ import (
 	"strconv"
 	"time"
 
-	"github.com/containerd/containerd/v2/pkg/deprecation"
 	"github.com/containerd/containerd/v2/plugins"
-	"github.com/containerd/containerd/v2/plugins/services/warning"
 	"github.com/containerd/errdefs"
 	"github.com/containerd/plugin"
 	"github.com/containerd/plugin/registry"
@@ -76,16 +74,12 @@ func init() {
 		},
 	})
 	registry.Register(&plugin.Registration{
-		ID:     "tracing",
-		Type:   plugins.InternalPlugin,
-		Config: &TraceConfig{},
+		ID:   "tracing",
+		Type: plugins.InternalPlugin,
 		Requires: []plugin.Type{
 			plugins.TracingProcessorPlugin,
 		},
 		InitFn: func(ic *plugin.InitContext) (any, error) {
-			if err := warnTraceConfig(ic); err != nil {
-				return nil, err
-			}
 			if err := checkDisabled(); err != nil {
 				return nil, err
 			}
@@ -106,12 +100,6 @@ func init() {
 	})
 }
 
-// TraceConfig is the common configuration for open telemetry.
-type TraceConfig struct {
-	ServiceName        string  `toml:"service_name,omitempty"`
-	TraceSamplingRatio float64 `toml:"sampling_ratio,omitempty"`
-}
-
 func checkDisabled() error {
 	v := os.Getenv(sdkDisabledEnv)
 	if v != "" {
@@ -190,30 +178,3 @@ func newTracer(ctx context.Context, procs []trace.SpanProcessor) (io.Closer, err
 	}), nil
 
 }
-
-func warnTraceConfig(ic *plugin.InitContext) error {
-	if ic.Config == nil {
-		return nil
-	}
-	ctx := ic.Context
-	cfg := ic.Config.(*TraceConfig)
-	var warn bool
-	if cfg.ServiceName != "" {
-		warn = true
-	}
-	if cfg.TraceSamplingRatio != 0 {
-		warn = true
-	}
-
-	if !warn {
-		return nil
-	}
-
-	wp, err := ic.GetSingle(plugins.WarningPlugin)
-	if err != nil {
-		return err
-	}
-	ws := wp.(warning.Service)
-	ws.Emit(ctx, deprecation.TracingServiceConfig)
-	return nil
-}

pkg/tracing/plugin/otlp_test.go

@@ -19,66 +19,192 @@ package plugin
 import (
 	"context"
 	"errors"
+	"os"
+	"os/exec"
+	"strings"
 	"testing"
 
 	"github.com/containerd/errdefs"
 	"github.com/containerd/plugin"
+	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/sdk/trace"
 	"go.opentelemetry.io/otel/sdk/trace/tracetest"
 )
 
 func TestNewExporter(t *testing.T) {
 	for _, tc := range []struct {
-		name           string
-		protocol       string
-		tracesProtocol string
-		output         error
+		name          string
+		protocol      string
+		traceProtocol string
+		expectedErr   error
 	}{
 		{
-			name:     "Test http/protobuf protocol, expect no error",
-			protocol: "http/protobuf",
-			output:   nil,
+			name:        "default protocol",
+			expectedErr: nil,
 		},
 		{
-			name:     "Test default protocol, expect no error",
-			protocol: "",
-			output:   nil,
+			name:        "http/protobuf protocol",
+			protocol:    "http/protobuf",
+			expectedErr: nil,
 		},
 		{
-			name:     "Test grpc protocol, expect no error",
-			protocol: "grpc",
-			output:   nil,
+			name:        "grpc protocol",
+			protocol:    "grpc",
+			expectedErr: nil,
 		},
 		{
-			name:     "Test http/json protocol which is not supported, expect not implemented error",
-			protocol: "http/json",
-			output:   errdefs.ErrNotImplemented,
+			name:          "traces protocol overrides generic protocol",
+			protocol:      "http/protobuf",
+			traceProtocol: "grpc",
+			expectedErr:   nil,
 		},
 		{
-			name:           "Test traces protocol takes precedence over protocol",
-			protocol:       "http/json",
-			tracesProtocol: "http/protobuf",
-			output:         nil,
+			name:        "unsupported protocol http/json",
+			protocol:    "http/json",
+			expectedErr: errdefs.ErrNotImplemented,
 		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Setenv(otlpProtocolEnv, tc.protocol)
-			t.Setenv(otlpTracesProtocolEnv, tc.tracesProtocol)
+			t.Setenv(otlpTracesProtocolEnv, tc.traceProtocol)
 
-			ctx := context.TODO()
+			ctx := context.Background()
 			exp, err := newExporter(ctx)
-			if tc.output == nil {
-				if err != nil {
-					t.Fatalf("unexpected error: %v", err)
+			if tc.expectedErr != nil {
+				if !errors.Is(err, tc.expectedErr) {
+					t.Fatalf("expected error %v, got %v", tc.expectedErr, err)
 				}
-				if exp == nil {
-					t.Fatal("expected exporter to be created, got nil")
-				}
-			} else {
-				if !errors.Is(err, tc.output) {
-					t.Fatalf("expected error %v, got %v", tc.output, err)
+				return
+			}
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			if exp == nil {
+				t.Fatal("expected exporter, got nil")
+			}
+		})
+	}
+}
+
+func TestNewTracerServiceName(t *testing.T) {
+	if os.Getenv("TEST_SERVICE_NAME_HELPER") == "1" {
+		expectedService := os.Getenv("EXPECTED_SERVICE_NAME")
+		rec := tracetest.NewSpanRecorder()
+		closer, err := newTracer(context.Background(), []trace.SpanProcessor{rec})
+		if err != nil {
+			t.Fatalf("unexpected error creating tracer: %v", err)
+		}
+		defer closer.Close()
+
+		ctx := context.Background()
+		tracer := otel.GetTracerProvider().Tracer("test-tracer")
+		_, span := tracer.Start(ctx, "test-span")
+		span.End()
+
+		ended := rec.Ended()
+		if len(ended) != 1 {
+			t.Fatalf("expected 1 ended span, got %d", len(ended))
+		}
+
+		val, ok := ended[0].Resource().Set().Value("service.name")
+		if !ok {
+			t.Fatalf("expected service.name attribute on resource, but found none")
+		}
+		svc := val.AsString()
+		if svc != expectedService {
+			t.Fatalf("expected service name %q, got %q", expectedService, svc)
+		}
+		return
+	}
+
+	for _, tc := range []struct {
+		name            string
+		envServiceName  string
+		expectedService string
+	}{
+		{
+			name:            "default service name",
+			envServiceName:  "",
+			expectedService: "containerd",
+		},
+		{
+			name:            "custom service name",
+			envServiceName:  "custom-containerd",
+			expectedService: "custom-containerd",
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			cmd := exec.Command(os.Args[0], "-test.run=^TestNewTracerServiceName$")
+			var env []string
+			for _, e := range os.Environ() {
+				if !strings.HasPrefix(e, "OTEL_SERVICE_NAME=") &&
+					!strings.HasPrefix(e, "TEST_SERVICE_NAME_HELPER=") &&
+					!strings.HasPrefix(e, "EXPECTED_SERVICE_NAME=") {
+					env = append(env, e)
 				}
 			}
+			env = append(env, "TEST_SERVICE_NAME_HELPER=1", "EXPECTED_SERVICE_NAME="+tc.expectedService)
+			if tc.envServiceName != "" {
+				env = append(env, "OTEL_SERVICE_NAME="+tc.envServiceName)
+			}
+			cmd.Env = env
+			out, err := cmd.CombinedOutput()
+			if err != nil {
+				t.Fatalf("helper process failed: %v\nOutput: %s", err, string(out))
+			}
+		})
+	}
+}
+
+func TestNewTracerSampling(t *testing.T) {
+	for _, tc := range []struct {
+		name        string
+		sampler     string
+		samplerArg  string
+		wantSampled bool
+	}{
+		{
+			name:        "always_on sampler",
+			sampler:     "always_on",
+			wantSampled: true,
+		},
+		{
+			name:        "always_off sampler",
+			sampler:     "always_off",
+			wantSampled: false,
+		},
+		{
+			name:        "traceidratio 1.0",
+			sampler:     "traceidratio",
+			samplerArg:  "1.0",
+			wantSampled: true,
+		},
+		{
+			name:        "traceidratio 0.0",
+			sampler:     "traceidratio",
+			samplerArg:  "0.0",
+			wantSampled: false,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Setenv("OTEL_TRACES_SAMPLER", tc.sampler)
+			t.Setenv("OTEL_TRACES_SAMPLER_ARG", tc.samplerArg)
+
+			rec := tracetest.NewSpanRecorder()
+			closer, err := newTracer(context.Background(), []trace.SpanProcessor{rec})
+			if err != nil {
+				t.Fatalf("unexpected error creating tracer: %v", err)
+			}
+			defer closer.Close()
+
+			ctx := context.Background()
+			tracer := otel.GetTracerProvider().Tracer("test-tracer")
+			_, span := tracer.Start(ctx, "test-span")
+			defer span.End()
+
+			if got := span.SpanContext().IsSampled(); got != tc.wantSampled {
+				t.Fatalf("expected span sampled=%v, got %v", tc.wantSampled, got)
+			}
 		})
 	}
 }
@@ -86,78 +212,65 @@ func TestNewExporter(t *testing.T) {
 func TestCheckDisabled(t *testing.T) {
 	for _, tc := range []struct {
 		name           string
+		sdkDisabled    string
 		endpoint       string
 		tracesEndpoint string
-		sdkDisabled    string
-		output         error
+		expectedErr    error
 	}{
 		{
-			name:   "No endpoint configured, expect ErrSkipPlugin",
-			output: plugin.ErrSkipPlugin,
+			name:        "endpoints not configured",
+			expectedErr: plugin.ErrSkipPlugin,
 		},
 		{
-			name:     "OTEL_EXPORTER_OTLP_ENDPOINT configured, expect no error",
-			endpoint: "http://localhost:4318",
-			output:   nil,
+			name:        "endpoint configured",
+			endpoint:    "http://localhost:4318",
+			expectedErr: nil,
 		},
 		{
-			name:           "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT configured, expect no error",
+			name:           "traces endpoint configured",
 			tracesEndpoint: "http://localhost:4318",
-			output:         nil,
+			expectedErr:    nil,
 		},
 		{
-			name:        "OTEL_SDK_DISABLED=true with endpoint, expect ErrSkipPlugin",
-			endpoint:    "http://localhost:4318",
+			name:        "sdk disabled true",
 			sdkDisabled: "true",
-			output:      plugin.ErrSkipPlugin,
+			endpoint:    "http://localhost:4318",
+			expectedErr: plugin.ErrSkipPlugin,
 		},
 		{
-			name:        "OTEL_SDK_DISABLED=1 with endpoint, expect ErrSkipPlugin",
-			endpoint:    "http://localhost:4318",
+			name:        "sdk disabled 1",
 			sdkDisabled: "1",
-			output:      plugin.ErrSkipPlugin,
+			endpoint:    "http://localhost:4318",
+			expectedErr: plugin.ErrSkipPlugin,
 		},
 		{
-			name:        "OTEL_SDK_DISABLED=false with endpoint, expect no error",
-			endpoint:    "http://localhost:4318",
+			name:        "sdk disabled false",
 			sdkDisabled: "false",
-			output:      nil,
+			endpoint:    "http://localhost:4318",
+			expectedErr: nil,
 		},
 		{
-			name:        "OTEL_SDK_DISABLED invalid value, expect ErrInvalidArgument",
-			endpoint:    "http://localhost:4318",
+			name:        "sdk disabled invalid",
 			sdkDisabled: "invalid",
-			output:      errdefs.ErrInvalidArgument,
+			endpoint:    "http://localhost:4318",
+			expectedErr: errdefs.ErrInvalidArgument,
 		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
+			t.Setenv(sdkDisabledEnv, tc.sdkDisabled)
 			t.Setenv(otlpEndpointEnv, tc.endpoint)
 			t.Setenv(otlpTracesEndpointEnv, tc.tracesEndpoint)
-			t.Setenv(sdkDisabledEnv, tc.sdkDisabled)
 
 			err := checkDisabled()
-			if tc.output == nil {
-				if err != nil {
-					t.Fatalf("unexpected error: %v", err)
-				}
-			} else {
-				if !errors.Is(err, tc.output) {
-					t.Fatalf("expected error %v, got %v", tc.output, err)
+			if tc.expectedErr != nil {
+				if !errors.Is(err, tc.expectedErr) {
+					t.Fatalf("expected error %v, got %v", tc.expectedErr, err)
 				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
 			}
 		})
 	}
 }
-
-func TestNewTracer(t *testing.T) {
-	exp := tracetest.NewInMemoryExporter()
-	proc := trace.NewBatchSpanProcessor(exp)
-	procs := []trace.SpanProcessor{proc}
-
-	ctx := context.TODO()
-	tracerCloser, err := newTracer(ctx, procs)
-	if err != nil {
-		t.Fatalf("unexpected error creating tracer: %v", err)
-	}
-	defer tracerCloser.Close()
-}