Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ddtrace/tracer/abandonedspans_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ func TestReportAbandonedSpans(t *testing.T) {
tracer, _, _, stop, err := startTestTracer(t, WithLogger(tp), WithDebugSpansMode(100*time.Millisecond))
assert.Nil(err)
defer stop()
assert.True(tracer.config.debugAbandonedSpans)
assert.True(tracer.config.internalConfig.DebugAbandonedSpans())
assert.Equal(tracer.config.spanTimeout, 100*time.Millisecond)
})

Expand Down Expand Up @@ -350,7 +350,7 @@ func TestDebugAbandonedSpansOff(t *testing.T) {

t.Run("default", func(t *testing.T) {
assert := assert.New(t)
assert.False(tracer.config.debugAbandonedSpans)
assert.False(tracer.config.internalConfig.DebugAbandonedSpans())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(For here and every other place) is it ever possible for internalConfig on the tracer.config to be nil? Or for config itself to be nil?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible, yes, if tracer has not initialized internalConfig via the Get() function, but Config is designed with the expectations that callers must use Get() to get the instance before calling getters/setters.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The alternative is to make every getter/setter on Config check if the receiver c is not nil, but I think it's more standard Golang to design under the assumptions described above.

assert.Equal(time.Duration(0), tracer.config.spanTimeout)
expected := "Abandoned spans logs enabled."
s := tracer.StartSpan("operation", StartTime(spanStartTime))
Expand Down
2 changes: 1 addition & 1 deletion ddtrace/tracer/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
20 changes: 7 additions & 13 deletions ddtrace/tracer/option.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -274,9 +274,6 @@ type config struct {
// peerServiceMappings holds a set of service mappings to dynamically rename peer.service values.
peerServiceMappings map[string]string

// debugAbandonedSpans controls if the tracer should log when old, open spans are found
debugAbandonedSpans bool

// spanTimeout represents how old a span can be before it should be logged as a possible
// misconfiguration
spanTimeout time.Duration
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand All @@ -497,8 +493,7 @@ func newConfig(opts ...StartOption) (*config, error) {
log.Warn("ignoring DD_TRACE_CLIENT_HOSTNAME_COMPAT, invalid version %q", compatMode)
}
}
c.debugAbandonedSpans = internal.BoolEnv("DD_TRACE_DEBUG_ABANDONED_SPANS", false)
if c.debugAbandonedSpans {
if c.internalConfig.DebugAbandonedSpans() {
c.spanTimeout = internal.DurationEnv("DD_TRACE_ABANDONED_SPAN_TIMEOUT", 10*time.Minute)
}
c.statsComputationEnabled = internal.BoolEnv("DD_TRACE_STATS_COMPUTATION_ENABLED", true)
Expand Down Expand Up @@ -613,7 +608,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
Expand Down Expand Up @@ -1009,8 +1004,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)
}
}

Expand Down Expand Up @@ -1345,7 +1339,7 @@ func WithProfilerEndpoints(enabled bool) StartOption {
// be expensive, so it should only be enabled for debugging purposes.
func WithDebugSpansMode(timeout time.Duration) StartOption {
return func(c *config) {
c.debugAbandonedSpans = true
c.internalConfig.SetDebugAbandonedSpans(true, telemetry.OriginCode)
c.spanTimeout = timeout
}
}
Expand Down
32 changes: 13 additions & 19 deletions ddtrace/tracer/option_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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())
})
})

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -885,15 +879,15 @@ func TestTracerOptionsDefaults(t *testing.T) {
t.Run("defaults", func(t *testing.T) {
c, err := newTestConfig(WithAgentTimeout(2))
assert.NoError(t, err)
assert.Equal(t, false, c.debugAbandonedSpans)
assert.Equal(t, false, c.internalConfig.DebugAbandonedSpans())
assert.Equal(t, time.Duration(0), c.spanTimeout)
})

t.Run("debug-on", func(t *testing.T) {
t.Setenv("DD_TRACE_DEBUG_ABANDONED_SPANS", "true")
c, err := newTestConfig(WithAgentTimeout(2))
assert.NoError(t, err)
assert.Equal(t, true, c.debugAbandonedSpans)
assert.Equal(t, true, c.internalConfig.DebugAbandonedSpans())
assert.Equal(t, 10*time.Minute, c.spanTimeout)
})

Expand All @@ -902,15 +896,15 @@ func TestTracerOptionsDefaults(t *testing.T) {
t.Setenv("DD_TRACE_ABANDONED_SPAN_TIMEOUT", fmt.Sprint(time.Minute))
c, err := newTestConfig(WithAgentTimeout(2))
assert.NoError(t, err)
assert.Equal(t, true, c.debugAbandonedSpans)
assert.Equal(t, true, c.internalConfig.DebugAbandonedSpans())
assert.Equal(t, time.Minute, c.spanTimeout)
})

t.Run("with-function", func(t *testing.T) {
c, err := newTestConfig(WithAgentTimeout(2))
assert.NoError(t, err)
WithDebugSpansMode(time.Second)(c)
assert.Equal(t, true, c.debugAbandonedSpans)
assert.Equal(t, true, c.internalConfig.DebugAbandonedSpans())
assert.Equal(t, time.Second, c.spanTimeout)
})
})
Expand Down
2 changes: 1 addition & 1 deletion ddtrace/tracer/span.go
Original file line number Diff line number Diff line change
Expand Up @@ -746,7 +746,7 @@ func (s *Span) finish(finishTime int64) {
// the agent supports dropping p0's in the client
keep = shouldKeep(s)
}
if tracer.config.debugAbandonedSpans {
if tracer.config.internalConfig.DebugAbandonedSpans() {
// the tracer supports debugging abandoned spans
tracer.submitAbandonedSpan(s, true)
}
Expand Down
2 changes: 1 addition & 1 deletion ddtrace/tracer/telemetry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions ddtrace/tracer/tracer.go
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ func newTracer(opts ...StartOption) (*tracer, error) {
t.reportRuntimeMetrics(defaultMetricsReportInterval)
}()
}
if c.debugAbandonedSpans {
if c.internalConfig.DebugAbandonedSpans() {
log.Info("Abandoned spans logs enabled.")
t.abandonedSpansDebugger = newAbandonedSpansDebugger()
t.abandonedSpansDebugger.Start(t.config.spanTimeout)
Expand Down Expand Up @@ -806,7 +806,7 @@ func (t *tracer) StartSpan(operationName string, options ...StartSpanOption) *Sp
} else {
span.pprofCtxRestore = nil
}
if t.config.debugAbandonedSpans {
if t.config.internalConfig.DebugAbandonedSpans() {
select {
case t.abandonedSpansDebugger.In <- newAbandonedSpanCandidate(span, false):
// ok
Expand Down Expand Up @@ -972,7 +972,7 @@ func (t *tracer) TracerConf() TracerConf {
return TracerConf{
CanComputeStats: t.config.canComputeStats(),
CanDropP0s: t.config.canDropP0s(),
DebugAbandonedSpans: t.config.debugAbandonedSpans,
DebugAbandonedSpans: t.config.internalConfig.DebugAbandonedSpans(),
Disabled: !t.config.enabled.current,
PartialFlush: t.config.partialFlushEnabled,
PartialFlushMinSpans: t.config.partialFlushMinSpans,
Expand Down
3 changes: 3 additions & 0 deletions ddtrace/tracer/tracer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
61 changes: 51 additions & 10 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,24 @@ package config
import (
"net/url"
"sync"
"sync/atomic"
"time"

"github.com/DataDog/dd-trace-go/v2/internal/telemetry"
)

var (
instance *config
configOnce sync.Once
useFreshConfig atomic.Bool
instance atomic.Value
once sync.Once
)

// 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
Expand Down Expand Up @@ -49,8 +57,8 @@ type config struct {

// 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"})
Expand Down Expand Up @@ -88,13 +96,46 @@ 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() {
instance = loadConfig()
func Get() *Config {
if useFreshConfig.Load() {
cfg := loadConfig()
instance.Store(cfg)
return cfg
}

once.Do(func() {
cfg := loadConfig()
instance.Store(cfg)
})
return instance
return instance.Load().(*Config)
}

func (c *config) Debug() bool {
func SetUseFreshConfig(use bool) {
useFreshConfig.Store(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) DebugAbandonedSpans() bool {
c.mu.RLock()
defer c.mu.RUnlock()
return c.debugAbandonedSpans
}

func (c *Config) SetDebugAbandonedSpans(enabled bool, origin telemetry.Origin) {
c.mu.Lock()
defer c.mu.Unlock()
c.debugAbandonedSpans = enabled
telemetry.RegisterAppConfig("DD_TRACE_DEBUG_ABANDONED_SPANS", enabled, origin)
}
Loading
Loading