Extract OTEL provider initialization out of runServeCmd (#46421)
Extracts the OTEL trace, metric, and log provider setup out of `runServeCmd` and into `initOTELProviders` in a new `cmd/fleet/otel.go`. Same pattern as the prior extractions on this issue (#44929, #45343, #45583, #46166). Side effects (`otel.SetTracerProvider`, `otel.SetMeterProvider`) are preserved inside the extracted function, so runtime behavior is identical. Three unit tests in `cmd/fleet/otel_test.go`: - OTEL disabled (the common production path) returns `(nil, nil, nil)` and never calls `initFatal`. - OTEL enabled without log export returns non-nil trace and meter providers; logger provider stays nil. - Log export enabled returns all three providers non-nil. One honest note on coverage: the four `initFatal` sites inside the function are paranoid wrapping for OTEL SDK constructors that don't dial at construction time, so the error paths are hard to drive in tests without mocking the SDK. The tests above exercise the success paths and the disabled gate, which is the bulk of the realistic flow. This continues the path toward `serve.go` >60% coverage per the discussion on #33370 — `serve.go` is now ~100 lines shorter and the OTEL phase is testable as a unit. Remaining slices per the broader plan: MDM Apple init, datastore init, Redis init. **Related issue:** Refs #33370 # Checklist for submitter - [x] Added/updated automated tests - Changes file: not applicable — internal refactor with no user-visible behavior change <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Centralized OpenTelemetry provider initialization into a single setup path, simplifying startup and shutdown behavior and making observability configuration clearer. * **Tests** * Added unit tests covering disabled/enabled telemetry paths and optional log export, plus cleanup logic to ensure providers are shut down correctly. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/fleetdm/fleet/pull/46421?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -0,0 +1,137 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"context"
|
||||
|
||||
"github.com/fleetdm/fleet/v4/server/config"
|
||||
"github.com/fleetdm/fleet/v4/server/version"
|
||||
"go.opentelemetry.io/otel"
|
||||
"go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc"
|
||||
"go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc"
|
||||
"go.opentelemetry.io/otel/exporters/otlp/otlptrace"
|
||||
"go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc"
|
||||
otelsdklog "go.opentelemetry.io/otel/sdk/log"
|
||||
sdkmetric "go.opentelemetry.io/otel/sdk/metric"
|
||||
"go.opentelemetry.io/otel/sdk/resource"
|
||||
sdktrace "go.opentelemetry.io/otel/sdk/trace"
|
||||
semconv "go.opentelemetry.io/otel/semconv/v1.40.0"
|
||||
)
|
||||
|
||||
// initOTELProviders constructs the OpenTelemetry trace, metric, and (when
|
||||
// log export is enabled) log providers. Returns nil providers when OTEL is
|
||||
// disabled in the configuration. Fatal errors during exporter setup are
|
||||
// reported through initFatal so the server can fail fast at startup.
|
||||
//
|
||||
// As a side effect, the constructed tracer and meter providers are registered
|
||||
// as the OTEL globals via otel.SetTracerProvider and otel.SetMeterProvider —
|
||||
// matching the original inline behavior in runServeCmd.
|
||||
func initOTELProviders(cfg config.FleetConfig, initFatal func(err error, msg string)) (
|
||||
*otelsdklog.LoggerProvider,
|
||||
*sdktrace.TracerProvider,
|
||||
*sdkmetric.MeterProvider,
|
||||
) {
|
||||
if !cfg.OTELEnabled() {
|
||||
return nil, nil, nil
|
||||
}
|
||||
|
||||
// Create shared resource with service identification attributes.
|
||||
// OTEL_SERVICE_NAME and OTEL_RESOURCE_ATTRIBUTES env vars can override
|
||||
// the defaults below.
|
||||
res, err := resource.New(context.Background(),
|
||||
resource.WithSchemaURL(semconv.SchemaURL),
|
||||
resource.WithAttributes(
|
||||
semconv.ServiceName("fleet"),
|
||||
semconv.ServiceVersion(version.Version().Version),
|
||||
),
|
||||
resource.WithFromEnv(),
|
||||
resource.WithTelemetrySDK(),
|
||||
)
|
||||
if err != nil {
|
||||
initFatal(err, "Failed to create OTEL resource")
|
||||
// Returning here makes the function safe even if a caller's
|
||||
// initFatal does not terminate (e.g., tests using a recorder).
|
||||
return nil, nil, nil
|
||||
}
|
||||
|
||||
// Initialize OTEL traces.
|
||||
otlpTraceExporter, err := otlptrace.New(context.Background(), otlptracegrpc.NewClient(
|
||||
otlptracegrpc.WithCompressor("gzip"),
|
||||
))
|
||||
if err != nil {
|
||||
initFatal(err, "Failed to initialize OTEL trace exporter")
|
||||
return nil, nil, nil
|
||||
}
|
||||
// Configure batch span processor with smaller batch size to avoid exceeding
|
||||
// message size limits (4MB default limit).
|
||||
batchSpanProcessor := sdktrace.NewBatchSpanProcessor(otlpTraceExporter,
|
||||
sdktrace.WithMaxExportBatchSize(256), // Reduce from default 512 to 256
|
||||
)
|
||||
tracerProvider := sdktrace.NewTracerProvider(
|
||||
sdktrace.WithResource(res),
|
||||
sdktrace.WithSpanProcessor(batchSpanProcessor),
|
||||
)
|
||||
otel.SetTracerProvider(tracerProvider)
|
||||
|
||||
// Initialize OTEL metrics.
|
||||
metricExporter, err := otlpmetricgrpc.New(context.Background(),
|
||||
otlpmetricgrpc.WithCompressor("gzip"),
|
||||
)
|
||||
if err != nil {
|
||||
initFatal(err, "Failed to initialize OTEL metrics exporter")
|
||||
return nil, nil, nil
|
||||
}
|
||||
|
||||
// Create views to rename otelsql metrics to match what OpenTelemetry Signoz expects.
|
||||
// Reference: https://opentelemetry.io/docs/specs/semconv/db/database-metrics/
|
||||
dbMetricViews := []sdkmetric.View{
|
||||
sdkmetric.NewView(
|
||||
sdkmetric.Instrument{Name: "db.sql.connection.open"},
|
||||
sdkmetric.Stream{Name: "db.client.connection.count"},
|
||||
),
|
||||
sdkmetric.NewView(
|
||||
sdkmetric.Instrument{Name: "db.sql.connection.max_open"},
|
||||
sdkmetric.Stream{Name: "db.client.connection.max"},
|
||||
),
|
||||
sdkmetric.NewView(
|
||||
sdkmetric.Instrument{Name: "db.sql.connection.wait"},
|
||||
sdkmetric.Stream{Name: "db.client.connection.wait_count"},
|
||||
),
|
||||
sdkmetric.NewView(
|
||||
sdkmetric.Instrument{Name: "db.sql.connection.wait_duration"},
|
||||
sdkmetric.Stream{Name: "db.client.connection.wait_time"},
|
||||
),
|
||||
sdkmetric.NewView(
|
||||
sdkmetric.Instrument{Name: "db.sql.connection.closed_max_idle"},
|
||||
sdkmetric.Stream{Name: "db.client.connection.closed.max_idle"},
|
||||
),
|
||||
sdkmetric.NewView(
|
||||
sdkmetric.Instrument{Name: "db.sql.connection.closed_max_idle_time"},
|
||||
sdkmetric.Stream{Name: "db.client.connection.closed.max_idle_time"},
|
||||
),
|
||||
}
|
||||
|
||||
meterProvider := sdkmetric.NewMeterProvider(
|
||||
sdkmetric.WithResource(res),
|
||||
sdkmetric.WithReader(sdkmetric.NewPeriodicReader(metricExporter)),
|
||||
sdkmetric.WithView(dbMetricViews...),
|
||||
)
|
||||
otel.SetMeterProvider(meterProvider)
|
||||
|
||||
// Initialize OTEL logs.
|
||||
var loggerProvider *otelsdklog.LoggerProvider
|
||||
if cfg.Logging.OtelLogsEnabled {
|
||||
logExporter, err := otlploggrpc.New(context.Background(),
|
||||
otlploggrpc.WithCompressor("gzip"),
|
||||
)
|
||||
if err != nil {
|
||||
initFatal(err, "Failed to initialize OTEL log exporter")
|
||||
return nil, nil, nil
|
||||
}
|
||||
loggerProvider = otelsdklog.NewLoggerProvider(
|
||||
otelsdklog.WithResource(res),
|
||||
otelsdklog.WithProcessor(otelsdklog.NewBatchProcessor(logExporter)),
|
||||
)
|
||||
}
|
||||
|
||||
return loggerProvider, tracerProvider, meterProvider
|
||||
}
|
||||
@@ -0,0 +1,85 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/fleetdm/fleet/v4/server/config"
|
||||
otelsdklog "go.opentelemetry.io/otel/sdk/log"
|
||||
sdkmetric "go.opentelemetry.io/otel/sdk/metric"
|
||||
sdktrace "go.opentelemetry.io/otel/sdk/trace"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// shutdownOTELProviders releases the background goroutines and exporters held
|
||||
// by the providers initOTELProviders constructs. A short context timeout keeps
|
||||
// the cleanup fast even when no OTLP collector is listening — the periodic
|
||||
// exporters would otherwise block trying to flush in-flight batches.
|
||||
func shutdownOTELProviders(t *testing.T, lp *otelsdklog.LoggerProvider, tp *sdktrace.TracerProvider, mp *sdkmetric.MeterProvider) {
|
||||
t.Helper()
|
||||
t.Cleanup(func() {
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
|
||||
defer cancel()
|
||||
if tp != nil {
|
||||
_ = tp.Shutdown(ctx)
|
||||
}
|
||||
if mp != nil {
|
||||
_ = mp.Shutdown(ctx)
|
||||
}
|
||||
if lp != nil {
|
||||
_ = lp.Shutdown(ctx)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestInitOTELProviders_DisabledReturnsNilProviders(t *testing.T) {
|
||||
// Default config has OTEL disabled. The init function should be a no-op
|
||||
// and return nil providers without calling initFatal.
|
||||
cfg := config.FleetConfig{}
|
||||
require.False(t, cfg.OTELEnabled(), "precondition: default config must have OTEL disabled")
|
||||
|
||||
called := false
|
||||
lp, tp, mp := initOTELProviders(cfg, func(err error, msg string) { called = true })
|
||||
|
||||
require.False(t, called, "initFatal must not be called when OTEL is disabled")
|
||||
require.Nil(t, lp)
|
||||
require.Nil(t, tp)
|
||||
require.Nil(t, mp)
|
||||
}
|
||||
|
||||
func TestInitOTELProviders_EnabledReturnsTracerAndMeterProviders(t *testing.T) {
|
||||
// When OTEL is enabled but OtelLogsEnabled is false, the trace and meter
|
||||
// providers should be constructed and the logger provider should remain
|
||||
// nil. The OTLP exporter constructors don't dial at construction time, so
|
||||
// this is safe to run without a collector.
|
||||
cfg := config.FleetConfig{
|
||||
Logging: config.LoggingConfig{TracingEnabled: true},
|
||||
}
|
||||
require.True(t, cfg.OTELEnabled(), "precondition: tracing-enabled config must have OTEL enabled")
|
||||
|
||||
called := false
|
||||
lp, tp, mp := initOTELProviders(cfg, func(err error, msg string) { called = true })
|
||||
shutdownOTELProviders(t, lp, tp, mp)
|
||||
|
||||
require.False(t, called, "initFatal must not be called for a healthy enabled config")
|
||||
require.Nil(t, lp, "logger provider should be nil when OtelLogsEnabled is false")
|
||||
require.NotNil(t, tp)
|
||||
require.NotNil(t, mp)
|
||||
}
|
||||
|
||||
func TestInitOTELProviders_LogExportEnabledReturnsLoggerProvider(t *testing.T) {
|
||||
cfg := config.FleetConfig{
|
||||
Logging: config.LoggingConfig{TracingEnabled: true, OtelLogsEnabled: true},
|
||||
}
|
||||
|
||||
called := false
|
||||
lp, tp, mp := initOTELProviders(cfg, func(err error, msg string) { called = true })
|
||||
shutdownOTELProviders(t, lp, tp, mp)
|
||||
|
||||
require.False(t, called)
|
||||
require.NotNil(t, lp, "logger provider should be set when OtelLogsEnabled is true")
|
||||
require.NotNil(t, tp)
|
||||
require.NotNil(t, mp)
|
||||
}
|
||||
+1
-105
@@ -111,16 +111,6 @@ import (
|
||||
"go.elastic.co/apm/module/apmhttp/v2"
|
||||
_ "go.elastic.co/apm/module/apmsql/v2"
|
||||
_ "go.elastic.co/apm/module/apmsql/v2/mysql"
|
||||
"go.opentelemetry.io/otel"
|
||||
"go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc"
|
||||
"go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc"
|
||||
"go.opentelemetry.io/otel/exporters/otlp/otlptrace"
|
||||
"go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc"
|
||||
otelsdklog "go.opentelemetry.io/otel/sdk/log"
|
||||
sdkmetric "go.opentelemetry.io/otel/sdk/metric"
|
||||
"go.opentelemetry.io/otel/sdk/resource"
|
||||
sdktrace "go.opentelemetry.io/otel/sdk/trace"
|
||||
semconv "go.opentelemetry.io/otel/semconv/v1.40.0"
|
||||
"google.golang.org/grpc"
|
||||
_ "google.golang.org/grpc/encoding/gzip" // Because we use gzip compression for OTLP
|
||||
)
|
||||
@@ -192,101 +182,7 @@ func runServeCmd(cmd *cobra.Command, configManager configpkg.Manager, debug, dev
|
||||
config.Logging.Validate(initFatal)
|
||||
|
||||
// Init OTEL providers (traces, metrics, logs)
|
||||
var loggerProvider *otelsdklog.LoggerProvider
|
||||
var tracerProvider *sdktrace.TracerProvider
|
||||
var meterProvider *sdkmetric.MeterProvider
|
||||
if config.OTELEnabled() {
|
||||
// Create shared resource with service identification attributes.
|
||||
// OTEL_SERVICE_NAME and OTEL_RESOURCE_ATTRIBUTES env vars can override
|
||||
// the defaults below.
|
||||
res, err := resource.New(context.Background(),
|
||||
resource.WithSchemaURL(semconv.SchemaURL),
|
||||
resource.WithAttributes(
|
||||
semconv.ServiceName("fleet"),
|
||||
semconv.ServiceVersion(version.Version().Version),
|
||||
),
|
||||
resource.WithFromEnv(),
|
||||
resource.WithTelemetrySDK(),
|
||||
)
|
||||
if err != nil {
|
||||
initFatal(err, "Failed to create OTEL resource")
|
||||
}
|
||||
|
||||
// Initialize OTEL traces
|
||||
otlpTraceExporter, err := otlptrace.New(context.Background(), otlptracegrpc.NewClient(
|
||||
otlptracegrpc.WithCompressor("gzip"),
|
||||
))
|
||||
if err != nil {
|
||||
initFatal(err, "Failed to initialize OTEL trace exporter")
|
||||
}
|
||||
// Configure batch span processor with smaller batch size to avoid exceeding message size limits (4MB default limit)
|
||||
batchSpanProcessor := sdktrace.NewBatchSpanProcessor(otlpTraceExporter,
|
||||
sdktrace.WithMaxExportBatchSize(256), // Reduce from default 512 to 256
|
||||
)
|
||||
tracerProvider = sdktrace.NewTracerProvider(
|
||||
sdktrace.WithResource(res),
|
||||
sdktrace.WithSpanProcessor(batchSpanProcessor),
|
||||
)
|
||||
otel.SetTracerProvider(tracerProvider)
|
||||
|
||||
// Initialize OTEL metrics
|
||||
metricExporter, err := otlpmetricgrpc.New(context.Background(),
|
||||
otlpmetricgrpc.WithCompressor("gzip"),
|
||||
)
|
||||
if err != nil {
|
||||
initFatal(err, "Failed to initialize OTEL metrics exporter")
|
||||
}
|
||||
|
||||
// Create views to rename otelsql metrics to match what OpenTelemetry Signoz expects
|
||||
// Reference: https://opentelemetry.io/docs/specs/semconv/db/database-metrics/
|
||||
dbMetricViews := []sdkmetric.View{
|
||||
sdkmetric.NewView(
|
||||
sdkmetric.Instrument{Name: "db.sql.connection.open"},
|
||||
sdkmetric.Stream{Name: "db.client.connection.count"},
|
||||
),
|
||||
sdkmetric.NewView(
|
||||
sdkmetric.Instrument{Name: "db.sql.connection.max_open"},
|
||||
sdkmetric.Stream{Name: "db.client.connection.max"},
|
||||
),
|
||||
sdkmetric.NewView(
|
||||
sdkmetric.Instrument{Name: "db.sql.connection.wait"},
|
||||
sdkmetric.Stream{Name: "db.client.connection.wait_count"},
|
||||
),
|
||||
sdkmetric.NewView(
|
||||
sdkmetric.Instrument{Name: "db.sql.connection.wait_duration"},
|
||||
sdkmetric.Stream{Name: "db.client.connection.wait_time"},
|
||||
),
|
||||
sdkmetric.NewView(
|
||||
sdkmetric.Instrument{Name: "db.sql.connection.closed_max_idle"},
|
||||
sdkmetric.Stream{Name: "db.client.connection.closed.max_idle"},
|
||||
),
|
||||
sdkmetric.NewView(
|
||||
sdkmetric.Instrument{Name: "db.sql.connection.closed_max_idle_time"},
|
||||
sdkmetric.Stream{Name: "db.client.connection.closed.max_idle_time"},
|
||||
),
|
||||
}
|
||||
|
||||
meterProvider = sdkmetric.NewMeterProvider(
|
||||
sdkmetric.WithResource(res),
|
||||
sdkmetric.WithReader(sdkmetric.NewPeriodicReader(metricExporter)),
|
||||
sdkmetric.WithView(dbMetricViews...),
|
||||
)
|
||||
otel.SetMeterProvider(meterProvider)
|
||||
|
||||
// Initialize OTEL logs
|
||||
if config.Logging.OtelLogsEnabled {
|
||||
logExporter, err := otlploggrpc.New(context.Background(),
|
||||
otlploggrpc.WithCompressor("gzip"),
|
||||
)
|
||||
if err != nil {
|
||||
initFatal(err, "Failed to initialize OTEL log exporter")
|
||||
}
|
||||
loggerProvider = otelsdklog.NewLoggerProvider(
|
||||
otelsdklog.WithResource(res),
|
||||
otelsdklog.WithProcessor(otelsdklog.NewBatchProcessor(logExporter)),
|
||||
)
|
||||
}
|
||||
}
|
||||
loggerProvider, tracerProvider, meterProvider := initOTELProviders(config, initFatal)
|
||||
|
||||
logger := initLogger(config, loggerProvider)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user