diff --git a/ddtrace/tracer/log.go b/ddtrace/tracer/log.go index e311823663..cd05d83096 100644 --- a/ddtrace/tracer/log.go +++ b/ddtrace/tracer/log.go @@ -150,7 +150,7 @@ func logStartup(t *tracer) { Integrations: t.config.integrations, AppSec: appsec.Enabled(), PartialFlushEnabled: t.config.internalConfig.PartialFlushEnabled(), - PartialFlushMinSpans: t.config.partialFlushMinSpans, + PartialFlushMinSpans: t.config.internalConfig.PartialFlushMinSpans(), Orchestrion: t.config.orchestrionCfg, FeatureFlags: featureFlags, PropagationStyleInject: injectorNames, diff --git a/ddtrace/tracer/option.go b/ddtrace/tracer/option.go index 2e7a255d1f..e764c8617d 100644 --- a/ddtrace/tracer/option.go +++ b/ddtrace/tracer/option.go @@ -257,11 +257,6 @@ type config struct { // misconfiguration spanTimeout time.Duration - // partialFlushMinSpans is the number of finished spans in a single trace to trigger a - // partial flush, or 0 if partial flushing is disabled. - // Value from DD_TRACE_PARTIAL_FLUSH_MIN_SPANS, default 1000. - partialFlushMinSpans int - // statsComputationEnabled enables client-side stats computation (aka trace metrics). statsComputationEnabled bool @@ -331,9 +326,6 @@ type StartOption func(*config) // maxPropagatedTagsLength limits the size of DD_TRACE_X_DATADOG_TAGS_MAX_LENGTH to prevent HTTP 413 responses. const maxPropagatedTagsLength = 512 -// partialFlushMinSpansDefault is the default number of spans for partial flushing, if enabled. -const partialFlushMinSpansDefault = 1000 - // newConfig renders the tracer configuration based on defaults, environment variables // and passed user opts. func newConfig(opts ...StartOption) (*config, error) { @@ -418,17 +410,6 @@ func newConfig(opts ...StartOption) (*config, error) { c.spanTimeout = internal.DurationEnv("DD_TRACE_ABANDONED_SPAN_TIMEOUT", 10*time.Minute) } c.statsComputationEnabled = internal.BoolEnv("DD_TRACE_STATS_COMPUTATION_ENABLED", true) - c.partialFlushMinSpans = internal.IntEnv("DD_TRACE_PARTIAL_FLUSH_MIN_SPANS", partialFlushMinSpansDefault) - if c.partialFlushMinSpans <= 0 { - log.Warn("DD_TRACE_PARTIAL_FLUSH_MIN_SPANS=%d is not a valid value, setting to default %d", c.partialFlushMinSpans, partialFlushMinSpansDefault) - c.partialFlushMinSpans = partialFlushMinSpansDefault - } else if c.partialFlushMinSpans >= traceMaxSize { - log.Warn("DD_TRACE_PARTIAL_FLUSH_MIN_SPANS=%d is above the max number of spans that can be kept in memory for a single trace (%d spans), so partial flushing will never trigger, setting to default %d", c.partialFlushMinSpans, traceMaxSize, partialFlushMinSpansDefault) - c.partialFlushMinSpans = partialFlushMinSpansDefault - } - // TODO(partialFlush): consider logging a warning if DD_TRACE_PARTIAL_FLUSH_MIN_SPANS - // is set, but DD_TRACE_PARTIAL_FLUSH_ENABLED is not true. Or just assume it should be enabled - // if it's explicitly set, and don't require both variables to be configured. c.dynamicInstrumentationEnabled, _, _ = stableconfig.Bool("DD_DYNAMIC_INSTRUMENTATION_ENABLED", false) @@ -1273,7 +1254,7 @@ func WithDebugSpansMode(timeout time.Duration) StartOption { func WithPartialFlushing(numSpans int) StartOption { return func(c *config) { c.internalConfig.SetPartialFlushEnabled(true, internalconfig.OriginCode) - c.partialFlushMinSpans = numSpans + c.internalConfig.SetPartialFlushMinSpans(numSpans, internalconfig.OriginCode) } } diff --git a/ddtrace/tracer/option_test.go b/ddtrace/tracer/option_test.go index 2d67c3b132..e9f4cb15a7 100644 --- a/ddtrace/tracer/option_test.go +++ b/ddtrace/tracer/option_test.go @@ -20,12 +20,14 @@ import ( "reflect" "runtime" "runtime/debug" + "strconv" "strings" "testing" "time" "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" @@ -1656,32 +1658,33 @@ func TestHostnameDisabled(t *testing.T) { } func TestPartialFlushing(t *testing.T) { + partialFlushMinSpansDefault := 1000 t.Run("None", func(t *testing.T) { c, err := newTestConfig() assert.NoError(t, err) assert.False(t, c.internalConfig.PartialFlushEnabled()) - assert.Equal(t, partialFlushMinSpansDefault, c.partialFlushMinSpans) + assert.Equal(t, partialFlushMinSpansDefault, c.internalConfig.PartialFlushMinSpans()) }) t.Run("Disabled-DefaultMinSpans", func(t *testing.T) { t.Setenv("DD_TRACE_PARTIAL_FLUSH_ENABLED", "false") c, err := newTestConfig() assert.NoError(t, err) assert.False(t, c.internalConfig.PartialFlushEnabled()) - assert.Equal(t, partialFlushMinSpansDefault, c.partialFlushMinSpans) + assert.Equal(t, partialFlushMinSpansDefault, c.internalConfig.PartialFlushMinSpans()) }) t.Run("Default-SetMinSpans", func(t *testing.T) { t.Setenv("DD_TRACE_PARTIAL_FLUSH_MIN_SPANS", "10") c, err := newTestConfig() assert.NoError(t, err) assert.False(t, c.internalConfig.PartialFlushEnabled()) - assert.Equal(t, 10, c.partialFlushMinSpans) + assert.Equal(t, 10, c.internalConfig.PartialFlushMinSpans()) }) t.Run("Enabled-DefaultMinSpans", func(t *testing.T) { t.Setenv("DD_TRACE_PARTIAL_FLUSH_ENABLED", "true") c, err := newTestConfig() assert.NoError(t, err) assert.True(t, c.internalConfig.PartialFlushEnabled()) - assert.Equal(t, partialFlushMinSpansDefault, c.partialFlushMinSpans) + assert.Equal(t, partialFlushMinSpansDefault, c.internalConfig.PartialFlushMinSpans()) }) t.Run("Enabled-SetMinSpans", func(t *testing.T) { t.Setenv("DD_TRACE_PARTIAL_FLUSH_ENABLED", "true") @@ -1689,7 +1692,7 @@ func TestPartialFlushing(t *testing.T) { c, err := newTestConfig() assert.NoError(t, err) assert.True(t, c.internalConfig.PartialFlushEnabled()) - assert.Equal(t, 10, c.partialFlushMinSpans) + assert.Equal(t, 10, c.internalConfig.PartialFlushMinSpans()) }) t.Run("Enabled-SetMinSpansNegative", func(t *testing.T) { t.Setenv("DD_TRACE_PARTIAL_FLUSH_ENABLED", "true") @@ -1697,14 +1700,30 @@ func TestPartialFlushing(t *testing.T) { c, err := newTestConfig() assert.NoError(t, err) assert.True(t, c.internalConfig.PartialFlushEnabled()) - assert.Equal(t, partialFlushMinSpansDefault, c.partialFlushMinSpans) + assert.Equal(t, partialFlushMinSpansDefault, c.internalConfig.PartialFlushMinSpans()) + }) + t.Run("Enabled-SetMinSpansAboveMax", func(t *testing.T) { + t.Setenv("DD_TRACE_PARTIAL_FLUSH_ENABLED", "true") + t.Setenv("DD_TRACE_PARTIAL_FLUSH_MIN_SPANS", strconv.Itoa(internalconfig.TraceMaxSize)) + c, err := newTestConfig() + assert.NoError(t, err) + assert.True(t, c.internalConfig.PartialFlushEnabled()) + assert.Equal(t, partialFlushMinSpansDefault, c.internalConfig.PartialFlushMinSpans()) + }) + t.Run("Enabled-SetMinSpans0", func(t *testing.T) { + t.Setenv("DD_TRACE_PARTIAL_FLUSH_ENABLED", "true") + t.Setenv("DD_TRACE_PARTIAL_FLUSH_MIN_SPANS", "0") + c, err := newTestConfig() + assert.NoError(t, err) + assert.True(t, c.internalConfig.PartialFlushEnabled()) + assert.Equal(t, partialFlushMinSpansDefault, c.internalConfig.PartialFlushMinSpans()) }) t.Run("WithPartialFlushOption", func(t *testing.T) { c, err := newTestConfig() assert.NoError(t, err) WithPartialFlushing(20)(c) assert.True(t, c.internalConfig.PartialFlushEnabled()) - assert.Equal(t, 20, c.partialFlushMinSpans) + assert.Equal(t, 20, c.internalConfig.PartialFlushMinSpans()) }) } diff --git a/ddtrace/tracer/spancontext.go b/ddtrace/tracer/spancontext.go index 954fef911d..9daf243e17 100644 --- a/ddtrace/tracer/spancontext.go +++ b/ddtrace/tracer/spancontext.go @@ -19,6 +19,7 @@ import ( "github.com/DataDog/dd-trace-go/v2/ddtrace/ext" "github.com/DataDog/dd-trace-go/v2/ddtrace/internal/tracerstats" sharedinternal "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/log" "github.com/DataDog/dd-trace-go/v2/internal/samplernames" "github.com/DataDog/dd-trace-go/v2/internal/telemetry" @@ -372,12 +373,7 @@ var ( // reasonable as span is actually way bigger, and avoids re-allocating // over and over. Could be fine-tuned at runtime. traceStartSize = 10 - // traceMaxSize is the maximum number of spans we keep in memory for a - // single trace. This is to avoid memory leaks. If more spans than this - // are added to a trace, then the trace is dropped and the spans are - // discarded. Adding additional spans after a trace is dropped does - // nothing. - traceMaxSize = int(1e5) + traceMaxSize = internalconfig.TraceMaxSize ) // newTrace creates a new trace using the given callback which will be called diff --git a/ddtrace/tracer/tracer.go b/ddtrace/tracer/tracer.go index 6d1c96bd26..b1aaac9d3d 100644 --- a/ddtrace/tracer/tracer.go +++ b/ddtrace/tracer/tracer.go @@ -975,7 +975,7 @@ func (t *tracer) TracerConf() TracerConf { DebugAbandonedSpans: t.config.debugAbandonedSpans, Disabled: !t.config.enabled.current, PartialFlush: t.config.internalConfig.PartialFlushEnabled(), - PartialFlushMinSpans: t.config.partialFlushMinSpans, + PartialFlushMinSpans: t.config.internalConfig.PartialFlushMinSpans(), PeerServiceDefaults: t.config.peerServiceDefaultsEnabled, PeerServiceMappings: t.config.peerServiceMappings, EnvTag: t.config.env, diff --git a/internal/config/config.go b/internal/config/config.go index 505459e54f..aaf2ac8fb7 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -100,7 +100,7 @@ func loadConfig() *Config { cfg.peerServiceMappings = provider.getMap("DD_TRACE_PEER_SERVICE_MAPPING", nil) cfg.debugAbandonedSpans = provider.getBool("DD_TRACE_DEBUG_ABANDONED_SPANS", false) cfg.spanTimeout = provider.getDuration("DD_TRACE_ABANDONED_SPAN_TIMEOUT", 0) - cfg.partialFlushMinSpans = provider.getInt("DD_TRACE_PARTIAL_FLUSH_MIN_SPANS", 0) + cfg.partialFlushMinSpans = provider.getIntWithValidator("DD_TRACE_PARTIAL_FLUSH_MIN_SPANS", 1000, validatePartialFlushMinSpans) cfg.partialFlushEnabled = provider.getBool("DD_TRACE_PARTIAL_FLUSH_ENABLED", false) cfg.statsComputationEnabled = provider.getBool("DD_TRACE_STATS_COMPUTATION_ENABLED", false) cfg.dataStreamsMonitoringEnabled = provider.getBool("DD_DATA_STREAMS_ENABLED", false) @@ -299,3 +299,16 @@ func (c *Config) SetPartialFlushEnabled(enabled bool, origin telemetry.Origin) { c.partialFlushEnabled = enabled telemetry.RegisterAppConfig("DD_TRACE_PARTIAL_FLUSH_ENABLED", enabled, origin) } + +func (c *Config) PartialFlushMinSpans() int { + c.mu.RLock() + defer c.mu.RUnlock() + return c.partialFlushMinSpans +} + +func (c *Config) SetPartialFlushMinSpans(minSpans int, origin telemetry.Origin) { + c.mu.Lock() + defer c.mu.Unlock() + c.partialFlushMinSpans = minSpans + telemetry.RegisterAppConfig("DD_TRACE_PARTIAL_FLUSH_MIN_SPANS", minSpans, origin) +} diff --git a/internal/config/config_helpers.go b/internal/config/config_helpers.go index 715be6f667..862c9f4cb1 100644 --- a/internal/config/config_helpers.go +++ b/internal/config/config_helpers.go @@ -9,7 +9,14 @@ import "github.com/DataDog/dd-trace-go/v2/internal/log" const ( // DefaultRateLimit specifies the default rate limit per second for traces. + // TODO: Maybe delete this. We will have defaults in supported_configuration.json anyway. DefaultRateLimit = 100.0 + // traceMaxSize is the maximum number of spans we keep in memory for a + // single trace. This is to avoid memory leaks. If more spans than this + // are added to a trace, then the trace is dropped and the spans are + // discarded. Adding additional spans after a trace is dropped does + // nothing. + TraceMaxSize = int(1e5) ) func validateSampleRate(rate float64) bool { @@ -27,3 +34,15 @@ func validateRateLimit(rate float64) bool { } return true } + +func validatePartialFlushMinSpans(minSpans int) bool { + if minSpans <= 0 { + log.Warn("ignoring DD_TRACE_PARTIAL_FLUSH_MIN_SPANS: negative value %d", minSpans) + return false + } + if minSpans >= TraceMaxSize { + log.Warn("ignoring DD_TRACE_PARTIAL_FLUSH_MIN_SPANS: value %d is greater than the max number of spans that can be kept in memory for a single trace (%d spans)", minSpans, TraceMaxSize) + return false + } + return true +} diff --git a/internal/config/configprovider.go b/internal/config/configprovider.go index 51fe4b937e..8d4869cc25 100644 --- a/internal/config/configprovider.go +++ b/internal/config/configprovider.go @@ -83,6 +83,19 @@ func (p *configProvider) getInt(key string, def int) int { }) } +func (p *configProvider) getIntWithValidator(key string, def int, validate func(int) bool) int { + return get(p, key, def, func(v string) (int, bool) { + intVal, err := strconv.Atoi(v) + if err == nil { + if validate != nil && !validate(intVal) { + return 0, false + } + return intVal, true + } + return 0, false + }) +} + func (p *configProvider) getMap(key string, def map[string]string) map[string]string { return get(p, key, def, func(v string) (map[string]string, bool) { m := parseMapString(v)