fix: address second round of review feedback

- Use fresh context.Background() with 30s timeout for telemetry shutdown
  instead of app ctx (which may be canceled on SIGTERM)
- Only log 'telemetry initialized' when endpoint is non-empty
- Wrap /auth and /admin handlers with otelhttp.NewHandler for consistent
  trace instrumentation across all routes
- Clamp TraceSampleRate to [0,1] and update field doc comment
- Fix version.go doc comment (Get() -> Version(), Commit(), BuildDate())

commit: 77e28e6b3d8c9e9d917ae2be1a5c620f3e3778f5
author: brice · 8mo ago · 2026-02-10 07:47 UTC
parent: eef0d4e
signature: unsigned

3 files · +27 -9

M internal/adapters/out/telemetry/provider.go +11 -4


@@ -27,7 +27,7 @@ type Config struct {
27	27		Traces bool `mapstructure:"traces"` // Enable trace export
28	28		Metrics bool `mapstructure:"metrics"` // Enable metric export
29	29		Logs bool `mapstructure:"logs"` // Bridge zerowrap logs to OTLP
30		-	TraceSampleRate float64 `mapstructure:"trace_sample_rate"` // 0.0-1.0, default 1.0
	30	+	TraceSampleRate float64 `mapstructure:"trace_sample_rate"` // 0=disabled, (0,1)=ratio-based, >=1=always sample
31	31		}
32	32
33	33		// Provider holds the initialized OTel providers.

@@ -141,11 +141,18 @@ func (p Provider) setupTracing(ctx context.Context, ep endpointConfig, cfg Con
141	141		}
142	142
143	143		// 0 = never sample (disabled), (0,1) = ratio-based, >= 1 = always sample.
	144	+	// Clamp to [0, 1] so out-of-range values don't produce surprising behavior.
	145	+	rate := cfg.TraceSampleRate
	146	+	if rate < 0 {
	147	+	rate = 0
	148	+	} else if rate > 1 {
	149	+	rate = 1
	150	+	}
144	151		sampler := trace.AlwaysSample()
145		-	if cfg.TraceSampleRate == 0 {
	152	+	if rate == 0 {
146	153		sampler = trace.NeverSample()
147		-	} else if cfg.TraceSampleRate < 1.0 {
148		-	sampler = trace.TraceIDRatioBased(cfg.TraceSampleRate)
	154	+	} else if rate < 1.0 {
	155	+	sampler = trace.TraceIDRatioBased(rate)
149	156		}
150	157
151	158		tp := trace.NewTracerProvider(

 	Traces          bool    `mapstructure:"traces"`            // Enable trace export
 	Metrics         bool    `mapstructure:"metrics"`           // Enable metric export
 	Logs            bool    `mapstructure:"logs"`              // Bridge zerowrap logs to OTLP
-	TraceSampleRate float64 `mapstructure:"trace_sample_rate"` // 0.0-1.0, default 1.0
+	TraceSampleRate float64 `mapstructure:"trace_sample_rate"` // 0=disabled, (0,1)=ratio-based, >=1=always sample
 }
 // Provider holds the initialized OTel providers.
 	}
 	// 0 = never sample (disabled), (0,1) = ratio-based, >= 1 = always sample.
+	// Clamp to [0, 1] so out-of-range values don't produce surprising behavior.
+	rate := cfg.TraceSampleRate
+	if rate < 0 {
+		rate = 0
+	} else if rate > 1 {
+		rate = 1
+	}
 	sampler := trace.AlwaysSample()
-	if cfg.TraceSampleRate == 0 {
+	if rate == 0 {
 		sampler = trace.NeverSample()
-	} else if cfg.TraceSampleRate < 1.0 {
+	} else if rate < 1.0 {
-		sampler = trace.TraceIDRatioBased(cfg.TraceSampleRate)
+		sampler = trace.TraceIDRatioBased(rate)
 	}
 	tp := trace.NewTracerProvider(

M internal/app/run.go +15 -4


@@ -217,8 +217,13 @@ func Run(ctx context.Context, configPath string) error {
217	217		if err != nil {
218	218		log.Warn().Err(err).Msg("failed to initialize telemetry, continuing without it")
219	219		} else {
220		-	defer telShutdown(ctx)
221		-	if cfg.Telemetry.Enabled {
	220	+	// Use a fresh context for shutdown so a canceled app ctx doesn't prevent flushing.
	221	+	defer func() {
	222	+	shutdownCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
	223	+	defer cancel()
	224	+	telShutdown(shutdownCtx)
	225	+	}()
	226	+	if cfg.Telemetry.Enabled && cfg.Telemetry.Endpoint != "" {
222	227		// Bridge zerowrap logs to OTel if log export is enabled
223	228		if cfg.Telemetry.Logs && telProvider.LogProvider != nil {
224	229		otelHook := zerowrapotel.NewHookWithProvider(telProvider.LogProvider, "gordon")

@@ -1510,7 +1515,10 @@ func registerAuthRoutes(
1510	1515		authMiddlewares = append(authMiddlewares, cidrAllowlistMiddleware)
1511	1516		}
1512	1517		authMiddlewares = append(authMiddlewares, rateLimitMiddleware)
1513		-	authWithMiddleware := middleware.Chain(authMiddlewares...)(svc.authHandler)
	1518	+	authWithMiddleware := otelhttp.NewHandler(
	1519	+	middleware.Chain(authMiddlewares...)(svc.authHandler),
	1520	+	"gordon.auth",
	1521	+	)
1514	1522		registryMux.Handle("/auth/", authWithMiddleware)
1515	1523		}
1516	1524

@@ -1555,7 +1563,10 @@ func registerAdminRoutes(registryMux, proxyMux http.ServeMux, svc services, cf
1555	1563		})
1556	1564		}
1557	1565
1558		-	adminWithMiddleware := middleware.Chain(adminMiddlewares...)(svc.adminHandler)
	1566	+	adminWithMiddleware := otelhttp.NewHandler(
	1567	+	middleware.Chain(adminMiddlewares...)(svc.adminHandler),
	1568	+	"gordon.admin",
	1569	+	)
1559	1570		registryMux.Handle("/admin/", loopbackOnly(adminWithMiddleware, log))
1560	1571		proxyMux.Handle("/admin/", adminHostOnly(adminWithMiddleware, cfg.Server.RegistryDomain, log))
1561	1572		}

 	if err != nil {
 		log.Warn().Err(err).Msg("failed to initialize telemetry, continuing without it")
 	} else {
-		defer telShutdown(ctx)
+		// Use a fresh context for shutdown so a canceled app ctx doesn't prevent flushing.
-		if cfg.Telemetry.Enabled {
+		defer func() {
+			shutdownCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+			defer cancel()
+			telShutdown(shutdownCtx)
+		}()
+		if cfg.Telemetry.Enabled && cfg.Telemetry.Endpoint != "" {
 			// Bridge zerowrap logs to OTel if log export is enabled
 			if cfg.Telemetry.Logs && telProvider.LogProvider != nil {
 				otelHook := zerowrapotel.NewHookWithProvider(telProvider.LogProvider, "gordon")
 		authMiddlewares = append(authMiddlewares, cidrAllowlistMiddleware)
 	}
 	authMiddlewares = append(authMiddlewares, rateLimitMiddleware)
-	authWithMiddleware := middleware.Chain(authMiddlewares...)(svc.authHandler)
+	authWithMiddleware := otelhttp.NewHandler(
+		middleware.Chain(authMiddlewares...)(svc.authHandler),
+		"gordon.auth",
+	)
 	registryMux.Handle("/auth/", authWithMiddleware)
 }
 		})
 	}
-	adminWithMiddleware := middleware.Chain(adminMiddlewares...)(svc.adminHandler)
+	adminWithMiddleware := otelhttp.NewHandler(
+		middleware.Chain(adminMiddlewares...)(svc.adminHandler),
+		"gordon.admin",
+	)
 	registryMux.Handle("/admin/", loopbackOnly(adminWithMiddleware, log))
 	proxyMux.Handle("/admin/", adminHostOnly(adminWithMiddleware, cfg.Server.RegistryDomain, log))
 }

M pkg/version/version.go +1 -1

@@ -1,5 +1,5 @@
1	1		// Package version holds build-time version info for Gordon.
2		-	// Set via main using Set(), read from anywhere via Get().
	2	+	// Set via main using Set(), read from anywhere via Version(), Commit(), or BuildDate().
3	3		package version
4	4
5	5		// Build information, populated by Set() at startup.

@@ -1,5 +1,5 @@
1		// Package version holds build-time version info for Gordon.	1		// Package version holds build-time version info for Gordon.
2	-	// Set via main using Set(), read from anywhere via Get().	2	+	// Set via main using Set(), read from anywhere via Version(), Commit(), or BuildDate().
3		package version	3		package version
4			4
5		// Build information, populated by Set() at startup.	5		// Build information, populated by Set() at startup.