Merge pull request #8970 from cpuguy83/otel_off_by_default

Configure otel from env instead of config.toml

RELEASES.md

@@ -430,6 +430,9 @@ The deprecated properties in [`config.toml`](./docs/cri/config.md) are shown in
 |`[plugins."io.containerd.grpc.v1.cri".registry]`                      | `auths`                      | containerd v1.3     | containerd v2.0            | Use [`ImagePullSecrets`](https://kubernetes.io/docs/tasks/configure-pod-container/pull-image-private-registry/). See also [#8228](https://github.com/containerd/containerd/issues/8228). |
 |`[plugins."io.containerd.grpc.v1.cri".registry]`                      | `configs`                    | containerd v1.5     | containerd v2.0            | Use [`config_path`](./docs/hosts.md)            |
 |`[plugins."io.containerd.grpc.v1.cri".registry]`                      | `mirrors`                    | containerd v1.5     | containerd v2.0            | Use [`config_path`](./docs/hosts.md)            |
+|`[plugins."io.containerd.tracing.processor.v1.otlp"]`                 | `endpoint`, `protocol`, `insecure` | containerd v1.6.29 | containerd v2.0       | 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.internal.v1.tracing"]`                       | `service_name`, `sampling_ratio`   | containerd v1.6.29 | containerd v2.0       | Instead use [OTel environment variables](https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/), e.g. OTEL_SERVICE_NAME, OTEL_TRACES_SAMPLER*  |
+
 
 > **Note**
 >

integration/client/testdata/default-1.6.toml

@@ -168,9 +168,10 @@ version = 2
   [plugins."io.containerd.internal.v1.restart"]
     interval = "10s"
 
-  [plugins."io.containerd.internal.v1.tracing"]
+  # Removed in latest
-    sampling_ratio = 1.0
+  #[plugins."io.containerd.internal.v1.tracing"]
-    service_name = "containerd"
+  #  sampling_ratio = 1.0
+  #  service_name = "containerd"
 
   [plugins."io.containerd.metadata.v1.bolt"]
     content_sharing_policy = "shared"
@@ -225,10 +226,11 @@ version = 2
   #[plugins."io.containerd.snapshotter.v1.zfs"]
   #  root_path = ""
 
-  [plugins."io.containerd.tracing.processor.v1.otlp"]
+  # Removed in latest
-    endpoint = ""
+  #[plugins."io.containerd.tracing.processor.v1.otlp"]
-    insecure = false
+  #  endpoint = ""
-    protocol = ""
+  #  insecure = false
+  #  protocol = ""
 
 [proxy_plugins]
 

integration/client/testdata/default-1.7.toml

@@ -185,9 +185,10 @@ version = 2
   [plugins."io.containerd.internal.v1.restart"]
     interval = "10s"
 
-  [plugins."io.containerd.internal.v1.tracing"]
+  # Removed in latest
-    sampling_ratio = 1.0
+  #[plugins."io.containerd.internal.v1.tracing"]
-    service_name = "containerd"
+  #  sampling_ratio = 1.0
+  #  service_name = "containerd"
 
   [plugins."io.containerd.metadata.v1.bolt"]
     content_sharing_policy = "shared"
@@ -260,10 +261,11 @@ version = 2
   #[plugins."io.containerd.snapshotter.v1.zfs"]
   #  root_path = ""
 
-  [plugins."io.containerd.tracing.processor.v1.otlp"]
+  # Removed in latest
-    endpoint = ""
+  #[plugins."io.containerd.tracing.processor.v1.otlp"]
-    insecure = false
+  #  endpoint = ""
-    protocol = ""
+  #  insecure = false
+  #  protocol = ""
 
   [plugins."io.containerd.transfer.v1.local"]
     config_path = ""

pkg/deprecation/deprecation.go

@@ -31,6 +31,10 @@ const (
 	CRIRegistryAuths Warning = Prefix + "cri-registry-auths"
 	// CRIRegistryConfigs is a warning for the use of the `configs` property
 	CRIRegistryConfigs Warning = Prefix + "cri-registry-configs"
+	// OTLPTracingConfig is a warning for the use of the `otlp` property
+	TracingOTLPConfig Warning = Prefix + "tracing-processor-config"
+	// TracingServiceConfig is a warning for the use of the `tracing` property
+	TracingServiceConfig Warning = Prefix + "tracing-service-config"
 )
 
 const (
@@ -48,6 +52,11 @@ var messages = map[Warning]string{
 		"Use `ImagePullSecrets` instead.",
 	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.0." +
 		"Use `config_path` instead.",
+
+	TracingOTLPConfig: "The `otlp` property of `[plugins.\"io.containerd.tracing.processor.v1\".otlp]` is deprecated since containerd v1.6 and will be removed in containerd v2.0." +
+		"Use OTLP environment variables instead: https://opentelemetry.io/docs/specs/otel/protocol/exporter/",
+	TracingServiceConfig: "The `tracing` property of `[plugins.\"io.containerd.internal.v1\".tracing]` is deprecated since containerd v1.6 and will be removed in containerd v2.0." +
+		"Use OTEL environment variables instead: https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/",
 }
 
 // Valid checks whether a given Warning is valid

pkg/tracing/plugin/otlp.go

@@ -18,14 +18,16 @@ package plugin
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"io"
-	"net/url"
+	"os"
+	"strconv"
 	"time"
 
+	"github.com/containerd/containerd/v2/pkg/deprecation"
 	"github.com/containerd/containerd/v2/pkg/tracing"
 	"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"
@@ -35,21 +37,44 @@ import (
 	"go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc"
 	"go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp"
 	"go.opentelemetry.io/otel/propagation"
-	"go.opentelemetry.io/otel/sdk/resource"
 	"go.opentelemetry.io/otel/sdk/trace"
-	semconv "go.opentelemetry.io/otel/semconv/v1.21.0"
 )
 
 const exporterPlugin = "otlp"
 
+// OTEL and OTLP standard env vars
+// See https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/
+const (
+	sdkDisabledEnv = "OTEL_SDK_DISABLED"
+
+	otlpEndpointEnv       = "OTEL_EXPORTER_OTLP_ENDPOINT"
+	otlpTracesEndpointEnv = "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT"
+	otlpProtocolEnv       = "OTEL_EXPORTER_OTLP_PROTOCOL"
+	otlpTracesProtocolEnv = "OTEL_EXPORTER_OTLP_TRACES_PROTOCOL"
+
+	otelTracesExporterEnv = "OTEL_TRACES_EXPORTER"
+	otelServiceNameEnv    = "OTEL_SERVICE_NAME"
+)
+
 func init() {
 	registry.Register(&plugin.Registration{
 		ID:     exporterPlugin,
 		Type:   plugins.TracingProcessorPlugin,
 		Config: &OTLPConfig{},
 		InitFn: func(ic *plugin.InitContext) (interface{}, error) {
-			cfg := ic.Config.(*OTLPConfig)
+			if err := warnOTLPConfig(ic); err != nil {
-			exp, err := newExporter(ic.Context, cfg)
+				return nil, err
+			}
+			if err := checkDisabled(); err != nil {
+				return nil, err
+			}
+
+			// If OTEL_TRACES_EXPORTER is set, it must be "otlp"
+			if v := os.Getenv(otelTracesExporterEnv); v != "" && v != "otlp" {
+				return nil, fmt.Errorf("unsupported traces exporter %q: %w", v, errdefs.ErrInvalidArgument)
+			}
+
+			exp, err := newExporter(ic.Context)
 			if err != nil {
 				return nil, err
 			}
@@ -57,26 +82,32 @@ func init() {
 		},
 	})
 	registry.Register(&plugin.Registration{
-		ID:   "tracing",
+		ID:     "tracing",
-		Type: plugins.InternalPlugin,
+		Type:   plugins.InternalPlugin,
+		Config: &TraceConfig{},
 		Requires: []plugin.Type{
 			plugins.TracingProcessorPlugin,
 		},
-		Config: &TraceConfig{
-			ServiceName:        "containerd",
-			TraceSamplingRatio: 1.0,
-		},
 		InitFn: func(ic *plugin.InitContext) (interface{}, error) {
-			// get TracingProcessorPlugin which is a dependency
+			if err := warnTraceConfig(ic); err != nil {
+				return nil, err
+			}
+			if err := checkDisabled(); err != nil {
+				return nil, err
+			}
+
+			//get TracingProcessorPlugin which is a dependency
 			plugins, 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 = append(procs, p.(trace.SpanProcessor))
 			}
-			return newTracer(ic.Context, ic.Config.(*TraceConfig), procs)
+
+			return newTracer(ic.Context, procs)
 		},
 	})
 
@@ -86,61 +117,63 @@ func init() {
 
 // OTLPConfig holds the configurations for the built-in otlp span processor
 type OTLPConfig struct {
-	Endpoint string `toml:"endpoint"`
+	Endpoint string `toml:"endpoint,omitempty"`
-	Protocol string `toml:"protocol"`
+	Protocol string `toml:"protocol,omitempty"`
-	Insecure bool   `toml:"insecure"`
+	Insecure bool   `toml:"insecure,omitempty"`
 }
 
 // TraceConfig is the common configuration for open telemetry.
 type TraceConfig struct {
-	ServiceName        string  `toml:"service_name"`
+	ServiceName        string  `toml:"service_name,omitempty"`
-	TraceSamplingRatio float64 `toml:"sampling_ratio"`
+	TraceSamplingRatio float64 `toml:"sampling_ratio,omitempty"`
 }
 
-type closer struct {
+func checkDisabled() error {
-	close func() error
+	v := os.Getenv(sdkDisabledEnv)
+	if v != "" {
+		disable, err := strconv.ParseBool(v)
+		if err != nil {
+			return fmt.Errorf("invalid value for %s: %w: %w", sdkDisabledEnv, err, errdefs.ErrInvalidArgument)
+		}
+		if disable {
+			return fmt.Errorf("%w: tracing disabled by env %s=%s", plugin.ErrSkipPlugin, sdkDisabledEnv, v)
+		}
+	}
+
+	if os.Getenv(otlpEndpointEnv) == "" && os.Getenv(otlpTracesEndpointEnv) == "" {
+		return fmt.Errorf("%w: tracing endpoint not configured", plugin.ErrSkipPlugin)
+	}
+	return nil
 }
 
-func (c *closer) Close() error {
+type closerFunc func() error
-	return c.close()
+
+func (f closerFunc) Close() error {
+	return f()
 }
 
 // newExporter creates an exporter based on the given configuration.
 //
 // The default protocol is http/protobuf since it is recommended by
 // https://github.com/open-telemetry/opentelemetry-specification/blob/v1.8.0/specification/protocol/exporter.md#specify-protocol.
-func newExporter(ctx context.Context, cfg *OTLPConfig) (*otlptrace.Exporter, error) {
+func newExporter(ctx context.Context) (*otlptrace.Exporter, error) {
 	const timeout = 5 * time.Second
 
+	v := os.Getenv(otlpTracesProtocolEnv)
+	if v == "" {
+		v = os.Getenv(otlpProtocolEnv)
+	}
+
 	ctx, cancel := context.WithTimeout(ctx, timeout)
 	defer cancel()
-
+	switch v {
-	switch cfg.Protocol {
 	case "", "http/protobuf":
-		var opts []otlptracehttp.Option
+		return otlptracehttp.New(ctx)
-		if cfg.Endpoint != "" {
-			u, err := url.Parse(cfg.Endpoint)
-			if err != nil {
-				return nil, fmt.Errorf("OpenTelemetry endpoint %q %w : %v", cfg.Endpoint, errdefs.ErrInvalidArgument, err)
-			}
-			opts = append(opts, otlptracehttp.WithEndpoint(u.Host))
-			if u.Scheme == "http" {
-				opts = append(opts, otlptracehttp.WithInsecure())
-			}
-		}
-		return otlptracehttp.New(ctx, opts...)
 	case "grpc":
-		var opts []otlptracegrpc.Option
+		return otlptracegrpc.New(ctx)
-		if cfg.Endpoint != "" {
-			opts = append(opts, otlptracegrpc.WithEndpoint(cfg.Endpoint))
-		}
-		if cfg.Insecure {
-			opts = append(opts, otlptracegrpc.WithInsecure())
-		}
-		return otlptracegrpc.New(ctx, opts...)
 	default:
 		// Other protocols such as "http/json" are not supported.
-		return nil, fmt.Errorf("OpenTelemetry protocol %q : %w", cfg.Protocol, errdefs.ErrNotImplemented)
+		return nil, fmt.Errorf("OpenTelemetry protocol %q : %w", v, errdefs.ErrNotImplemented)
 	}
 }
 
@@ -148,46 +181,74 @@ func newExporter(ctx context.Context, cfg *OTLPConfig) (*otlptrace.Exporter, err
 // its sampling ratio and returns io.Closer.
 //
 // Note that this function sets process-wide tracing configuration.
-func newTracer(ctx context.Context, config *TraceConfig, procs []trace.SpanProcessor) (io.Closer, error) {
+func newTracer(ctx context.Context, procs []trace.SpanProcessor) (io.Closer, error) {
-	res, err := resource.New(ctx,
+	// Let otel configure the service name from env
-		resource.WithHost(),
+	if os.Getenv(otelServiceNameEnv) == "" {
-		resource.WithAttributes(
+		os.Setenv(otelServiceNameEnv, "containerd")
-			// Service name used to displace traces in backends
-			semconv.ServiceNameKey.String(config.ServiceName),
-		),
-	)
-	if err != nil {
-		return nil, fmt.Errorf("failed to create resource: %w", err)
 	}
 
-	sampler := trace.ParentBased(trace.TraceIDRatioBased(config.TraceSamplingRatio))
+	otel.SetTextMapPropagator(propagation.NewCompositeTextMapPropagator(propagation.TraceContext{}, propagation.Baggage{}))
-
-	opts := []trace.TracerProviderOption{
-		trace.WithSampler(sampler),
-		trace.WithResource(res),
-	}
 
+	opts := make([]trace.TracerProviderOption, 0, len(procs))
 	for _, proc := range procs {
 		opts = append(opts, trace.WithSpanProcessor(proc))
 	}
-
 	provider := trace.NewTracerProvider(opts...)
-
 	otel.SetTracerProvider(provider)
 
-	otel.SetTextMapPropagator(propagators())
+	return closerFunc(func() error {
+		return provider.Shutdown(ctx)
+	}), nil
 
-	return &closer{close: func() error {
+}
-		for _, p := range procs {
+
-			if err := p.Shutdown(ctx); err != nil && !errors.Is(err, context.Canceled) {
+func warnTraceConfig(ic *plugin.InitContext) error {
-				return err
+	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
-	}}, nil
+	}
+
+	wp, err := ic.GetSingle(plugins.WarningPlugin)
+	if err != nil {
+		return err
+	}
+	ws := wp.(warning.Service)
+	ws.Emit(ctx, deprecation.TracingServiceConfig)
+	return nil
 }
 
-// Returns a composite TestMap propagator
+func warnOTLPConfig(ic *plugin.InitContext) error {
-func propagators() propagation.TextMapPropagator {
+	ctx := ic.Context
-	return propagation.NewCompositeTextMapPropagator(propagation.TraceContext{}, propagation.Baggage{})
+	cfg := ic.Config.(*OTLPConfig)
+	var warn bool
+	if cfg.Endpoint != "" {
+		warn = true
+	}
+	if cfg.Protocol != "" {
+		warn = true
+	}
+	if cfg.Insecure {
+		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.TracingOTLPConfig)
+	return nil
 }

pkg/tracing/plugin/otlp_test.go

@@ -1,118 +0,0 @@
-/*
-   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 plugin
-
-import (
-	"context"
-	"errors"
-	"testing"
-
-	"github.com/containerd/errdefs"
-	"go.opentelemetry.io/otel/sdk/trace"
-	"go.opentelemetry.io/otel/sdk/trace/tracetest"
-)
-
-// TestNewExporter runs tests with different combinations of configuration for NewExporter function
-func TestNewExporter(t *testing.T) {
-
-	for _, testcase := range []struct {
-		name   string
-		input  OTLPConfig
-		output error
-	}{
-		{
-			name: "Test http/protobuf protocol, expect no error",
-			input: OTLPConfig{Endpoint: "http://localhost:4318",
-				Protocol: "http/protobuf",
-				Insecure: false},
-			output: nil,
-		},
-		{
-			name: "Test invalid endpoint, expect ErrInvalidArgument error",
-			input: OTLPConfig{Endpoint: "http://localhost\n:4318",
-				Protocol: "http/protobuf",
-				Insecure: false},
-			output: errdefs.ErrInvalidArgument,
-		},
-		{
-			name: "Test default protocol, expect no error",
-			input: OTLPConfig{Endpoint: "http://localhost:4318",
-				Protocol: "",
-				Insecure: false},
-			output: nil,
-		},
-		{
-			name: "Test grpc protocol, expect no error",
-			input: OTLPConfig{Endpoint: "http://localhost:4317",
-				Protocol: "grpc",
-				Insecure: false},
-			output: nil,
-		},
-		{
-			name: "Test http/json protocol which is not supported, expect not implemented error",
-			input: OTLPConfig{Endpoint: "http://localhost:4318",
-				Protocol: "http/json",
-				Insecure: false},
-			output: errdefs.ErrNotImplemented,
-		},
-	} {
-		testcase := testcase
-		t.Run(testcase.name, func(t *testing.T) {
-			t.Logf("input: %v", testcase.input)
-
-			ctx := context.TODO()
-			exp, err := newExporter(ctx, &testcase.input)
-			t.Logf("output: %v", err)
-
-			if err == nil {
-				if err != testcase.output {
-					t.Fatalf("Expect to get error: %v, however no error got\n", testcase.output)
-				} else if exp == nil {
-					t.Fatalf("Something went wrong, Exporter not created as expected\n")
-				}
-			} else {
-				if !errors.Is(err, testcase.output) {
-					t.Fatalf("Expect to get error: %v, however error %v returned\n", testcase.output, err)
-				}
-			}
-
-		})
-	}
-}
-
-// TestNewTracer runs test for NewTracer function
-func TestNewTracer(t *testing.T) {
-
-	config := &TraceConfig{ServiceName: "containerd", TraceSamplingRatio: 1.0}
-	t.Logf("config: %v", config)
-
-	procs := make([]trace.SpanProcessor, 0, 1)
-
-	//Create a dummy in memory exporter for test
-	exp := tracetest.NewInMemoryExporter()
-	proc := trace.NewBatchSpanProcessor(exp)
-
-	procs = append(procs, proc)
-
-	ctx := context.TODO()
-	tracerCloser, err := newTracer(ctx, config, procs)
-	if err != nil {
-		t.Fatalf("Something went wrong, Tracer not created as expected\n")
-	}
-
-	defer tracerCloser.Close()
-}