fix(telemetry): disable OTLP exporters for empty endpoints (cherry-pick #16898 for 4.1) (#16930)

Signed-off-by: Eric Wyles <23637493+ericwyles@users.noreply.github.com>
Co-authored-by: Eric Wyles <23637493+ericwyles@users.noreply.github.com>

Author: argo-cd-cherry-pick-bot[bot]

docs/telemetry-configuration.md

@@ -16,7 +16,7 @@ To enable the OpenTelemetry protocol you must set the environment variable `OTEL
 This must be set on the workflow-controller, and ideally on all the containers (`main`, `init`, and `wait`) in the workload.
 You can use the [OpenTelemetry operator](https://opentelemetry.io/docs/kubernetes/operator/) to setup the collector and instrument the workflow-controller.
 The OpenTelemetry operator can also instrument your workload pods so that they emit spans as part of workflow tracing and this is the recommended setup.
-It will not be enabled if left blank, unlike some other implementations.
+It will not be enabled unless `OTEL_EXPORTER_OTLP_ENDPOINT` or the applicable signal-specific endpoint is configured and non-empty.
 
 You can configure the protocol using the environment variables documented in the [OpenTelemetry standard environment variables](https://opentelemetry.io/docs/languages/sdk-configuration/otlp-exporter/).
 

util/telemetry/metrics.go

@@ -60,14 +60,13 @@ func (m *Metrics) Shutdown(ctx context.Context) error {
 	return nil
 }
 
+// NewMetrics creates a metrics provider configured with the enabled OTLP and Prometheus exporters.
 func NewMetrics(ctx context.Context, serviceName, prometheusName string, config *MetricsConfig, extraOpts ...metricsdk.Option) (*Metrics, error) {
 	options := make([]metricsdk.Option, 0)
 	options = append(options, metricsdk.WithResource(workflowsResource(ctx, serviceName)))
-	_, otlpEnabled := os.LookupEnv(`OTEL_EXPORTER_OTLP_ENDPOINT`)
-	_, otlpMetricsEnabled := os.LookupEnv(`OTEL_EXPORTER_OTLP_METRICS_ENDPOINT`)
 	logger := logging.RequireLoggerFromContext(ctx)
 
-	if otlpEnabled || otlpMetricsEnabled {
+	if endpoint := resolveOTLPEndpoint(otlpMetricsEndpointEnv); endpoint != "" {
 		// NOTE: The OTel SDK default changed from gRPC to http/protobuf. For backwards compatibility,
 		// gRPC is preserved as the default in workflows controller, but http/protobuf can be opted-in
 		// to by setting the _PROTOCOL env var explicitly.
@@ -76,11 +75,6 @@ func NewMetrics(ctx context.Context, serviceName, prometheusName string, config
 		if otlpProtocol == "" {
 			otlpProtocol = os.Getenv(`OTEL_EXPORTER_OTLP_PROTOCOL`)
 		}
-		endpoint := os.Getenv(`OTEL_EXPORTER_OTLP_METRICS_ENDPOINT`)
-		if endpoint == "" {
-			endpoint = os.Getenv(`OTEL_EXPORTER_OTLP_ENDPOINT`)
-		}
-
 		switch {
 		case otlpProtocol == "" || otlpProtocol == "grpc":
 			logger.WithFields(logging.Fields{"protocol": "grpc", "endpoint": endpoint}).Info(ctx, "Starting OTLP metrics exporter")

util/telemetry/otlp.go

@@ -0,0 +1,20 @@
+package telemetry
+
+import (
+	"os"
+	"strings"
+)
+
+const (
+	otlpEndpointEnv        = "OTEL_EXPORTER_OTLP_ENDPOINT"
+	otlpMetricsEndpointEnv = "OTEL_EXPORTER_OTLP_METRICS_ENDPOINT"
+	otlpTracesEndpointEnv  = "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT"
+)
+
+// resolveOTLPEndpoint returns the signal-specific endpoint, falling back to the common OTLP endpoint.
+func resolveOTLPEndpoint(signalEndpointEnv string) string {
+	if endpoint := strings.TrimSpace(os.Getenv(signalEndpointEnv)); endpoint != "" {
+		return endpoint
+	}
+	return strings.TrimSpace(os.Getenv(otlpEndpointEnv))
+}

util/telemetry/otlp_test.go

@@ -0,0 +1,67 @@
+package telemetry
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+// TestResolveOTLPEndpoint verifies endpoint precedence and handling of empty values.
+func TestResolveOTLPEndpoint(t *testing.T) {
+	tests := []struct {
+		name                   string
+		commonEndpoint         string
+		signalSpecificEndpoint string
+		expectedEndpoint       string
+	}{
+		{
+			name: "empty values",
+		},
+		{
+			name:             "common endpoint",
+			commonEndpoint:   "common:4317",
+			expectedEndpoint: "common:4317",
+		},
+		{
+			name:                   "signal-specific endpoint",
+			signalSpecificEndpoint: "signal:4317",
+			expectedEndpoint:       "signal:4317",
+		},
+		{
+			name:                   "signal-specific endpoint takes precedence",
+			commonEndpoint:         "common:4317",
+			signalSpecificEndpoint: "signal:4317",
+			expectedEndpoint:       "signal:4317",
+		},
+		{
+			name:                   "whitespace-only values",
+			commonEndpoint:         " ",
+			signalSpecificEndpoint: "\t",
+		},
+		{
+			name:                   "endpoints with surrounding whitespace",
+			commonEndpoint:         " common:4317 ",
+			signalSpecificEndpoint: " signal:4317 ",
+			expectedEndpoint:       "signal:4317",
+		},
+		{
+			name:                   "whitespace-only signal-specific endpoint falls back to common endpoint",
+			commonEndpoint:         "common:4317",
+			signalSpecificEndpoint: " ",
+			expectedEndpoint:       "common:4317",
+		},
+	}
+
+	for _, signalEndpointEnv := range []string{otlpMetricsEndpointEnv, otlpTracesEndpointEnv} {
+		t.Run(signalEndpointEnv, func(t *testing.T) {
+			for _, test := range tests {
+				t.Run(test.name, func(t *testing.T) {
+					t.Setenv(otlpEndpointEnv, test.commonEndpoint)
+					t.Setenv(signalEndpointEnv, test.signalSpecificEndpoint)
+
+					require.Equal(t, test.expectedEndpoint, resolveOTLPEndpoint(signalEndpointEnv))
+				})
+			}
+		})
+	}
+}

util/telemetry/tracing.go

@@ -86,14 +86,13 @@ type Tracing struct {
 	tracer   trace.Tracer
 }
 
+// NewTracing creates a tracing provider configured with the enabled OTLP exporter.
 func NewTracing(ctx context.Context, serviceName string, extraOpts ...tracesdk.TracerProviderOption) (*Tracing, error) {
 	options := make([]tracesdk.TracerProviderOption, 0)
 	options = append(options, tracesdk.WithResource(workflowsResource(ctx, serviceName)))
 	options = append(options, extraOpts...)
-	_, otlpEnabled := os.LookupEnv(`OTEL_EXPORTER_OTLP_ENDPOINT`)
-	_, otlpTracingEnabled := os.LookupEnv(`OTEL_EXPORTER_OTLP_TRACES_ENDPOINT`)
 
-	if otlpEnabled || otlpTracingEnabled {
+	if endpoint := resolveOTLPEndpoint(otlpTracesEndpointEnv); endpoint != "" {
 		// NOTE: The OTel SDK default changed from gRPC to http/protobuf. For alignment with metrics
 		// gRPC is preserved as the default in workflows controller, but http/protobuf can be opted-in
 		// to by setting the _PROTOCOL env var explicitly.
@@ -104,11 +103,6 @@ func NewTracing(ctx context.Context, serviceName string, extraOpts ...tracesdk.T
 		}
 
 		logger := logging.RequireLoggerFromContext(ctx)
-		endpoint := os.Getenv(`OTEL_EXPORTER_OTLP_TRACES_ENDPOINT`)
-		if endpoint == "" {
-			endpoint = os.Getenv(`OTEL_EXPORTER_OTLP_ENDPOINT`)
-		}
-
 		switch {
 		case otlpProtocol == "" || otlpProtocol == "grpc":
 			logger.WithFields(logging.Fields{"protocol": "grpc", "endpoint": endpoint}).Info(ctx, "Starting OTLP tracing exporter")