diff --git a/RELEASES.md b/RELEASES.md index 8715ec4ee..69d8f27b3 100644 --- a/RELEASES.md +++ b/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** > diff --git a/integration/client/testdata/default-1.6.toml b/integration/client/testdata/default-1.6.toml index 810b78925..ddc9237a7 100644 --- a/integration/client/testdata/default-1.6.toml +++ b/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"] - sampling_ratio = 1.0 - service_name = "containerd" + # Removed in latest + #[plugins."io.containerd.internal.v1.tracing"] + # 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"] - endpoint = "" - insecure = false - protocol = "" + # Removed in latest + #[plugins."io.containerd.tracing.processor.v1.otlp"] + # endpoint = "" + # insecure = false + # protocol = "" [proxy_plugins] diff --git a/integration/client/testdata/default-1.7.toml b/integration/client/testdata/default-1.7.toml index 601190299..0c00ef26c 100644 --- a/integration/client/testdata/default-1.7.toml +++ b/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"] - sampling_ratio = 1.0 - service_name = "containerd" + # Removed in latest + #[plugins."io.containerd.internal.v1.tracing"] + # 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"] - endpoint = "" - insecure = false - protocol = "" + # Removed in latest + #[plugins."io.containerd.tracing.processor.v1.otlp"] + # endpoint = "" + # insecure = false + # protocol = "" [plugins."io.containerd.transfer.v1.local"] config_path = "" diff --git a/pkg/deprecation/deprecation.go b/pkg/deprecation/deprecation.go index 64723a7b2..1df9b9c88 100644 --- a/pkg/deprecation/deprecation.go +++ b/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 diff --git a/pkg/tracing/plugin/otlp.go b/pkg/tracing/plugin/otlp.go index 489233a7c..52586913e 100644 --- a/pkg/tracing/plugin/otlp.go +++ b/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) - exp, err := newExporter(ic.Context, cfg) + if err := warnOTLPConfig(ic); err != nil { + 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", - Type: plugins.InternalPlugin, + ID: "tracing", + 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"` - Protocol string `toml:"protocol"` - Insecure bool `toml:"insecure"` + Endpoint string `toml:"endpoint,omitempty"` + Protocol string `toml:"protocol,omitempty"` + Insecure bool `toml:"insecure,omitempty"` } // TraceConfig is the common configuration for open telemetry. type TraceConfig struct { - ServiceName string `toml:"service_name"` - TraceSamplingRatio float64 `toml:"sampling_ratio"` + ServiceName string `toml:"service_name,omitempty"` + TraceSamplingRatio float64 `toml:"sampling_ratio,omitempty"` } -type closer struct { - close func() error +func checkDisabled() 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 { - return c.close() +type closerFunc func() error + +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 cfg.Protocol { + switch v { case "", "http/protobuf": - var opts []otlptracehttp.Option - 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...) + return otlptracehttp.New(ctx) case "grpc": - var opts []otlptracegrpc.Option - if cfg.Endpoint != "" { - opts = append(opts, otlptracegrpc.WithEndpoint(cfg.Endpoint)) - } - if cfg.Insecure { - opts = append(opts, otlptracegrpc.WithInsecure()) - } - return otlptracegrpc.New(ctx, opts...) + return otlptracegrpc.New(ctx) 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) { - res, err := resource.New(ctx, - resource.WithHost(), - resource.WithAttributes( - // 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) +func newTracer(ctx context.Context, procs []trace.SpanProcessor) (io.Closer, error) { + // Let otel configure the service name from env + if os.Getenv(otelServiceNameEnv) == "" { + os.Setenv(otelServiceNameEnv, "containerd") } - sampler := trace.ParentBased(trace.TraceIDRatioBased(config.TraceSamplingRatio)) - - opts := []trace.TracerProviderOption{ - trace.WithSampler(sampler), - trace.WithResource(res), - } + otel.SetTextMapPropagator(propagation.NewCompositeTextMapPropagator(propagation.TraceContext{}, propagation.Baggage{})) + 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) { - return err - } - } +} + +func warnTraceConfig(ic *plugin.InitContext) error { + 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 propagators() propagation.TextMapPropagator { - return propagation.NewCompositeTextMapPropagator(propagation.TraceContext{}, propagation.Baggage{}) +func warnOTLPConfig(ic *plugin.InitContext) error { + ctx := ic.Context + 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 } diff --git a/pkg/tracing/plugin/otlp_test.go b/pkg/tracing/plugin/otlp_test.go deleted file mode 100644 index 2f314b4df..000000000 --- a/pkg/tracing/plugin/otlp_test.go +++ /dev/null @@ -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() -}