From 522fd00b2faee83dd9fe651a7b8ce3904d8348ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Thu, 12 Dec 2024 10:15:04 +0100 Subject: [PATCH 1/7] chore(go.mod): go get github.com/darccio/knobs --- go.mod | 1 + go.sum | 2 ++ internal/apps/go.mod | 1 + internal/apps/go.sum | 2 ++ internal/exectracetest/go.mod | 1 + internal/exectracetest/go.sum | 2 ++ 6 files changed, 9 insertions(+) diff --git a/go.mod b/go.mod index 6696dcc4cc..83296dcc92 100644 --- a/go.mod +++ b/go.mod @@ -32,6 +32,7 @@ require ( github.com/bradfitz/gomemcache v0.0.0-20230611145640-acc696258285 github.com/confluentinc/confluent-kafka-go v1.9.2 github.com/confluentinc/confluent-kafka-go/v2 v2.2.0 + github.com/darccio/knobs v0.0.0-20241211164423-8ba351cb0c0c github.com/denisenkom/go-mssqldb v0.11.0 github.com/dimfeld/httptreemux/v5 v5.5.0 github.com/eapache/queue/v2 v2.0.0-20230407133247-75960ed334e4 diff --git a/go.sum b/go.sum index b05429f344..d454e3af61 100644 --- a/go.sum +++ b/go.sum @@ -1032,6 +1032,8 @@ github.com/d2g/dhcp4 v0.0.0-20170904100407-a1d1b6c41b1c/go.mod h1:Ct2BUK8SB0YC1S github.com/d2g/dhcp4client v1.0.0/go.mod h1:j0hNfjhrt2SxUOw55nL0ATM/z4Yt3t2Kd1mW34z5W5s= github.com/d2g/dhcp4server v0.0.0-20181031114812-7d4a0a7f59a5/go.mod h1:Eo87+Kg/IX2hfWJfwxMzLyuSZyxSoAug2nGa1G2QAi8= github.com/d2g/hardwareaddr v0.0.0-20190221164911-e7d9fbe030e4/go.mod h1:bMl4RjIciD2oAxI7DmWRx6gbeqrkoLqv3MV0vzNad+I= +github.com/darccio/knobs v0.0.0-20241211164423-8ba351cb0c0c h1:3h9o1zuYKNAyi5vpoZlFmmckCXsT+w/VB/PWDtz20EA= +github.com/darccio/knobs v0.0.0-20241211164423-8ba351cb0c0c/go.mod h1:nJ3ntDIyIQgqzTWF2QiL7eRNTy6iZJbBot3WcB+SymI= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= diff --git a/internal/apps/go.mod b/internal/apps/go.mod index 9ce5485a85..196f23c32a 100644 --- a/internal/apps/go.mod +++ b/internal/apps/go.mod @@ -18,6 +18,7 @@ require ( github.com/DataDog/go-sqllexer v0.0.14 // indirect github.com/DataDog/opentelemetry-mapping-go/pkg/otlp/attributes v0.20.0 // indirect github.com/cihub/seelog v0.0.0-20170130134532-f561c5e57575 // indirect + github.com/darccio/knobs v0.0.0-20241211164423-8ba351cb0c0c // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/eapache/queue/v2 v2.0.0-20230407133247-75960ed334e4 // indirect github.com/ebitengine/purego v0.6.0-alpha.5 // indirect diff --git a/internal/apps/go.sum b/internal/apps/go.sum index a591990f7b..e6fd4dcfaa 100644 --- a/internal/apps/go.sum +++ b/internal/apps/go.sum @@ -40,6 +40,8 @@ github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UF github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/cihub/seelog v0.0.0-20170130134532-f561c5e57575 h1:kHaBemcxl8o/pQ5VM1c8PVE1PubbNx3mjUr09OqWGCs= github.com/cihub/seelog v0.0.0-20170130134532-f561c5e57575/go.mod h1:9d6lWj8KzO/fd/NrVaLscBKmPigpZpn5YawRPw+e3Yo= +github.com/darccio/knobs v0.0.0-20241211164423-8ba351cb0c0c h1:3h9o1zuYKNAyi5vpoZlFmmckCXsT+w/VB/PWDtz20EA= +github.com/darccio/knobs v0.0.0-20241211164423-8ba351cb0c0c/go.mod h1:nJ3ntDIyIQgqzTWF2QiL7eRNTy6iZJbBot3WcB+SymI= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= diff --git a/internal/exectracetest/go.mod b/internal/exectracetest/go.mod index 0bc1db3393..a8969ff06d 100644 --- a/internal/exectracetest/go.mod +++ b/internal/exectracetest/go.mod @@ -27,6 +27,7 @@ require ( github.com/Microsoft/go-winio v0.6.1 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/cihub/seelog v0.0.0-20170130134532-f561c5e57575 // indirect + github.com/darccio/knobs v0.0.0-20241211164423-8ba351cb0c0c // indirect github.com/dustin/go-humanize v1.0.1 // indirect github.com/eapache/queue/v2 v2.0.0-20230407133247-75960ed334e4 // indirect github.com/ebitengine/purego v0.6.0-alpha.5 // indirect diff --git a/internal/exectracetest/go.sum b/internal/exectracetest/go.sum index 0d8bd345c4..1b1c0e0063 100644 --- a/internal/exectracetest/go.sum +++ b/internal/exectracetest/go.sum @@ -40,6 +40,8 @@ github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UF github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/cihub/seelog v0.0.0-20170130134532-f561c5e57575 h1:kHaBemcxl8o/pQ5VM1c8PVE1PubbNx3mjUr09OqWGCs= github.com/cihub/seelog v0.0.0-20170130134532-f561c5e57575/go.mod h1:9d6lWj8KzO/fd/NrVaLscBKmPigpZpn5YawRPw+e3Yo= +github.com/darccio/knobs v0.0.0-20241211164423-8ba351cb0c0c h1:3h9o1zuYKNAyi5vpoZlFmmckCXsT+w/VB/PWDtz20EA= +github.com/darccio/knobs v0.0.0-20241211164423-8ba351cb0c0c/go.mod h1:nJ3ntDIyIQgqzTWF2QiL7eRNTy6iZJbBot3WcB+SymI= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= From 865937df267a0ae0d199b3f5ffc0e2c118c378b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Fri, 13 Dec 2024 14:16:13 +0100 Subject: [PATCH 2/7] chore(go.mod): go get github.com/darccio/knobs --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 83296dcc92..6fc5e06f10 100644 --- a/go.mod +++ b/go.mod @@ -32,7 +32,7 @@ require ( github.com/bradfitz/gomemcache v0.0.0-20230611145640-acc696258285 github.com/confluentinc/confluent-kafka-go v1.9.2 github.com/confluentinc/confluent-kafka-go/v2 v2.2.0 - github.com/darccio/knobs v0.0.0-20241211164423-8ba351cb0c0c + github.com/darccio/knobs v0.0.0-20241213130850-25d5d476358d github.com/denisenkom/go-mssqldb v0.11.0 github.com/dimfeld/httptreemux/v5 v5.5.0 github.com/eapache/queue/v2 v2.0.0-20230407133247-75960ed334e4 diff --git a/go.sum b/go.sum index d454e3af61..11e550823d 100644 --- a/go.sum +++ b/go.sum @@ -1032,8 +1032,8 @@ github.com/d2g/dhcp4 v0.0.0-20170904100407-a1d1b6c41b1c/go.mod h1:Ct2BUK8SB0YC1S github.com/d2g/dhcp4client v1.0.0/go.mod h1:j0hNfjhrt2SxUOw55nL0ATM/z4Yt3t2Kd1mW34z5W5s= github.com/d2g/dhcp4server v0.0.0-20181031114812-7d4a0a7f59a5/go.mod h1:Eo87+Kg/IX2hfWJfwxMzLyuSZyxSoAug2nGa1G2QAi8= github.com/d2g/hardwareaddr v0.0.0-20190221164911-e7d9fbe030e4/go.mod h1:bMl4RjIciD2oAxI7DmWRx6gbeqrkoLqv3MV0vzNad+I= -github.com/darccio/knobs v0.0.0-20241211164423-8ba351cb0c0c h1:3h9o1zuYKNAyi5vpoZlFmmckCXsT+w/VB/PWDtz20EA= -github.com/darccio/knobs v0.0.0-20241211164423-8ba351cb0c0c/go.mod h1:nJ3ntDIyIQgqzTWF2QiL7eRNTy6iZJbBot3WcB+SymI= +github.com/darccio/knobs v0.0.0-20241213130850-25d5d476358d h1:L8EQePHnMVEnQglNhfHKwvWEcbSX//GodoksWP6uyB8= +github.com/darccio/knobs v0.0.0-20241213130850-25d5d476358d/go.mod h1:nJ3ntDIyIQgqzTWF2QiL7eRNTy6iZJbBot3WcB+SymI= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= From bc43fca49509846ee7bc255dd8d40d3757265974 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Fri, 13 Dec 2024 14:26:43 +0100 Subject: [PATCH 3/7] poc(ddtrace/tracer): migrate partialFlushMinSpans & partialFlushEnabled --- ddtrace/tracer/log.go | 6 ++-- ddtrace/tracer/option.go | 66 +++++++++++++++++++++-------------- ddtrace/tracer/option_test.go | 29 +++++++-------- ddtrace/tracer/spancontext.go | 4 ++- 4 files changed, 61 insertions(+), 44 deletions(-) diff --git a/ddtrace/tracer/log.go b/ddtrace/tracer/log.go index 5b6e056f28..d55bd8ce33 100644 --- a/ddtrace/tracer/log.go +++ b/ddtrace/tracer/log.go @@ -19,6 +19,8 @@ import ( "gopkg.in/DataDog/dd-trace-go.v1/internal/log" "gopkg.in/DataDog/dd-trace-go.v1/internal/osinfo" "gopkg.in/DataDog/dd-trace-go.v1/internal/version" + + "github.com/darccio/knobs" ) // startupInfo contains various information about the status of the tracer on startup. @@ -141,8 +143,8 @@ func logStartup(t *tracer) { AgentFeatures: t.config.agent, Integrations: t.config.integrations, AppSec: appsec.Enabled(), - PartialFlushEnabled: t.config.partialFlushEnabled, - PartialFlushMinSpans: t.config.partialFlushMinSpans, + PartialFlushEnabled: knobs.GetScope(t.config.Scope, partialFlushEnabled), + PartialFlushMinSpans: knobs.GetScope(t.config.Scope, partialFlushMinSpans), Orchestrion: t.config.orchestrionCfg, FeatureFlags: featureFlags, PropagationStyleInject: injectorNames, diff --git a/ddtrace/tracer/option.go b/ddtrace/tracer/option.go index dbffa546df..c075084513 100644 --- a/ddtrace/tracer/option.go +++ b/ddtrace/tracer/option.go @@ -37,6 +37,7 @@ import ( "gopkg.in/DataDog/dd-trace-go.v1/internal/version" "github.com/DataDog/datadog-go/v5/statsd" + "github.com/darccio/knobs" ) var contribIntegrations = map[string]struct { @@ -115,6 +116,9 @@ var ( // config holds the tracer configuration. type config struct { + *knobs.Scope + + // debug, when true, writes details to logs. debug bool @@ -252,15 +256,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 - - // partialFlushEnabled specifices whether the tracer should enable partial flushing. Value - // from DD_TRACE_PARTIAL_FLUSH_ENABLED, default false. - partialFlushEnabled bool - // statsComputationEnabled enables client-side stats computation (aka trace metrics). statsComputationEnabled bool @@ -297,6 +292,38 @@ type config struct { logDirectory string } +var ( + // partialFlushEnabled specifices whether the tracer should enable partial flushing. Value + // from DD_TRACE_PARTIAL_FLUSH_ENABLED, default false. + partialFlushEnabled = knobs.Register(&knobs.Definition[bool]{ + Default: false, + EnvVars: []knobs.EnvVar{{Key: "DD_TRACE_PARTIAL_FLUSH_ENABLED"}}, + Parse: knobs.ToBool, + }) + + // 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 = knobs.Register(&knobs.Definition[int]{ + // 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. + Default: 1000, + EnvVars: []knobs.EnvVar{{Key: "DD_TRACE_PARTIAL_FLUSH_MIN_SPANS"}}, + Requires: []any{partialFlushEnabled}, + Parse: func(v string) (int, error) { + i, _ := strconv.Atoi(v) + if i <= 0 { + return 0, knobs.ErrInvalidValue + } else if i >= traceMaxSize { + err := fmt.Errorf("value 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", traceMaxSize) + return 0, err + } + return i, nil + }, + }) +) + // orchestrionConfig contains Orchestrion configuration. type orchestrionConfig struct { // Enabled indicates whether this tracer was instanciated via Orchestrion. @@ -318,13 +345,11 @@ 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 { c := new(config) + c.Scope = knobs.NewScope() c.sampler = NewAllSampler() sampleRate := math.NaN() if r := getDDorOtelConfig("sampleRate"); r != "" { @@ -419,19 +444,6 @@ func newConfig(opts ...StartOption) *config { } c.statsComputationEnabled = internal.BoolEnv("DD_TRACE_STATS_COMPUTATION_ENABLED", false) c.dataStreamsMonitoringEnabled = internal.BoolEnv("DD_DATA_STREAMS_ENABLED", false) - c.partialFlushEnabled = internal.BoolEnv("DD_TRACE_PARTIAL_FLUSH_ENABLED", false) - 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 = internal.BoolEnv("DD_DYNAMIC_INSTRUMENTATION_ENABLED", false) schemaVersionStr := os.Getenv("DD_TRACE_SPAN_ATTRIBUTE_SCHEMA") @@ -1192,8 +1204,8 @@ func WithDebugSpansMode(timeout time.Duration) StartOption { // is disabled by default. func WithPartialFlushing(numSpans int) StartOption { return func(c *config) { - c.partialFlushEnabled = true - c.partialFlushMinSpans = numSpans + knobs.SetScope(c.Scope, partialFlushEnabled, knobs.Code, true) + knobs.SetScope(c.Scope, partialFlushMinSpans, knobs.Code, numSpans) } } diff --git a/ddtrace/tracer/option_test.go b/ddtrace/tracer/option_test.go index d915cb45b7..35a150d058 100644 --- a/ddtrace/tracer/option_test.go +++ b/ddtrace/tracer/option_test.go @@ -34,6 +34,7 @@ import ( "gopkg.in/DataDog/dd-trace-go.v1/internal/traceprof" "gopkg.in/DataDog/dd-trace-go.v1/internal/version" + "github.com/darccio/knobs" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -1506,46 +1507,46 @@ func TestHostnameDisabled(t *testing.T) { func TestPartialFlushing(t *testing.T) { t.Run("None", func(t *testing.T) { c := newConfig() - assert.False(t, c.partialFlushEnabled) - assert.Equal(t, partialFlushMinSpansDefault, c.partialFlushMinSpans) + assert.False(t, knobs.GetScope(c.Scope, partialFlushEnabled)) + assert.Equal(t, 1000, knobs.GetScope(c.Scope, partialFlushMinSpans)) }) t.Run("Disabled-DefaultMinSpans", func(t *testing.T) { t.Setenv("DD_TRACE_PARTIAL_FLUSH_ENABLED", "false") c := newConfig() - assert.False(t, c.partialFlushEnabled) - assert.Equal(t, partialFlushMinSpansDefault, c.partialFlushMinSpans) + assert.False(t, knobs.GetScope(c.Scope, partialFlushEnabled)) + assert.Equal(t, 1000, knobs.GetScope(c.Scope, partialFlushMinSpans)) }) t.Run("Default-SetMinSpans", func(t *testing.T) { t.Setenv("DD_TRACE_PARTIAL_FLUSH_MIN_SPANS", "10") c := newConfig() - assert.False(t, c.partialFlushEnabled) - assert.Equal(t, 10, c.partialFlushMinSpans) + assert.False(t, knobs.GetScope(c.Scope, partialFlushEnabled)) + assert.Equal(t, 10, knobs.GetScope(c.Scope, partialFlushMinSpans)) }) t.Run("Enabled-DefaultMinSpans", func(t *testing.T) { t.Setenv("DD_TRACE_PARTIAL_FLUSH_ENABLED", "true") c := newConfig() - assert.True(t, c.partialFlushEnabled) - assert.Equal(t, partialFlushMinSpansDefault, c.partialFlushMinSpans) + assert.True(t, knobs.GetScope(c.Scope, partialFlushEnabled)) + assert.Equal(t, 1000, knobs.GetScope(c.Scope, partialFlushMinSpans)) }) t.Run("Enabled-SetMinSpans", func(t *testing.T) { t.Setenv("DD_TRACE_PARTIAL_FLUSH_ENABLED", "true") t.Setenv("DD_TRACE_PARTIAL_FLUSH_MIN_SPANS", "10") c := newConfig() - assert.True(t, c.partialFlushEnabled) - assert.Equal(t, 10, c.partialFlushMinSpans) + assert.True(t, knobs.GetScope(c.Scope, partialFlushEnabled)) + assert.Equal(t, 10, knobs.GetScope(c.Scope, partialFlushMinSpans)) }) t.Run("Enabled-SetMinSpansNegative", func(t *testing.T) { t.Setenv("DD_TRACE_PARTIAL_FLUSH_ENABLED", "true") t.Setenv("DD_TRACE_PARTIAL_FLUSH_MIN_SPANS", "-1") c := newConfig() - assert.True(t, c.partialFlushEnabled) - assert.Equal(t, partialFlushMinSpansDefault, c.partialFlushMinSpans) + assert.True(t, knobs.GetScope(c.Scope, partialFlushEnabled)) + assert.Equal(t, 1000, knobs.GetScope(c.Scope, partialFlushMinSpans)) }) t.Run("WithPartialFlushOption", func(t *testing.T) { c := newConfig() WithPartialFlushing(20)(c) - assert.True(t, c.partialFlushEnabled) - assert.Equal(t, 20, c.partialFlushMinSpans) + assert.True(t, knobs.GetScope(c.Scope, partialFlushEnabled)) + assert.Equal(t, 20, knobs.GetScope(c.Scope, partialFlushMinSpans)) }) } diff --git a/ddtrace/tracer/spancontext.go b/ddtrace/tracer/spancontext.go index 30a791ee8e..9b0b68d991 100644 --- a/ddtrace/tracer/spancontext.go +++ b/ddtrace/tracer/spancontext.go @@ -23,6 +23,8 @@ import ( "gopkg.in/DataDog/dd-trace-go.v1/internal/log" "gopkg.in/DataDog/dd-trace-go.v1/internal/samplernames" "gopkg.in/DataDog/dd-trace-go.v1/internal/telemetry" + + "github.com/darccio/knobs" ) var _ ddtrace.SpanContext = (*spanContext)(nil) @@ -499,7 +501,7 @@ func (t *trace) finishedOne(s *span) { return } - doPartialFlush := tr.config.partialFlushEnabled && t.finished >= tr.config.partialFlushMinSpans + doPartialFlush := knobs.GetScope(tr.config.Scope, partialFlushEnabled) && t.finished >= knobs.GetScope(tr.config.Scope, partialFlushMinSpans) if !doPartialFlush { return // The trace hasn't completed and partial flushing will not occur } From 2666a59977a384675becf878c1a39b24aea809f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Fri, 13 Dec 2024 14:27:07 +0100 Subject: [PATCH 4/7] fix(internal/telemetry/telemetrytest): initialize MockClient#Metrics map --- internal/telemetry/telemetrytest/telemetrytest.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/telemetry/telemetrytest/telemetrytest.go b/internal/telemetry/telemetrytest/telemetrytest.go index 0c8b22bd32..23dae2e8cf 100644 --- a/internal/telemetry/telemetrytest/telemetrytest.go +++ b/internal/telemetry/telemetrytest/telemetrytest.go @@ -73,6 +73,9 @@ func (c *MockClient) Record(ns telemetry.Namespace, _ telemetry.MetricKind, name c.On("Gauge", ns, name, val, tags, common).Return() c.On("Record", ns, name, val, tags, common).Return() _ = c.Called(ns, name, val, tags, common) + if len(c.Metrics) == 0 { + c.Metrics = make(map[telemetry.Namespace]map[string]float64) + } // record the val for tests that assert based on the value if _, ok := c.Metrics[ns]; !ok { c.Metrics[ns] = map[string]float64{} From 34f612878da520624dffbb50d825c493aa32038e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Fri, 13 Dec 2024 14:32:59 +0100 Subject: [PATCH 5/7] poc(ddtrace/tracer): migrate globalSampleRate --- ddtrace/tracer/option.go | 57 +++++++++++++++++++++--------- ddtrace/tracer/otel_dd_mappings.go | 10 ++++-- ddtrace/tracer/sampler_test.go | 49 ++++++++++++++++++------- ddtrace/tracer/tracer.go | 8 +++-- 4 files changed, 90 insertions(+), 34 deletions(-) diff --git a/ddtrace/tracer/option.go b/ddtrace/tracer/option.go index c075084513..9144f6d2a2 100644 --- a/ddtrace/tracer/option.go +++ b/ddtrace/tracer/option.go @@ -8,6 +8,7 @@ package tracer import ( "context" "encoding/json" + "errors" "fmt" "math" "net" @@ -279,9 +280,6 @@ type config struct { // Value from DD_DYNAMIC_INSTRUMENTATION_ENABLED, default false. dynamicInstrumentationEnabled bool - // globalSampleRate holds sample rate read from environment variables. - globalSampleRate float64 - // ciVisibilityEnabled controls if the tracer is loaded with CI Visibility mode. default false ciVisibilityEnabled bool @@ -293,6 +291,46 @@ type config struct { } var ( + errOutOfRange = errors.New("value out of range") +) + +var ( + // globalSampleRate holds sample rate read from environment variables. + globalSampleRate = knobs.Register(&knobs.Definition[float64]{ + Default: math.NaN(), + + EnvVars: []knobs.EnvVar{ + { + Key: "DD_TRACE_SAMPLE_RATE", + }, + { + Key: "OTEL_TRACES_SAMPLER", + Transform: mapSampleRate, + }, + }, + Resolve: func(environ map[string]string, decision string) (string, error) { + dd := "DD_TRACE_SAMPLE_RATE" + ot := "OTEL_TRACES_SAMPLER" + if _, ok := environ[dd]; ok { + if _, ok := environ[ot]; ok { + log.Warn("Both %s and %s are set, using %v=%v", dd, ot, dd, environ["DD_TRACE_SAMPLE_RATE"]) + reportHidingCount(dd, ot) + } + } + // The chosen environment variable is fine. + return decision, nil + }, + Parse: func(s string) (float64, error) { + if sampleRate, err := strconv.ParseFloat(s, 64); err != nil { + return 0.0, err + } else if sampleRate < 0.0 || sampleRate > 1.0 { + return 0.0, errOutOfRange + } else { + return sampleRate, nil + } + }, + }) + // partialFlushEnabled specifices whether the tracer should enable partial flushing. Value // from DD_TRACE_PARTIAL_FLUSH_ENABLED, default false. partialFlushEnabled = knobs.Register(&knobs.Definition[bool]{ @@ -351,19 +389,6 @@ func newConfig(opts ...StartOption) *config { c := new(config) c.Scope = knobs.NewScope() c.sampler = NewAllSampler() - sampleRate := math.NaN() - if r := getDDorOtelConfig("sampleRate"); r != "" { - var err error - sampleRate, err = strconv.ParseFloat(r, 64) - if err != nil { - log.Warn("ignoring DD_TRACE_SAMPLE_RATE, error: %v", err) - sampleRate = math.NaN() - } else if sampleRate < 0.0 || sampleRate > 1.0 { - log.Warn("ignoring DD_TRACE_SAMPLE_RATE: out of range %f", sampleRate) - sampleRate = math.NaN() - } - } - c.globalSampleRate = sampleRate c.httpClientTimeout = time.Second * 10 // 10 seconds if v := os.Getenv("OTEL_LOGS_EXPORTER"); v != "" { diff --git a/ddtrace/tracer/otel_dd_mappings.go b/ddtrace/tracer/otel_dd_mappings.go index 1b17d86560..7b1a21f032 100644 --- a/ddtrace/tracer/otel_dd_mappings.go +++ b/ddtrace/tracer/otel_dd_mappings.go @@ -93,8 +93,7 @@ func getDDorOtelConfig(configName string) string { otelPrefix := "config_opentelemetry:" if val != "" { log.Warn("Both %v and %v are set, using %v=%v", config.ot, config.dd, config.dd, val) - telemetryTags := []string{ddPrefix + strings.ToLower(config.dd), otelPrefix + strings.ToLower(config.ot)} - telemetry.GlobalClient.Count(telemetry.NamespaceTracers, "otel.env.hiding", 1.0, telemetryTags, true) + reportHidingCount(config.dd, config.ot) } else { v, err := config.remapper(otVal) if err != nil { @@ -108,6 +107,13 @@ func getDDorOtelConfig(configName string) string { return val } +func reportHidingCount(dd, ot string) { + ddPrefix := "config_datadog:" + otelPrefix := "config_opentelemetry:" + telemetryTags := []string{ddPrefix + strings.ToLower(dd), otelPrefix + strings.ToLower(ot)} + telemetry.GlobalClient.Count(telemetry.NamespaceTracers, "otel.env.hiding", 1.0, telemetryTags, true) +} + // mapDDTags maps OTEL_RESOURCE_ATTRIBUTES to DD_TAGS func mapDDTags(ot string) (string, error) { ddTags := make([]string, 0) diff --git a/ddtrace/tracer/sampler_test.go b/ddtrace/tracer/sampler_test.go index 117518b533..3e3473ce9b 100644 --- a/ddtrace/tracer/sampler_test.go +++ b/ddtrace/tracer/sampler_test.go @@ -20,6 +20,7 @@ import ( "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/internal" "gopkg.in/DataDog/dd-trace-go.v1/internal/samplernames" + "github.com/darccio/knobs" "github.com/stretchr/testify/assert" "golang.org/x/time/rate" ) @@ -201,7 +202,8 @@ func TestRuleEnvVars(t *testing.T) { {in: "1point0", out: math.NaN()}, // default if invalid value } { t.Setenv("DD_TRACE_SAMPLE_RATE", tt.in) - res := newConfig().globalSampleRate + c := newConfig() + res := knobs.GetScope(c.Scope, globalSampleRate) if math.IsNaN(tt.out) { assert.True(math.IsNaN(res)) } else { @@ -226,7 +228,9 @@ func TestRuleEnvVars(t *testing.T) { assert := assert.New(t) t.Setenv("OTEL_TRACES_SAMPLER", tt.config) t.Setenv("OTEL_TRACES_SAMPLER_ARG", fmt.Sprintf("%f", tt.rate)) - assert.Equal(tt.rate, newConfig().globalSampleRate) + c := newConfig() + sampleRate := knobs.GetScope(c.Scope, globalSampleRate) + assert.Equal(tt.rate, sampleRate) }) } }) @@ -477,7 +481,9 @@ func TestRulesSampler(t *testing.T) { } t.Run("no-rules", func(t *testing.T) { assert := assert.New(t) - rs := newRulesSampler(nil, nil, newConfig().globalSampleRate) + c := newConfig() + sampleRate := knobs.GetScope(c.Scope, globalSampleRate) + rs := newRulesSampler(nil, nil, sampleRate) span := makeSpan("http.request", "test-service") result := rs.SampleTrace(span) @@ -542,7 +548,9 @@ func TestRulesSampler(t *testing.T) { assert.Nil(t, err) assert := assert.New(t) - rs := newRulesSampler(rules, nil, newConfig().globalSampleRate) + c := newConfig() + sampleRate := knobs.GetScope(c.Scope, globalSampleRate) + rs := newRulesSampler(rules, nil, sampleRate) span := makeFinishedSpan(tt.spanName, tt.spanSrv, tt.spanRsc, tt.spanTags) @@ -566,7 +574,9 @@ func TestRulesSampler(t *testing.T) { for _, v := range traceRules { t.Run("", func(t *testing.T) { assert := assert.New(t) - rs := newRulesSampler(v, nil, newConfig().globalSampleRate) + c := newConfig() + sampleRate := knobs.GetScope(c.Scope, globalSampleRate) + rs := newRulesSampler(v, nil, sampleRate) span := makeSpan("http.request", "test-service") result := rs.SampleTrace(span) @@ -592,7 +602,9 @@ func TestRulesSampler(t *testing.T) { for _, v := range traceRules { t.Run("", func(t *testing.T) { assert := assert.New(t) - rs := newRulesSampler(v, nil, newConfig().globalSampleRate) + c := newConfig() + sampleRate := knobs.GetScope(c.Scope, globalSampleRate) + rs := newRulesSampler(v, nil, sampleRate) span := makeSpan("http.request", "test-service") result := rs.SampleTrace(span) @@ -638,7 +650,9 @@ func TestRulesSampler(t *testing.T) { _, rules, err := samplingRulesFromEnv() assert.Nil(t, err) assert := assert.New(t) - rs := newRulesSampler(nil, rules, newConfig().globalSampleRate) + c := newConfig() + sampleRate := knobs.GetScope(c.Scope, globalSampleRate) + rs := newRulesSampler(nil, rules, sampleRate) span := makeFinishedSpan(tt.spanName, tt.spanSrv, "res-10", map[string]interface{}{"hostname": "hn-30"}) @@ -761,7 +775,8 @@ func TestRulesSampler(t *testing.T) { t.Run(fmt.Sprintf("%v", i), func(t *testing.T) { assert := assert.New(t) c := newConfig(WithSamplingRules(tt.rules)) - rs := newRulesSampler(nil, c.spanRules, newConfig().globalSampleRate) + sampleRate := knobs.GetScope(c.Scope, globalSampleRate) + rs := newRulesSampler(nil, c.spanRules, sampleRate) span := makeFinishedSpan(tt.spanName, tt.spanSrv, "res-10", map[string]interface{}{"hostname": "hn-30", "tag": 20.1, @@ -829,7 +844,8 @@ func TestRulesSampler(t *testing.T) { _, rules, _ := samplingRulesFromEnv() assert := assert.New(t) - sampleRate := newConfig().globalSampleRate + c := newConfig() + sampleRate := knobs.GetScope(c.Scope, globalSampleRate) rs := newRulesSampler(nil, rules, sampleRate) span := makeFinishedSpan(tt.spanName, tt.spanSrv, tt.resName, map[string]interface{}{"hostname": "hn-30"}) @@ -940,7 +956,8 @@ func TestRulesSampler(t *testing.T) { t.Run("", func(t *testing.T) { assert := assert.New(t) c := newConfig(WithSamplingRules(tt.rules)) - rs := newRulesSampler(nil, c.spanRules, newConfig().globalSampleRate) + sampleRate := knobs.GetScope(c.Scope, globalSampleRate) + rs := newRulesSampler(nil, c.spanRules, sampleRate) span := makeFinishedSpan(tt.spanName, tt.spanSrv, "res-10", map[string]interface{}{"hostname": "hn-30", "tag": 20.1, @@ -969,7 +986,9 @@ func TestRulesSampler(t *testing.T) { t.Run("", func(t *testing.T) { assert := assert.New(t) t.Setenv("DD_TRACE_SAMPLE_RATE", fmt.Sprint(rate)) - rs := newRulesSampler(nil, rules, newConfig().globalSampleRate) + c := newConfig() + sampleRate := knobs.GetScope(c.Scope, globalSampleRate) + rs := newRulesSampler(nil, rules, sampleRate) span := makeSpan("http.request", "test-service") result := rs.SampleTrace(span) @@ -1250,7 +1269,9 @@ func TestRulesSamplerInternals(t *testing.T) { t.Run("full-rate", func(t *testing.T) { assert := assert.New(t) now := time.Now() - rs := newRulesSampler(nil, nil, newConfig().globalSampleRate) + c := newConfig() + sampleRate := knobs.GetScope(c.Scope, globalSampleRate) + rs := newRulesSampler(nil, nil, sampleRate) // set samplingLimiter to specific state rs.traces.limiter.prevTime = now.Add(-1 * time.Second) rs.traces.limiter.allowed = 1 @@ -1265,7 +1286,9 @@ func TestRulesSamplerInternals(t *testing.T) { t.Run("limited-rate", func(t *testing.T) { assert := assert.New(t) now := time.Now() - rs := newRulesSampler(nil, nil, newConfig().globalSampleRate) + c := newConfig() + sampleRate := knobs.GetScope(c.Scope, globalSampleRate) + rs := newRulesSampler(nil, nil, sampleRate) // force sampling limiter to 1.0 spans/sec rs.traces.limiter.limiter = rate.NewLimiter(rate.Limit(1.0), 1) rs.traces.limiter.prevTime = now.Add(-1 * time.Second) diff --git a/ddtrace/tracer/tracer.go b/ddtrace/tracer/tracer.go index 89277045de..357ccd4c9c 100644 --- a/ddtrace/tracer/tracer.go +++ b/ddtrace/tracer/tracer.go @@ -34,6 +34,7 @@ import ( "github.com/DataDog/datadog-agent/pkg/obfuscate" "github.com/DataDog/go-runtime-metrics-internal/pkg/runtimemetrics" + "github.com/darccio/knobs" ) var _ ddtrace.Tracer = (*tracer)(nil) @@ -267,13 +268,14 @@ func newUnstartedTracer(opts ...StartOption) *tracer { if spans != nil { c.spanRules = spans } - rulesSampler := newRulesSampler(c.traceRules, c.spanRules, c.globalSampleRate) - c.traceSampleRate = newDynamicConfig("trace_sample_rate", c.globalSampleRate, rulesSampler.traces.setGlobalSampleRate, equal[float64]) + gsr := knobs.GetScope(c.Scope, globalSampleRate) + rulesSampler := newRulesSampler(c.traceRules, c.spanRules, gsr) + c.traceSampleRate = newDynamicConfig("trace_sample_rate", gsr, rulesSampler.traces.setGlobalSampleRate, equal[float64]) // If globalSampleRate returns NaN, it means the environment variable was not set or valid. // We could always set the origin to "env_var" inconditionally, but then it wouldn't be possible // to distinguish between the case where the environment variable was not set and the case where // it default to NaN. - if !math.IsNaN(c.globalSampleRate) { + if !math.IsNaN(gsr) { c.traceSampleRate.cfgOrigin = telemetry.OriginEnvVar } c.traceSampleRules = newDynamicConfig("trace_sample_rules", c.traceRules, From e7f68931305831d8b7e22be97468c5b2cb5899c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Fri, 13 Dec 2024 14:34:30 +0100 Subject: [PATCH 6/7] poc(ddtrace/tracer): migrate enabled (RC) --- ddtrace/tracer/option.go | 36 ++++++++++++++++++++++------ ddtrace/tracer/option_test.go | 25 ++++++++++++------- ddtrace/tracer/remote_config.go | 10 ++++---- ddtrace/tracer/remote_config_test.go | 8 ++++--- ddtrace/tracer/span.go | 3 ++- ddtrace/tracer/telemetry.go | 4 +++- ddtrace/tracer/tracer.go | 8 +++---- 7 files changed, 64 insertions(+), 30 deletions(-) diff --git a/ddtrace/tracer/option.go b/ddtrace/tracer/option.go index 9144f6d2a2..1ad080de72 100644 --- a/ddtrace/tracer/option.go +++ b/ddtrace/tracer/option.go @@ -119,6 +119,10 @@ var ( type config struct { *knobs.Scope + dynamic struct { + // enabled reports whether tracing is enabled. + enabled dynamicConfig[bool] + } // debug, when true, writes details to logs. debug bool @@ -235,9 +239,6 @@ type config struct { // profilerEndpoints specifies whether profiler endpoint filtering is enabled. profilerEndpoints bool - // enabled reports whether tracing is enabled. - enabled dynamicConfig[bool] - // enableHostnameDetection specifies whether the tracer should enable hostname detection. enableHostnameDetection bool @@ -295,6 +296,26 @@ var ( ) var ( + enabled = knobs.Register(&knobs.Definition[bool]{ + Default: true, + EnvVars: []knobs.EnvVar{ + { + Key: "DD_TRACE_ENABLED", + }, + { + Key: "OTEL_TRACES_EXPORTER", + Transform: mapEnabled, + }, + }, + Parse: func(s string) (bool, error) { + if s == "" { + // Short-circuit if the value is empty, to avoid parsing errors. + return true, nil + } + return strconv.ParseBool(s) + }, + }) + // globalSampleRate holds sample rate read from environment variables. globalSampleRate = knobs.Register(&knobs.Definition[float64]{ Default: math.NaN(), @@ -450,9 +471,10 @@ func newConfig(opts ...StartOption) *config { c.runtimeMetricsV2 = internal.BoolEnv("DD_RUNTIME_METRICS_V2_ENABLED", false) c.debug = internal.BoolVal(getDDorOtelConfig("debugMode"), false) c.logDirectory = os.Getenv("DD_TRACE_LOG_DIRECTORY") - c.enabled = newDynamicConfig("tracing_enabled", internal.BoolVal(getDDorOtelConfig("enabled"), true), func(b bool) bool { return true }, equal[bool]) + c.dynamic.enabled = newDynamicConfig("tracing_enabled", knobs.GetScope(c.Scope, enabled), func(b bool) bool { knobs.SetScope(c.Scope, enabled, knobs.Code, b); return true }, equal[bool]) if _, ok := os.LookupEnv("DD_TRACE_ENABLED"); ok { - c.enabled.cfgOrigin = telemetry.OriginEnvVar + // TODO: shouldn't we track this for the OTEL_TRACES_EXPORTER case? + c.dynamic.enabled.cfgOrigin = telemetry.OriginEnvVar } c.profilerEndpoints = internal.BoolEnv(traceprof.EndpointEnvVar, true) c.profilerHotspots = internal.BoolEnv(traceprof.CodeHotspotsEnvVar, true) @@ -567,7 +589,7 @@ func newConfig(opts ...StartOption) *config { log.SetLevel(log.LevelDebug) } // if using stdout or traces are disabled, agent is disabled - agentDisabled := c.logToStdout || !c.enabled.current + agentDisabled := c.logToStdout || !knobs.GetScope(c.Scope, enabled) c.agent = loadAgentFeatures(agentDisabled, c.agentURL, c.httpClient) info, ok := debug.ReadBuildInfo() if !ok { @@ -1169,7 +1191,7 @@ func WithHostname(name string) StartOption { // WithTraceEnabled allows specifying whether tracing will be enabled func WithTraceEnabled(enabled bool) StartOption { return func(c *config) { - c.enabled = newDynamicConfig("tracing_enabled", enabled, func(b bool) bool { return true }, equal[bool]) + c.dynamic.enabled.update(enabled, telemetry.OriginCode) } } diff --git a/ddtrace/tracer/option_test.go b/ddtrace/tracer/option_test.go index 35a150d058..acc3f0d728 100644 --- a/ddtrace/tracer/option_test.go +++ b/ddtrace/tracer/option_test.go @@ -637,8 +637,9 @@ func TestTracerOptionsDefaults(t *testing.T) { tracer := newTracer(WithAgentTimeout(2)) defer tracer.Stop() c := tracer.config - assert.True(t, c.enabled.current) - assert.Equal(t, c.enabled.cfgOrigin, telemetry.OriginDefault) + assert.True(t, c.dynamic.enabled.current) + assert.True(t, knobs.GetScope(c.Scope, enabled)) + assert.Equal(t, c.dynamic.enabled.cfgOrigin, telemetry.OriginDefault) }) t.Run("override", func(t *testing.T) { @@ -646,8 +647,9 @@ func TestTracerOptionsDefaults(t *testing.T) { tracer := newTracer(WithAgentTimeout(2)) defer tracer.Stop() c := tracer.config - assert.False(t, c.enabled.current) - assert.Equal(t, c.enabled.cfgOrigin, telemetry.OriginEnvVar) + assert.False(t, c.dynamic.enabled.current) + assert.False(t, knobs.GetScope(c.Scope, enabled)) + assert.Equal(t, c.dynamic.enabled.cfgOrigin, telemetry.OriginEnvVar) }) }) @@ -1365,21 +1367,24 @@ func TestWithTraceEnabled(t *testing.T) { t.Run("WithTraceEnabled", func(t *testing.T) { assert := assert.New(t) c := newConfig(WithTraceEnabled(false)) - assert.False(c.enabled.current) + assert.False(c.dynamic.enabled.current) + assert.False(knobs.GetScope(c.Scope, enabled)) }) t.Run("otel-env", func(t *testing.T) { assert := assert.New(t) t.Setenv("OTEL_TRACES_EXPORTER", "none") c := newConfig() - assert.False(c.enabled.current) + assert.False(c.dynamic.enabled.current) + assert.False(knobs.GetScope(c.Scope, enabled)) }) t.Run("dd-env", func(t *testing.T) { assert := assert.New(t) t.Setenv("DD_TRACE_ENABLED", "false") c := newConfig() - assert.False(c.enabled.current) + assert.False(c.dynamic.enabled.current) + assert.False(knobs.GetScope(c.Scope, enabled)) }) t.Run("override-chain", func(t *testing.T) { @@ -1388,10 +1393,12 @@ func TestWithTraceEnabled(t *testing.T) { t.Setenv("OTEL_TRACES_EXPORTER", "none") t.Setenv("DD_TRACE_ENABLED", "true") c := newConfig() - assert.True(c.enabled.current) + assert.True(c.dynamic.enabled.current) + assert.True(knobs.GetScope(c.Scope, enabled)) // tracer option overrides dd env c = newConfig(WithTraceEnabled(false)) - assert.False(c.enabled.current) + assert.False(c.dynamic.enabled.current) + assert.False(knobs.GetScope(c.Scope, enabled)) }) } diff --git a/ddtrace/tracer/remote_config.go b/ddtrace/tracer/remote_config.go index eaa8641730..1809bd9395 100644 --- a/ddtrace/tracer/remote_config.go +++ b/ddtrace/tracer/remote_config.go @@ -181,7 +181,7 @@ func (t *tracer) onRemoteConfigUpdate(u remoteconfig.ProductUpdate) map[string]s if updated { telemConfigs = append(telemConfigs, t.config.globalTags.toTelemetry()) } - if !t.config.enabled.current { + if !t.config.dynamic.enabled.current { log.Debug("APM Tracing is disabled. Restart the service to enable it.") } if len(telemConfigs) > 0 { @@ -233,11 +233,11 @@ func (t *tracer) onRemoteConfigUpdate(u remoteconfig.ProductUpdate) map[string]s telemConfigs = append(telemConfigs, t.config.globalTags.toTelemetry()) } if c.LibConfig.Enabled != nil { - if t.config.enabled.current == true && *c.LibConfig.Enabled == false { + if t.config.dynamic.enabled.current == true && *c.LibConfig.Enabled == false { log.Debug("Disabled APM Tracing through RC. Restart the service to enable it.") - t.config.enabled.handleRC(c.LibConfig.Enabled) - telemConfigs = append(telemConfigs, t.config.enabled.toTelemetry()) - } else if t.config.enabled.current == false && *c.LibConfig.Enabled == true { + t.config.dynamic.enabled.handleRC(c.LibConfig.Enabled) + telemConfigs = append(telemConfigs, t.config.dynamic.enabled.toTelemetry()) + } else if t.config.dynamic.enabled.current == false && *c.LibConfig.Enabled == true { log.Debug("APM Tracing is disabled. Restart the service to enable it.") } } diff --git a/ddtrace/tracer/remote_config_test.go b/ddtrace/tracer/remote_config_test.go index d64c2666fe..a6d703d272 100644 --- a/ddtrace/tracer/remote_config_test.go +++ b/ddtrace/tracer/remote_config_test.go @@ -18,6 +18,7 @@ import ( "gopkg.in/DataDog/dd-trace-go.v1/internal/telemetry/telemetrytest" "github.com/DataDog/datadog-agent/pkg/remoteconfig/state" + "github.com/darccio/knobs" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -395,7 +396,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { applyStatus := tr.onRemoteConfigUpdate(input) require.Equal(t, state.ApplyStateAcknowledged, applyStatus["path"].State) - require.Equal(t, false, tr.config.enabled.current) + require.Equal(t, false, tr.config.dynamic.enabled.current) headers := TextMapCarrier{ traceparentHeader: "00-12345678901234567890123456789012-1234567890123456-01", tracestateHeader: "dd=s:2;o:rum;t.usr.id:baz64~~", @@ -417,7 +418,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { input = remoteconfig.ProductUpdate{"path": nil} applyStatus = tr.onRemoteConfigUpdate(input) require.Equal(t, state.ApplyStateAcknowledged, applyStatus["path"].State) - require.Equal(t, false, tr.config.enabled.current) + require.Equal(t, false, knobs.GetScope(tr.config.Scope, enabled)) // turning tracing back explicitly is not allowed input = remoteconfig.ProductUpdate{ @@ -427,7 +428,8 @@ func TestOnRemoteConfigUpdate(t *testing.T) { } applyStatus = tr.onRemoteConfigUpdate(input) require.Equal(t, state.ApplyStateAcknowledged, applyStatus["path"].State) - require.Equal(t, false, tr.config.enabled.current) + require.Equal(t, false, tr.config.dynamic.enabled.current) + require.Equal(t, false, knobs.GetScope(tr.config.Scope, enabled)) telemetryClient.AssertNumberOfCalls(t, "ConfigChange", 1) }) diff --git a/ddtrace/tracer/span.go b/ddtrace/tracer/span.go index ebfa681fb3..1fe8f9385c 100644 --- a/ddtrace/tracer/span.go +++ b/ddtrace/tracer/span.go @@ -32,6 +32,7 @@ import ( "gopkg.in/DataDog/dd-trace-go.v1/internal/samplernames" "gopkg.in/DataDog/dd-trace-go.v1/internal/traceprof" + "github.com/darccio/knobs" "github.com/tinylib/msgp/msgp" "golang.org/x/xerrors" @@ -548,7 +549,7 @@ func (s *span) finish(finishTime int64) { keep := true if t, ok := internal.GetGlobalTracer().(*tracer); ok { - if !t.config.enabled.current { + if !knobs.GetScope(t.config.Scope, enabled) { return } // we have an active tracer diff --git a/ddtrace/tracer/telemetry.go b/ddtrace/tracer/telemetry.go index bc9185858f..d4591b38e5 100644 --- a/ddtrace/tracer/telemetry.go +++ b/ddtrace/tracer/telemetry.go @@ -10,6 +10,8 @@ import ( "strings" "gopkg.in/DataDog/dd-trace-go.v1/internal/telemetry" + + "github.com/darccio/knobs" ) var additionalConfigs []telemetry.Configuration @@ -63,7 +65,7 @@ func startTelemetry(c *config) { {Name: "trace_span_attribute_schema", Value: c.spanAttributeSchemaVersion}, {Name: "trace_peer_service_defaults_enabled", Value: c.peerServiceDefaultsEnabled}, {Name: "orchestrion_enabled", Value: c.orchestrionCfg.Enabled}, - {Name: "trace_enabled", Value: c.enabled.current, Origin: c.enabled.cfgOrigin}, + {Name: "trace_enabled", Value: knobs.GetScope(c.Scope, enabled), Origin: c.dynamic.enabled.cfgOrigin}, {Name: "trace_log_directory", Value: c.logDirectory}, c.traceSampleRate.toTelemetry(), c.headerAsTags.toTelemetry(), diff --git a/ddtrace/tracer/tracer.go b/ddtrace/tracer/tracer.go index 357ccd4c9c..d931b3e0c9 100644 --- a/ddtrace/tracer/tracer.go +++ b/ddtrace/tracer/tracer.go @@ -152,7 +152,7 @@ func Start(opts ...StartOption) { } defer telemetry.Time(telemetry.NamespaceGeneral, "init_time", nil, true)() t := newTracer(opts...) - if !t.config.enabled.current { + if !knobs.GetScope(t.config.Scope, enabled) { // TODO: instrumentation telemetry client won't get started // if tracing is disabled, but we still want to capture this // telemetry information. Will be fixed when the tracer and profiler @@ -500,7 +500,7 @@ func (t *tracer) pushChunk(trace *chunk) { // StartSpan creates, starts, and returns a new Span with the given `operationName`. func (t *tracer) StartSpan(operationName string, options ...ddtrace.StartSpanOption) ddtrace.Span { - if !t.config.enabled.current { + if !knobs.GetScope(t.config.Scope, enabled) { return internal.NoopSpan{} } var opts ddtrace.StartSpanConfig @@ -716,7 +716,7 @@ func (t *tracer) Stop() { // Inject uses the configured or default TextMap Propagator. func (t *tracer) Inject(ctx ddtrace.SpanContext, carrier interface{}) error { - if !t.config.enabled.current { + if !knobs.GetScope(t.config.Scope, enabled) { return nil } t.updateSampling(ctx) @@ -755,7 +755,7 @@ func (t *tracer) updateSampling(ctx ddtrace.SpanContext) { // Extract uses the configured or default TextMap Propagator. func (t *tracer) Extract(carrier interface{}) (ddtrace.SpanContext, error) { - if !t.config.enabled.current { + if !knobs.GetScope(t.config.Scope, enabled) { return internal.NoopSpanContext{}, nil } return t.config.propagator.Extract(carrier) From 6bacb78c3d27227b85008d6638e63c79224aa54d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Fri, 13 Dec 2024 14:57:05 +0100 Subject: [PATCH 7/7] chore(go.mod): pin darccio/knobs against main --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 6fc5e06f10..3b147fd7f0 100644 --- a/go.mod +++ b/go.mod @@ -32,7 +32,7 @@ require ( github.com/bradfitz/gomemcache v0.0.0-20230611145640-acc696258285 github.com/confluentinc/confluent-kafka-go v1.9.2 github.com/confluentinc/confluent-kafka-go/v2 v2.2.0 - github.com/darccio/knobs v0.0.0-20241213130850-25d5d476358d + github.com/darccio/knobs v0.0.0-20241213135620-026046f4510d github.com/denisenkom/go-mssqldb v0.11.0 github.com/dimfeld/httptreemux/v5 v5.5.0 github.com/eapache/queue/v2 v2.0.0-20230407133247-75960ed334e4 diff --git a/go.sum b/go.sum index 11e550823d..fa38484288 100644 --- a/go.sum +++ b/go.sum @@ -1032,8 +1032,8 @@ github.com/d2g/dhcp4 v0.0.0-20170904100407-a1d1b6c41b1c/go.mod h1:Ct2BUK8SB0YC1S github.com/d2g/dhcp4client v1.0.0/go.mod h1:j0hNfjhrt2SxUOw55nL0ATM/z4Yt3t2Kd1mW34z5W5s= github.com/d2g/dhcp4server v0.0.0-20181031114812-7d4a0a7f59a5/go.mod h1:Eo87+Kg/IX2hfWJfwxMzLyuSZyxSoAug2nGa1G2QAi8= github.com/d2g/hardwareaddr v0.0.0-20190221164911-e7d9fbe030e4/go.mod h1:bMl4RjIciD2oAxI7DmWRx6gbeqrkoLqv3MV0vzNad+I= -github.com/darccio/knobs v0.0.0-20241213130850-25d5d476358d h1:L8EQePHnMVEnQglNhfHKwvWEcbSX//GodoksWP6uyB8= -github.com/darccio/knobs v0.0.0-20241213130850-25d5d476358d/go.mod h1:nJ3ntDIyIQgqzTWF2QiL7eRNTy6iZJbBot3WcB+SymI= +github.com/darccio/knobs v0.0.0-20241213135620-026046f4510d h1:fOnj9dUTHsRxV3DKhHhnJ/DUM5pURJ87cAN6pSrtefM= +github.com/darccio/knobs v0.0.0-20241213135620-026046f4510d/go.mod h1:nJ3ntDIyIQgqzTWF2QiL7eRNTy6iZJbBot3WcB+SymI= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM=