fix: enable OTel auto-detection via standard OTEL_ env vars, deprecate --otel-provider (fixes #712)

pkg/cmd/serve.go

@@ -209,6 +209,11 @@
 	// NOTE: cobraotel.New takes service name as an arg rather than command name.
 	otel := cobraotel.New("spicedb")
 	otel.RegisterFlags(tracingFlags)
+	// Deprecate --otel-provider: the provider is now inferred automatically
+	// from standard OTEL_* environment variables when not explicitly set.
+	if err := tracingFlags.MarkDeprecated("otel-provider", "provider is inferred automatically from configuration; this flag will be removed in a future release"); err != nil {
+		return fmt.Errorf("failed to mark otel-provider as deprecated: %w", err)
+	}
 
 	loggingFlagSet := nfs.FlagSet(BoldBlue("Logging"))
 	loggingFlagSet.BoolVar(&config.EnableRequestLogs, "grpc-log-requests-enabled", false, "enable logging of API request payloads")

pkg/cmd/serve_test.go

@@ -1,3 +1,3 @@
 package cmd
 
 import (
@@ -7,6 +7,7 @@
 	"strings"
 	"testing"
 
+	cobrautil "github.com/jzelinskie/cobrautil/v2"
 	"github.com/spf13/cobra"
 	"github.com/stretchr/testify/require"
 
@@ -198,3 +199,121 @@
 		require.Equal(t, uint16(23000), mergedConfig.DatastoreConfig.WatchBufferLength)
 	})
 }
+
+// runOTelProviderTest wires up a real serve command (so all flags are
+// registered) but replaces PreRunE with a minimal stack that only runs
+// SyncViperDotEnvPreRunE followed by inferOTelProviderFromEnvPreRunE. This
+// means no actual OTel exporter connection is ever opened, keeping the tests
+// hermetic. The resolved value of --otel-provider is returned.
+func runOTelProviderTest(t *testing.T, args []string, envVars map[string]string) string {
+	t.Helper()
+
+	for k, v := range envVars {
+		t.Setenv(k, v)
+	}
+
+	config := server.NewConfigWithOptionsAndDefaults()
+	cmd := NewServeCommand("spicedb", config)
+	require.NoError(t, RegisterRootFlags(cmd))
+	require.NoError(t, RegisterServeFlags(cmd, config))
+
+	// Disable metrics singletons to avoid test pollution.
+	config.DispatchClusterMetricsEnabled = false
+	config.DispatchClientMetricsEnabled = false
+	config.DatastoreConfig.EnableDatastoreMetrics = false
+	config.DispatchCacheConfig.Metrics = false
+	config.ClusterDispatchCacheConfig.Metrics = false
+	config.NamespaceCacheConfig.Metrics = false
+
+	var resolvedProvider string
+
+	// Replace PreRunE with only the two hooks we care about:
+	//   1. SyncViper – so SPICEDB_* env vars (and .env file) are applied.
+	//   2. inferOTelProviderFromEnvPreRunE – the logic under test.
+	// cobraotel.RunE is deliberately excluded so no collector connection
+	// is ever opened.
+	cmd.PreRunE = func(cmd *cobra.Command, cmdArgs []string) error {
+		syncViper := cobrautil.SyncViperPreRunE("spicedb")
+		if err := syncViper(cmd, cmdArgs); err != nil {
+			return err
+		}
+		infer := server.InferOTelProviderFromEnvPreRunE()
+		return infer(cmd, cmdArgs)
+	}
+
+	// RunE just reads the flag after PreRunE has fired.
+	cmd.RunE = func(cmd *cobra.Command, _ []string) error {
+		f := cmd.Flags().Lookup("otel-provider")
+		if f != nil {
+			resolvedProvider = f.Value.String()
+		}
+		return nil
+	}
+
+	baseArgs := []string{"--grpc-preshared-key", "test-key"}
+	cmd.SetArgs(append(baseArgs, args...))
+	require.NoError(t, cmd.Execute())
+
+	return resolvedProvider
+}
+
+// TestOTelEnvOnlyEnablesTracing verifies that setting a standard OTEL_*
+// environment variable (without any SpiceDB flags) causes the provider to be
+// inferred as "otlpgrpc".
+func TestOTelEnvOnlyEnablesTracing(t *testing.T) {
+	provider := runOTelProviderTest(t, nil, map[string]string{
+		"OTEL_EXPORTER_OTLP_ENDPOINT": "localhost:4317",
+	})
+	require.Equal(t, "otlpgrpc", provider)
+}
+
+// TestOTelTracesEndpointEnablesTracing covers the OTEL_EXPORTER_OTLP_TRACES_ENDPOINT variant.
+func TestOTelTracesEndpointEnablesTracing(t *testing.T) {
+	provider := runOTelProviderTest(t, nil, map[string]string{
+		"OTEL_EXPORTER_OTLP_TRACES_ENDPOINT": "localhost:4317",
+	})
+	require.Equal(t, "otlpgrpc", provider)
+}
+
+// TestOTelTracesExporterEnablesTracing covers the OTEL_TRACES_EXPORTER variant.
+func TestOTelTracesExporterEnablesTracing(t *testing.T) {
+	provider := runOTelProviderTest(t, nil, map[string]string{
+		"OTEL_TRACES_EXPORTER": "otlp",
+	})
+	require.Equal(t, "otlpgrpc", provider)
+}
+
+// TestOTelFlagOnlyUnchanged verifies that when only SpiceDB flags are used,
+// they are passed through unchanged.
+func TestOTelFlagOnlyUnchanged(t *testing.T) {
+	provider := runOTelProviderTest(t,
+		[]string{"--otel-provider", "otlphttp"},
+		nil,
+	)
+	require.Equal(t, "otlphttp", provider)
+}
+
+// TestOTelExplicitNoneDisablesTracing verifies that --otel-provider=none
+// overrides any OTEL_* environment variables; tracing stays disabled.
+func TestOTelExplicitNoneDisablesTracing(t *testing.T) {
+	provider := runOTelProviderTest(t,
+		[]string{"--otel-provider", "none"},
+		map[string]string{
+			"OTEL_EXPORTER_OTLP_ENDPOINT": "localhost:4317",
+		},
+	)
+	require.Equal(t, "none", provider)
+}
+
+// TestOTelNoConfigDefaultsToNone verifies the baseline: with no flags and no
+// OTEL_* env vars, the provider remains "none" and tracing is disabled.
+func TestOTelNoConfigDefaultsToNone(t *testing.T) {
+	// Scrub any OTEL_* vars that might be set in the test environment.
+	os.Unsetenv("OTEL_EXPORTER_OTLP_ENDPOINT")
+	os.Unsetenv("OTEL_EXPORTER_OTLP_TRACES_ENDPOINT")
+	os.Unsetenv("OTEL_TRACES_EXPORTER")
+
+	provider := runOTelProviderTest(t, nil, nil)
+	require.Equal(t, "none", provider)
+}
+

pkg/cmd/server/defaults.go

@@ -6,6 +6,7 @@
 	"fmt"
 	"net/http"
 	"net/http/pprof"
+	"os"
 	"sync"
 	"time"
 
@@ -25,6 +26,7 @@
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 	"github.com/rs/zerolog"
+	"github.com/spf13/cobra"
 	"go.opentelemetry.io/otel/trace"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
@@ -66,6 +68,43 @@
 	)
 }
 
+// otelEnvVars is the set of standard OpenTelemetry environment variables that
+// indicate the user intends to configure tracing via the environment rather
+// than SpiceDB-specific flags.
+var otelEnvVars = []string{
+	"OTEL_EXPORTER_OTLP_ENDPOINT",
+	"OTEL_EXPORTER_OTLP_TRACES_ENDPOINT",
+	"OTEL_TRACES_EXPORTER",
+}
+
+// InferOTelProviderFromEnvPreRunE returns a CobraRunFunc that, when the
+// --otel-provider flag has NOT been explicitly set by the user, checks whether
+// any standard OTEL_* environment variables are present. If they are, it
+// promotes the provider to "otlpgrpc" so that cobraotel.RunE will actually
+// initialise a tracer provider and let the OpenTelemetry SDK read the rest of
+// the configuration from the environment.
+func InferOTelProviderFromEnvPreRunE() cobrautil.CobraRunFunc {
+	return func(cmd *cobra.Command, args []string) error {
+		flag := cmd.Flags().Lookup("otel-provider")
+		if flag == nil || flag.Changed {
+			// Flag does not exist on this command, or the user set it explicitly.
+			// In either case do not touch it.
+			return nil
+		}
+
+		for _, envVar := range otelEnvVars {
+			if os.Getenv(envVar) != "" {
+				// At least one standard OTEL env var is set; activate tracing.
+				if err := flag.Value.Set("otlpgrpc"); err != nil {
+					return fmt.Errorf("failed to infer otel provider from environment: %w", err)
+				}
+				return nil
+			}
+		}
+		return nil
+	}
+}
+
 // DefaultPreRunE sets up viper, zerolog, and OpenTelemetry flag handling for a
 // command.
 func DefaultPreRunE(programName string) cobrautil.CobraRunFunc {
@@ -82,6 +121,10 @@
 		// and zero under the same load and 0.9
 		cobraproclimits.SetMemLimitRunE(memlimit.WithRatio(0.9)),
 		cobraproclimits.SetProcLimitRunE(),
+		// Infer the OTel provider from standard OTEL_* env vars when the user
+		// has not explicitly specified --otel-provider. This must run before
+		// cobraotel.RunE so the correct provider is in place.
+		InferOTelProviderFromEnvPreRunE(),
 		cobraotel.New("spicedb",
 			cobraotel.WithLogger(zerologr.New(&logging.Logger)),
 		).RunE(),