fix(server): prevent otel tracer provider from immediately being closed (#7369)

coadler · web-flow · commit 75ad72de5678 · 2023-05-02T12:06:58.000-05:00
diff --git a/cli/server.go b/cli/server.go
@@ -256,7 +256,13 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
 			// which is caught by goleaks.
 			defer http.DefaultClient.CloseIdleConnections()
 
-			tracerProvider, sqlDriver := ConfigureTraceProvider(ctx, logger, inv, cfg)
+			tracerProvider, sqlDriver, closeTracing := ConfigureTraceProvider(ctx, logger, inv, cfg)
+			defer func() {
+				logger.Debug(ctx, "closing tracing")
+				traceCloseErr := shutdownWithTimeout(closeTracing, 5*time.Second)
+				logger.Debug(ctx, "tracing closed", slog.Error(traceCloseErr))
+			}()
+
 			httpServers, err := ConfigureHTTPServers(inv, cfg)
 			if err != nil {
 				return xerrors.Errorf("configure http(s): %w", err)
@@ -1863,9 +1869,15 @@ func (s *HTTPServers) Close() {
 	}
 }
 
-func ConfigureTraceProvider(ctx context.Context, logger slog.Logger, inv *clibase.Invocation, cfg *codersdk.DeploymentValues) (trace.TracerProvider, string) {
+func ConfigureTraceProvider(
+	ctx context.Context,
+	logger slog.Logger,
+	inv *clibase.Invocation,
+	cfg *codersdk.DeploymentValues,
+) (trace.TracerProvider, string, func(context.Context) error) {
 	var (
-		tracerProvider trace.TracerProvider
+		tracerProvider = trace.NewNoopTracerProvider()
+		closeTracing   = func(context.Context) error { return nil }
 		sqlDriver      = "postgres"
 	)
 	// Coder tracing should be disabled if telemetry is disabled unless
@@ -1878,19 +1890,14 @@ func ConfigureTraceProvider(ctx context.Context, logger slog.Logger, inv *clibas
 	}
 
 	if cfg.Trace.Enable.Value() || shouldCoderTrace || cfg.Trace.HoneycombAPIKey != "" {
-		sdkTracerProvider, closeTracing, err := tracing.TracerProvider(ctx, "coderd", tracing.TracerOpts{
+		sdkTracerProvider, _closeTracing, err := tracing.TracerProvider(ctx, "coderd", tracing.TracerOpts{
 			Default:   cfg.Trace.Enable.Value(),
 			Coder:     shouldCoderTrace,
 			Honeycomb: cfg.Trace.HoneycombAPIKey.String(),
 		})
 		if err != nil {
 			logger.Warn(ctx, "start telemetry exporter", slog.Error(err))
 		} else {
-			// allow time for traces to flush even if command context is canceled
-			defer func() {
-				_ = shutdownWithTimeout(closeTracing, 5*time.Second)
-			}()
-
 			d, err := tracing.PostgresDriver(sdkTracerProvider, "coderd.database")
 			if err != nil {
 				logger.Warn(ctx, "start postgres tracing driver", slog.Error(err))
@@ -1899,9 +1906,10 @@ func ConfigureTraceProvider(ctx context.Context, logger slog.Logger, inv *clibas
 			}
 
 			tracerProvider = sdkTracerProvider
+			closeTracing = _closeTracing
 		}
 	}
-	return tracerProvider, sqlDriver
+	return tracerProvider, sqlDriver, closeTracing
 }
 
 func ConfigureHTTPServers(inv *clibase.Invocation, cfg *codersdk.DeploymentValues) (_ *HTTPServers, err error) {
diff --git a/enterprise/cli/proxyserver.go b/enterprise/cli/proxyserver.go
@@ -21,6 +21,7 @@ import (
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 	"golang.org/x/xerrors"
 
+	"cdr.dev/slog"
 	"github.com/coder/coder/cli"
 	"github.com/coder/coder/cli/clibase"
 	"github.com/coder/coder/cli/cliui"
@@ -136,7 +137,12 @@ func (*RootCmd) proxyServer() *clibase.Cmd {
 			defer http.DefaultClient.CloseIdleConnections()
 			closers.Add(http.DefaultClient.CloseIdleConnections)
 
-			tracer, _ := cli.ConfigureTraceProvider(ctx, logger, inv, cfg)
+			tracer, _, closeTracing := cli.ConfigureTraceProvider(ctx, logger, inv, cfg)
+			defer func() {
+				logger.Debug(ctx, "closing tracing")
+				traceCloseErr := shutdownWithTimeout(closeTracing, 5*time.Second)
+				logger.Debug(ctx, "tracing closed", slog.Error(traceCloseErr))
+			}()
 
 			httpServers, err := cli.ConfigureHTTPServers(inv, cfg)
 			if err != nil {
@@ -345,3 +351,9 @@ func (*RootCmd) proxyServer() *clibase.Cmd {
 
 	return cmd
 }
+
+func shutdownWithTimeout(shutdown func(context.Context) error, timeout time.Duration) error {
+	ctx, cancel := context.WithTimeout(context.Background(), timeout)
+	defer cancel()
+	return shutdown(ctx)
+}
