diff --git a/ddtrace/tracer/log.go b/ddtrace/tracer/log.go index 9e4148b7f3..c3cf81b1c4 100644 --- a/ddtrace/tracer/log.go +++ b/ddtrace/tracer/log.go @@ -130,7 +130,7 @@ func logStartup(t *tracer) { Env: t.config.env, Service: t.config.serviceName, AgentURL: agentURL, - Debug: t.config.debug, + Debug: t.config.internalConfig.Debug(), AnalyticsEnabled: !math.IsNaN(globalconfig.AnalyticsRate()), SampleRate: fmt.Sprintf("%f", t.rulesSampling.traces.globalRate), SampleRateLimit: "disabled", @@ -155,7 +155,7 @@ func logStartup(t *tracer) { FeatureFlags: featureFlags, PropagationStyleInject: injectorNames, PropagationStyleExtract: extractorNames, - TracingAsTransport: t.config.tracingAsTransport, + TracingAsTransport: t.config.internalConfig.TracingAsTransport(), DogstatsdAddr: t.config.dogstatsdAddr, DataStreamsEnabled: t.config.dataStreamsMonitoringEnabled, } diff --git a/ddtrace/tracer/option.go b/ddtrace/tracer/option.go index d86bcbee79..d80d4cb17c 100644 --- a/ddtrace/tracer/option.go +++ b/ddtrace/tracer/option.go @@ -141,8 +141,8 @@ const ( // config holds the tracer configuration. type config struct { - // debug, when true, writes details to logs. - debug bool + // internalConfig holds a reference to the global configuration singleton. + internalConfig *internalconfig.Config // appsecStartOptions controls the options used when starting appsec features. appsecStartOptions []appsecconfig.StartOption @@ -328,9 +328,6 @@ type config struct { // logDirectory is directory for tracer logs specified by user-setting DD_TRACE_LOG_DIRECTORY. default empty/unused logDirectory string - // tracingAsTransport specifies whether the tracer is running in transport-only mode, where traces are only sent when other products request it. - tracingAsTransport bool - // traceRateLimitPerSecond specifies the rate limit for traces. traceRateLimitPerSecond float64 @@ -379,7 +376,7 @@ const partialFlushMinSpansDefault = 1000 // and passed user opts. func newConfig(opts ...StartOption) (*config, error) { c := new(config) - internalConfig := internalconfig.Get() + c.internalConfig = internalconfig.Get() // If this was built with a recent-enough version of Orchestrion, force the orchestrion config to // the baked-in values. We do this early so that opts can be used to override the baked-in values, @@ -482,7 +479,6 @@ func newConfig(opts ...StartOption) (*config, error) { c.logStartup = internal.BoolEnv("DD_TRACE_STARTUP_LOGS", true) c.runtimeMetrics = internal.BoolVal(getDDorOtelConfig("metrics"), false) c.runtimeMetricsV2 = internal.BoolEnv("DD_RUNTIME_METRICS_V2_ENABLED", true) - c.debug = internalConfig.Debug() c.logDirectory = env.Get("DD_TRACE_LOG_DIRECTORY") c.enabled = newDynamicConfig("tracing_enabled", internal.BoolVal(getDDorOtelConfig("enabled"), true), func(_ bool) bool { return true }, equal[bool]) if _, ok := env.Lookup("DD_TRACE_ENABLED"); ok { @@ -613,7 +609,7 @@ func newConfig(opts ...StartOption) (*config, error) { if c.logger != nil { log.UseLogger(c.logger) } - if c.debug { + if c.internalConfig.Debug() { log.SetLevel(log.LevelDebug) } // Check if CI Visibility mode is enabled @@ -692,7 +688,7 @@ func apmTracingDisabled(c *config) { // to let the backend know that it needs to keep APM UI disabled. c.globalSampleRate = 1.0 c.traceRateLimitPerSecond = 1.0 / 60 - c.tracingAsTransport = true + c.internalConfig.SetTracingAsTransport(true) WithGlobalTag("_dd.apm.enabled", 0)(c) // Disable runtime metrics. In `tracingAsTransport` mode, we'll still // tell the agent we computed them, so it doesn't do it either. @@ -1009,8 +1005,7 @@ func WithDebugStack(enabled bool) StartOption { // WithDebugMode enables debug mode on the tracer, resulting in more verbose logging. func WithDebugMode(enabled bool) StartOption { return func(c *config) { - telemetry.RegisterAppConfig("trace_debug_enabled", enabled, telemetry.OriginCode) - c.debug = enabled + c.internalConfig.SetDebug(enabled, telemetry.OriginCode) } } diff --git a/ddtrace/tracer/option_test.go b/ddtrace/tracer/option_test.go index 2c74ed2f12..b08360eca4 100644 --- a/ddtrace/tracer/option_test.go +++ b/ddtrace/tracer/option_test.go @@ -26,7 +26,6 @@ import ( "github.com/DataDog/dd-trace-go/v2/ddtrace/ext" "github.com/DataDog/dd-trace-go/v2/internal" - internalconfig "github.com/DataDog/dd-trace-go/v2/internal/config" "github.com/DataDog/dd-trace-go/v2/internal/globalconfig" "github.com/DataDog/dd-trace-go/v2/internal/log" "github.com/DataDog/dd-trace-go/v2/internal/telemetry" @@ -375,7 +374,7 @@ func TestTracerOptionsDefaults(t *testing.T) { assert.Equal(x.Timeout, y.Timeout) compareHTTPClients(t, x, y) assert.True(getFuncName(x.Transport.(*http.Transport).DialContext) == getFuncName(internal.DefaultDialer(30*time.Second).DialContext)) - assert.False(c.debug) + assert.False(c.internalConfig.Debug()) }) t.Run("http-client", func(t *testing.T) { @@ -430,48 +429,43 @@ func TestTracerOptionsDefaults(t *testing.T) { assert.NoError(t, err) defer tracer.Stop() c := tracer.config - assert.True(t, c.debug) + assert.True(t, c.internalConfig.Debug()) }) t.Run("env", func(t *testing.T) { t.Setenv("DD_TRACE_DEBUG", "true") - internalconfig.ResetForTesting() c, err := newTestConfig() assert.NoError(t, err) - assert.True(t, c.debug) + assert.True(t, c.internalConfig.Debug()) }) t.Run("otel-env-debug", func(t *testing.T) { t.Setenv("OTEL_LOG_LEVEL", "debug") - internalconfig.ResetForTesting() c, err := newTestConfig() assert.NoError(t, err) - assert.True(t, c.debug) + assert.True(t, c.internalConfig.Debug()) }) t.Run("otel-env-notdebug", func(t *testing.T) { // any value other than debug, does nothing t.Setenv("OTEL_LOG_LEVEL", "notdebug") - internalconfig.ResetForTesting() c, err := newTestConfig() assert.NoError(t, err) - assert.False(t, c.debug) + assert.False(t, c.internalConfig.Debug()) }) t.Run("override-chain", func(t *testing.T) { assert := assert.New(t) // option override otel t.Setenv("OTEL_LOG_LEVEL", "debug") - internalconfig.ResetForTesting() c, err := newTestConfig(WithDebugMode(false)) assert.NoError(err) - assert.False(c.debug) + assert.False(c.internalConfig.Debug()) // env override otel t.Setenv("DD_TRACE_DEBUG", "false") - internalconfig.ResetForTesting() c, err = newTestConfig() assert.NoError(err) - assert.False(c.debug) + assert.False(c.internalConfig.Debug()) // option override env c, err = newTestConfig(WithDebugMode(true)) assert.NoError(err) - assert.True(c.debug) + assert.True(c.internalConfig.Debug()) }) }) @@ -745,7 +739,7 @@ func TestTracerOptionsDefaults(t *testing.T) { assert.NotNil(c.globalTags.get()) assert.Equal("v", c.globalTags.get()["k"]) assert.Equal("testEnv", c.env) - assert.True(c.debug) + assert.True(c.internalConfig.Debug()) }) t.Run("env-tags", func(t *testing.T) { diff --git a/ddtrace/tracer/telemetry_test.go b/ddtrace/tracer/telemetry_test.go index 6857ca08ce..457aec78c7 100644 --- a/ddtrace/tracer/telemetry_test.go +++ b/ddtrace/tracer/telemetry_test.go @@ -67,7 +67,7 @@ func TestTelemetryEnabled(t *testing.T) { defer globalconfig.SetServiceName("") defer Stop() - telemetrytest.CheckConfig(t, telemetryClient.Configuration, "trace_debug_enabled", true) + telemetrytest.CheckConfig(t, telemetryClient.Configuration, "DD_TRACE_DEBUG", true) telemetrytest.CheckConfig(t, telemetryClient.Configuration, "service", "test-serv") telemetrytest.CheckConfig(t, telemetryClient.Configuration, "env", "test-env") telemetrytest.CheckConfig(t, telemetryClient.Configuration, "runtime_metrics_enabled", true) diff --git a/ddtrace/tracer/tracer.go b/ddtrace/tracer/tracer.go index 33b0541830..1614c5debe 100644 --- a/ddtrace/tracer/tracer.go +++ b/ddtrace/tracer/tracer.go @@ -560,7 +560,7 @@ func (t *tracer) worker(tick <-chan time.Time) { t.statsd.Incr("datadog.tracer.flush_triggered", []string{"reason:invoked"}, 1) t.traceWriter.flush() t.statsd.Flush() - if !t.config.tracingAsTransport { + if !t.config.internalConfig.TracingAsTransport() { t.stats.flushAndSend(time.Now(), withCurrentBucket) } // TODO(x): In reality, the traceWriter.flush() call is not synchronous @@ -905,7 +905,7 @@ func (t *tracer) Inject(ctx *SpanContext, carrier interface{}) error { return nil } - if t.config.tracingAsTransport { + if t.config.internalConfig.TracingAsTransport() { // in tracing as transport mode, only propagate when there is an upstream appsec event if ctx.trace != nil && !globalinternal.VerifyTraceSourceEnabled(ctx.trace.propagatingTag(keyPropagatedTraceSource), globalinternal.ASMTraceSource) { @@ -952,7 +952,7 @@ func (t *tracer) Extract(carrier interface{}) (*SpanContext, error) { return nil, nil } ctx, err := t.config.propagator.Extract(carrier) - if t.config.tracingAsTransport && ctx != nil { + if t.config.internalConfig.TracingAsTransport() && ctx != nil { // in tracing as transport mode, reset upstream sampling decision to make sure we keep 1 trace/minute if ctx.trace != nil && !globalinternal.VerifyTraceSourceEnabled(ctx.trace.propagatingTag(keyPropagatedTraceSource), globalinternal.ASMTraceSource) { @@ -981,7 +981,7 @@ func (t *tracer) TracerConf() TracerConf { EnvTag: t.config.env, VersionTag: t.config.version, ServiceTag: t.config.serviceName, - TracingAsTransport: t.config.tracingAsTransport, + TracingAsTransport: t.config.internalConfig.TracingAsTransport(), isLambdaFunction: t.config.isLambdaFunction, } } diff --git a/ddtrace/tracer/tracer_test.go b/ddtrace/tracer/tracer_test.go index 907af06558..5c1d81cfb0 100644 --- a/ddtrace/tracer/tracer_test.go +++ b/ddtrace/tracer/tracer_test.go @@ -31,6 +31,7 @@ import ( "github.com/DataDog/dd-trace-go/v2/ddtrace/ext" "github.com/DataDog/dd-trace-go/v2/ddtrace/internal/tracerstats" "github.com/DataDog/dd-trace-go/v2/internal" + internalconfig "github.com/DataDog/dd-trace-go/v2/internal/config" "github.com/DataDog/dd-trace-go/v2/internal/globalconfig" "github.com/DataDog/dd-trace-go/v2/internal/log" "github.com/DataDog/dd-trace-go/v2/internal/remoteconfig" @@ -74,6 +75,8 @@ var ( ) func TestMain(m *testing.M) { + internalconfig.SetUseFreshConfig(true) + // defer internalconfig.SetUseFreshConfig(false) if internal.BoolEnv("DD_APPSEC_ENABLED", false) { // things are slower with AppSec; double wait times timeMultiplicator = time.Duration(2) diff --git a/internal/config/config.go b/internal/config/config.go index 01d7450a02..cd4a204e6d 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -9,15 +9,33 @@ import ( "net/url" "sync" "time" + + "github.com/DataDog/dd-trace-go/v2/internal/telemetry" ) var ( - instance *config - configOnce sync.Once + useFreshConfig bool + instance *Config + // mu protects instance and useFreshConfig + mu sync.Mutex +) + +// Origin represents where a configuration value came from. +// Re-exported so callers don't need to import internal/telemetry. +type Origin = telemetry.Origin + +// Re-exported origin constants for common configuration sources +const ( + OriginCode = telemetry.OriginCode + OriginCalculated = telemetry.OriginCalculated ) // Config represents global configuration properties. -type config struct { +// Config instances should be obtained via Get() which always returns a non-nil value. +// Methods on Config assume a non-nil receiver and will panic if called on nil. +type Config struct { + mu sync.RWMutex + // Config fields are protected by the mutex. agentURL *url.URL debug bool logStartup bool @@ -45,12 +63,14 @@ type config struct { ciVisibilityAgentless bool logDirectory string traceRateLimitPerSecond float64 + // tracingAsTransport specifies whether the tracer is running in transport-only mode, where traces are only sent when other products request it. + tracingAsTransport bool } // loadConfig initializes and returns a new config by reading from all configured sources. // This function is NOT thread-safe and should only be called once through Get's sync.Once. -func loadConfig() *config { - cfg := new(config) +func loadConfig() *Config { + cfg := new(Config) // TODO: Use defaults from config json instead of hardcoding them here cfg.agentURL = provider.getURL("DD_TRACE_AGENT_URL", &url.URL{Scheme: "http", Host: "localhost:8126"}) @@ -80,6 +100,7 @@ func loadConfig() *config { cfg.ciVisibilityAgentless = provider.getBool("DD_CIVISIBILITY_AGENTLESS_ENABLED", false) cfg.logDirectory = provider.getString("DD_TRACE_LOG_DIRECTORY", "") cfg.traceRateLimitPerSecond = provider.getFloat("DD_TRACE_RATE_LIMIT", 0.0) + cfg.tracingAsTransport = false // TODO: Report telemetry? return cfg } @@ -88,13 +109,44 @@ func loadConfig() *config { // This function is thread-safe and can be called from multiple goroutines concurrently. // The configuration is lazily initialized on first access using sync.Once, ensuring // loadConfig() is called exactly once even under concurrent access. -func Get() *config { - configOnce.Do(func() { +func Get() *Config { + mu.Lock() + defer mu.Unlock() + if useFreshConfig || instance == nil { instance = loadConfig() - }) + } + return instance } -func (c *config) Debug() bool { +func SetUseFreshConfig(use bool) { + mu.Lock() + defer mu.Unlock() + useFreshConfig = use +} + +func (c *Config) Debug() bool { + c.mu.RLock() + defer c.mu.RUnlock() return c.debug } + +func (c *Config) SetDebug(enabled bool, origin telemetry.Origin) { + c.mu.Lock() + defer c.mu.Unlock() + c.debug = enabled + telemetry.RegisterAppConfig("DD_TRACE_DEBUG", enabled, origin) +} + +func (c *Config) TracingAsTransport() bool { + c.mu.RLock() + defer c.mu.RUnlock() + return c.tracingAsTransport +} + +func (c *Config) SetTracingAsTransport(enabled bool) { + c.mu.Lock() + defer c.mu.Unlock() + c.tracingAsTransport = enabled + // TODO: Report telemetry? +} diff --git a/internal/config/config_test.go b/internal/config/config_test.go new file mode 100644 index 0000000000..9996e258a1 --- /dev/null +++ b/internal/config/config_test.go @@ -0,0 +1,218 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2025 Datadog, Inc. + +package config + +import ( + "sync" + "sync/atomic" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGet(t *testing.T) { + t.Run("returns non-nil config", func(t *testing.T) { + resetGlobalState() + defer resetGlobalState() + + cfg := Get() + assert.NotNil(t, cfg, "Get() should never return nil") + }) + + t.Run("singleton behavior - returns same instance", func(t *testing.T) { + resetGlobalState() + defer resetGlobalState() + + cfg1 := Get() + cfg2 := Get() + cfg3 := Get() + + // All calls should return the same instance + assert.Same(t, cfg1, cfg2, "First and second Get() calls should return the same instance") + assert.Same(t, cfg1, cfg3, "First and third Get() calls should return the same instance") + }) + + t.Run("fresh config flag forces new instance", func(t *testing.T) { + resetGlobalState() + defer resetGlobalState() + + // Get the first instance + cfg1 := Get() + require.NotNil(t, cfg1) + + // Enable fresh config to allow us to create new instances + SetUseFreshConfig(true) + + // Get should now return a new instance + cfg2 := Get() + require.NotNil(t, cfg2) + assert.NotSame(t, cfg1, cfg2, "With useFreshConfig=true, Get() should return a new instance") + + // Another call with useFreshConfig still true should return another new instance + cfg3 := Get() + require.NotNil(t, cfg3) + assert.NotSame(t, cfg2, cfg3, "With useFreshConfig=true, each Get() call should return a new instance") + + // Disable fresh config to allow us to cache the same instance + SetUseFreshConfig(false) + + // Now it should cache the same instance + cfg4 := Get() + cfg5 := Get() + assert.Same(t, cfg4, cfg5, "With useFreshConfig=false, Get() should cache the same instance") + }) + + t.Run("concurrent access is safe", func(t *testing.T) { + resetGlobalState() + defer resetGlobalState() + + const numGoroutines = 100 + var wg sync.WaitGroup + wg.Add(numGoroutines) + + // All goroutines should get a non-nil config + configs := make([]*Config, numGoroutines) + + for i := range numGoroutines { + go func(j int) { + defer wg.Done() + configs[j] = Get() + }(i) + } + + wg.Wait() + + // All configs should be non-nil + for i, cfg := range configs { + assert.NotNil(t, cfg, "Config at index %d should not be nil", i) + } + + // All configs should be the same instance (singleton) + firstConfig := configs[0] + for i, cfg := range configs[1:] { + assert.Same(t, firstConfig, cfg, "Config at index %d should be the same instance", i+1) + } + }) + + t.Run("concurrent access with fresh config", func(t *testing.T) { + resetGlobalState() + defer resetGlobalState() + + // Enable fresh config to allow us to create new instances + SetUseFreshConfig(true) + + const numGoroutines = 50 + var wg sync.WaitGroup + wg.Add(numGoroutines) + + // Track if we get different instances (which is expected with useFreshConfig=true) + var uniqueInstances sync.Map + var configCount atomic.Int32 + + for range numGoroutines { + go func() { + defer wg.Done() + cfg := Get() + require.NotNil(t, cfg, "Get() should not return nil even under concurrent access") + + // Track unique instances + if _, loaded := uniqueInstances.LoadOrStore(cfg, true); !loaded { + configCount.Add(1) + } + }() + } + + wg.Wait() + + // With useFreshConfig=true, we should get multiple different instances + count := configCount.Load() + assert.Greater(t, count, int32(1), "With useFreshConfig=true, should get multiple different instances") + }) + + t.Run("config is properly initialized with values", func(t *testing.T) { + resetGlobalState() + defer resetGlobalState() + + // Set an environment variable to ensure it's loaded + t.Setenv("DD_TRACE_DEBUG", "true") + + cfg := Get() + require.NotNil(t, cfg) + + // Verify that config values are accessible (using the Debug() method) + debug := cfg.Debug() + assert.True(t, debug, "Config should have loaded DD_TRACE_DEBUG=true") + }) + + t.Run("Setter methods update config and maintain thread-safety", func(t *testing.T) { + resetGlobalState() + defer resetGlobalState() + + cfg := Get() + require.NotNil(t, cfg) + + initialDebug := cfg.Debug() + cfg.SetDebug(!initialDebug, "test") + assert.Equal(t, !initialDebug, cfg.Debug(), "Debug setting should have changed") + + // Verify concurrent reads don't panic + const numReaders = 100 + var wg sync.WaitGroup + wg.Add(numReaders) + + for range numReaders { + go func() { + defer wg.Done() + _ = cfg.Debug() + }() + } + + wg.Wait() + }) + + t.Run("SetDebug concurrent with reads is safe", func(t *testing.T) { + resetGlobalState() + defer resetGlobalState() + + cfg := Get() + require.NotNil(t, cfg) + + var wg sync.WaitGroup + const numOperations = 100 + + // Start readers + wg.Add(numOperations) + for range numOperations { + go func() { + defer wg.Done() + _ = cfg.Debug() + }() + } + + // Start writers + wg.Add(numOperations) + for i := range numOperations { + go func(val bool) { + defer wg.Done() + cfg.SetDebug(val, "test") + }(i%2 == 0) + } + + wg.Wait() + + // Should not panic and should have a valid boolean value + finalDebug := cfg.Debug() + assert.IsType(t, true, finalDebug) + }) +} + +// resetGlobalState resets all global singleton state for testing +func resetGlobalState() { + mu = sync.Mutex{} + instance = nil + useFreshConfig = false +} diff --git a/internal/config/config_testing.go b/internal/config/config_testing.go deleted file mode 100644 index a4018aa593..0000000000 --- a/internal/config/config_testing.go +++ /dev/null @@ -1,19 +0,0 @@ -// Unless explicitly stated otherwise all files in this repository are licensed -// under the Apache License Version 2.0. -// This product includes software developed at Datadog (https://www.datadoghq.com/). -// Copyright 2025 Datadog, Inc. - -package config - -import "sync" - -// ResetForTesting resets the global configuration state for testing. -// -// WARNING: This function is intended for use in tests only to reset state between -// test cases. It must not be called concurrently with Get() or other code that -// accesses the global config, as it can cause race conditions and violate the -// singleton initialization guarantee. -func ResetForTesting() { - instance = nil - configOnce = sync.Once{} -} diff --git a/internal/telemetry/api.go b/internal/telemetry/api.go index 870191afc4..c407eb6321 100644 --- a/internal/telemetry/api.go +++ b/internal/telemetry/api.go @@ -61,6 +61,7 @@ const ( OriginRemoteConfig Origin = transport.OriginRemoteConfig OriginLocalStableConfig Origin = transport.OriginLocalStableConfig OriginManagedStableConfig Origin = transport.OriginManagedStableConfig + OriginCalculated Origin = transport.OriginCalculated ) // EmptyID represents the absence of a configuration ID. diff --git a/internal/telemetry/internal/transport/origin.go b/internal/telemetry/internal/transport/origin.go index 3084d12c43..9712db99a8 100644 --- a/internal/telemetry/internal/transport/origin.go +++ b/internal/telemetry/internal/transport/origin.go @@ -16,4 +16,5 @@ const ( OriginRemoteConfig Origin = "remote_config" OriginLocalStableConfig Origin = "local_stable_config" OriginManagedStableConfig Origin = "fleet_stable_config" + OriginCalculated Origin = "calculated" )