From c65f534daca58a407d3565ed6cbe88372f15bc85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Wed, 20 Mar 2024 17:44:25 +0100 Subject: [PATCH 01/12] ddtrace/tracer: support default origin on dynamic config to support Active Tracing telemetry spec (re: AIT-8580) --- ddtrace/tracer/dynamic_config.go | 22 +++++++++++----- ddtrace/tracer/option.go | 4 ++- ddtrace/tracer/remote_config_test.go | 38 ++++++++++++++-------------- 3 files changed, 37 insertions(+), 27 deletions(-) diff --git a/ddtrace/tracer/dynamic_config.go b/ddtrace/tracer/dynamic_config.go index 919b4f7229..53bc449cd0 100644 --- a/ddtrace/tracer/dynamic_config.go +++ b/ddtrace/tracer/dynamic_config.go @@ -11,6 +11,12 @@ import ( "gopkg.in/DataDog/dd-trace-go.v1/internal/telemetry" ) +const ( + originDefault = "default" + originEnvVar = "env_var" + originRemoteConfig = "remote_config" +) + // dynamicConfig is a thread-safe generic data structure to represent configuration fields. // It's designed to satisfy the dynamic configuration semantics (i.e reset, update, apply configuration changes). // This structure will be extended to track the origin of configuration values as well (e.g remote_config, env_var). @@ -26,11 +32,12 @@ type dynamicConfig[T any] struct { func newDynamicConfig[T any](name string, val T, apply func(T) bool, equal func(x, y T) bool) dynamicConfig[T] { return dynamicConfig[T]{ - cfgName: name, - current: val, - startup: val, - apply: apply, - equal: equal, + cfgName: name, + current: val, + startup: val, + apply: apply, + equal: equal, + cfgOrigin: originDefault, } } @@ -61,7 +68,8 @@ func (dc *dynamicConfig[T]) reset() bool { return false } dc.current = dc.startup - dc.cfgOrigin = "" + // TODO: set the origin to the startup value's origin + dc.cfgOrigin = originDefault return dc.apply(dc.startup) } @@ -69,7 +77,7 @@ func (dc *dynamicConfig[T]) reset() bool { // Returns whether the configuration value has been updated or not func (dc *dynamicConfig[T]) handleRC(val *T) bool { if val != nil { - return dc.update(*val, "remote_config") + return dc.update(*val, originRemoteConfig) } return dc.reset() } diff --git a/ddtrace/tracer/option.go b/ddtrace/tracer/option.go index 9d7a8b8700..e4d0b5e600 100644 --- a/ddtrace/tracer/option.go +++ b/ddtrace/tracer/option.go @@ -334,7 +334,9 @@ func newConfig(opts ...StartOption) *config { } c.headerAsTags = newDynamicConfig("trace_header_tags", nil, setHeaderTags, equalSlice[string]) if v := os.Getenv("DD_TRACE_HEADER_TAGS"); v != "" { - WithHeaderTags(strings.Split(v, ","))(c) + c.headerAsTags.update(strings.Split(v, ","), originEnvVar) + // Required to ensure that the startup header tags are set on reset. + c.headerAsTags.startup = c.headerAsTags.current } if v := os.Getenv("DD_TAGS"); v != "" { tags := internal.ParseTagString(v) diff --git a/ddtrace/tracer/remote_config_test.go b/ddtrace/tracer/remote_config_test.go index 9a196eab3b..830849f0c6 100644 --- a/ddtrace/tracer/remote_config_test.go +++ b/ddtrace/tracer/remote_config_test.go @@ -41,7 +41,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { // Telemetry telemetryClient.AssertNumberOfCalls(t, "ConfigChange", 1) - telemetryClient.AssertCalled(t, "ConfigChange", []telemetry.Configuration{{Name: "trace_sample_rate", Value: 0.5, Origin: "remote_config"}}) + telemetryClient.AssertCalled(t, "ConfigChange", []telemetry.Configuration{{Name: "trace_sample_rate", Value: 0.5, Origin: originRemoteConfig}}) //Apply RC with sampling rules. Assert _dd.rule_psr shows the corresponding rule matched rate. input = remoteconfig.ProductUpdate{ @@ -99,7 +99,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { // Telemetry telemetryClient.AssertNumberOfCalls(t, "ConfigChange", 1) - telemetryClient.AssertCalled(t, "ConfigChange", []telemetry.Configuration{{Name: "trace_sample_rate", Value: 0.2, Origin: "remote_config"}}) + telemetryClient.AssertCalled(t, "ConfigChange", []telemetry.Configuration{{Name: "trace_sample_rate", Value: 0.2, Origin: originRemoteConfig}}) // Unset RC. Assert _dd.rule_psr shows the previous sampling rate (0.1) is applied input = remoteconfig.ProductUpdate{"path": []byte(`{"lib_config": {}, "service_target": {"service": "my-service", "env": "my-env"}}`)} @@ -111,7 +111,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { // Telemetry telemetryClient.AssertNumberOfCalls(t, "ConfigChange", 2) - telemetryClient.AssertCalled(t, "ConfigChange", []telemetry.Configuration{{Name: "trace_sample_rate", Value: 0.1, Origin: ""}}) + telemetryClient.AssertCalled(t, "ConfigChange", []telemetry.Configuration{{Name: "trace_sample_rate", Value: 0.1, Origin: originDefault}}) }) t.Run("DD_TRACE_SAMPLING_RULES rate=0.1 and RC trace sampling rules rate = 0.2", func(t *testing.T) { @@ -156,8 +156,8 @@ func TestOnRemoteConfigUpdate(t *testing.T) { telemetryClient.AssertNumberOfCalls(t, "ConfigChange", 1) telemetryClient.AssertCalled(t, "ConfigChange", []telemetry.Configuration{ - {Name: "trace_sample_rate", Value: 0.5, Origin: "remote_config"}, - {Name: "trace_sample_rules", Value: `[{"service":"my-service","name":"web.request","resource":"abc","sample_rate":0.2}]`, Origin: "remote_config"}}) + {Name: "trace_sample_rate", Value: 0.5, Origin: originRemoteConfig}, + {Name: "trace_sample_rules", Value: `[{"service":"my-service","name":"web.request","resource":"abc","sample_rate":0.2}]`, Origin: originRemoteConfig}}) }) t.Run("RC header tags = X-Test-Header:my-tag-name is applied and can be reverted", func(t *testing.T) { @@ -178,7 +178,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { require.Equal(t, "my-tag-name", globalconfig.HeaderTag("X-Test-Header")) telemetryClient.AssertNumberOfCalls(t, "ConfigChange", 1) - telemetryClient.AssertCalled(t, "ConfigChange", []telemetry.Configuration{{Name: "trace_header_tags", Value: "X-Test-Header:my-tag-name", Origin: "remote_config"}}) + telemetryClient.AssertCalled(t, "ConfigChange", []telemetry.Configuration{{Name: "trace_header_tags", Value: "X-Test-Header:my-tag-name", Origin: originRemoteConfig}}) // Unset RC. Assert header tags are not set input = remoteconfig.ProductUpdate{"path": []byte(`{"lib_config": {}, "service_target": {"service": "my-service", "env": "my-env"}}`)} @@ -188,7 +188,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { // Telemetry telemetryClient.AssertNumberOfCalls(t, "ConfigChange", 2) - telemetryClient.AssertCalled(t, "ConfigChange", []telemetry.Configuration{{Name: "trace_header_tags", Value: "", Origin: ""}}) + telemetryClient.AssertCalled(t, "ConfigChange", []telemetry.Configuration{{Name: "trace_header_tags", Value: "", Origin: originDefault}}) }) t.Run("RC tracing_enabled = false is applied", func(t *testing.T) { @@ -261,7 +261,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { // Telemetry telemetryClient.AssertNumberOfCalls(t, "ConfigChange", 1) - telemetryClient.AssertCalled(t, "ConfigChange", []telemetry.Configuration{{Name: "trace_header_tags", Value: "X-Test-Header:my-tag-name-from-rc", Origin: "remote_config"}}) + telemetryClient.AssertCalled(t, "ConfigChange", []telemetry.Configuration{{Name: "trace_header_tags", Value: "X-Test-Header:my-tag-name-from-rc", Origin: originRemoteConfig}}) // Unset RC. Assert global config shows the DD_TRACE_HEADER_TAGS header tag input = remoteconfig.ProductUpdate{"path": []byte(`{"lib_config": {}, "service_target": {"service": "my-service", "env": "my-env"}}`)} @@ -272,7 +272,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { // Telemetry telemetryClient.AssertNumberOfCalls(t, "ConfigChange", 2) - telemetryClient.AssertCalled(t, "ConfigChange", []telemetry.Configuration{{Name: "trace_header_tags", Value: "X-Test-Header:my-tag-name-from-env", Origin: ""}}) + telemetryClient.AssertCalled(t, "ConfigChange", []telemetry.Configuration{{Name: "trace_header_tags", Value: "X-Test-Header:my-tag-name-from-env", Origin: originDefault}}) }) t.Run("In code header tags = X-Test-Header:my-tag-name-in-code and RC header tags = X-Test-Header:my-tag-name-from-rc", func(t *testing.T) { @@ -294,7 +294,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { // Telemetry telemetryClient.AssertNumberOfCalls(t, "ConfigChange", 1) - telemetryClient.AssertCalled(t, "ConfigChange", []telemetry.Configuration{{Name: "trace_header_tags", Value: "X-Test-Header:my-tag-name-from-rc", Origin: "remote_config"}}) + telemetryClient.AssertCalled(t, "ConfigChange", []telemetry.Configuration{{Name: "trace_header_tags", Value: "X-Test-Header:my-tag-name-from-rc", Origin: originRemoteConfig}}) // Unset RC. Assert global config shows the in-code header tag input = remoteconfig.ProductUpdate{"path": []byte(`{"lib_config": {}, "service_target": {"service": "my-service", "env": "my-env"}}`)} @@ -305,7 +305,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { // Telemetry telemetryClient.AssertNumberOfCalls(t, "ConfigChange", 2) - telemetryClient.AssertCalled(t, "ConfigChange", []telemetry.Configuration{{Name: "trace_header_tags", Value: "X-Test-Header:my-tag-name-in-code", Origin: ""}}) + telemetryClient.AssertCalled(t, "ConfigChange", []telemetry.Configuration{{Name: "trace_header_tags", Value: "X-Test-Header:my-tag-name-in-code", Origin: originDefault}}) }) t.Run("Invalid payload", func(t *testing.T) { @@ -388,7 +388,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { // Telemetry telemetryClient.AssertNumberOfCalls(t, "ConfigChange", 1) - telemetryClient.AssertCalled(t, "ConfigChange", []telemetry.Configuration{{Name: "trace_tags", Value: "key3:val3,key4:val4," + runtimeIDTag, Origin: "remote_config"}}) + telemetryClient.AssertCalled(t, "ConfigChange", []telemetry.Configuration{{Name: "trace_tags", Value: "key3:val3,key4:val4," + runtimeIDTag, Origin: originRemoteConfig}}) // Unset RC. Assert config shows the original DD_TAGS + WithGlobalTag + runtime ID input = remoteconfig.ProductUpdate{"path": []byte(`{"lib_config": {}, "service_target": {"service": "my-service", "env": "my-env"}}`)} @@ -405,7 +405,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { // Telemetry telemetryClient.AssertNumberOfCalls(t, "ConfigChange", 2) - telemetryClient.AssertCalled(t, "ConfigChange", []telemetry.Configuration{{Name: "trace_tags", Value: "key0:val0,key1:val1,key2:val2," + runtimeIDTag, Origin: ""}}) + telemetryClient.AssertCalled(t, "ConfigChange", []telemetry.Configuration{{Name: "trace_tags", Value: "key0:val0,key1:val1,key2:val2," + runtimeIDTag, Origin: originDefault}}) }) t.Run("Deleted config", func(t *testing.T) { @@ -434,9 +434,9 @@ func TestOnRemoteConfigUpdate(t *testing.T) { // Telemetry telemetryClient.AssertNumberOfCalls(t, "ConfigChange", 1) telemetryClient.AssertCalled(t, "ConfigChange", []telemetry.Configuration{ - {Name: "trace_sample_rate", Value: 0.2, Origin: "remote_config"}, - {Name: "trace_header_tags", Value: "X-Test-Header:my-tag-from-rc", Origin: "remote_config"}, - {Name: "trace_tags", Value: "ddtag:from-rc," + ext.RuntimeID + ":" + globalconfig.RuntimeID(), Origin: "remote_config"}, + {Name: "trace_sample_rate", Value: 0.2, Origin: originRemoteConfig}, + {Name: "trace_header_tags", Value: "X-Test-Header:my-tag-from-rc", Origin: originRemoteConfig}, + {Name: "trace_tags", Value: "ddtag:from-rc," + ext.RuntimeID + ":" + globalconfig.RuntimeID(), Origin: originRemoteConfig}, }) // Remove RC. Assert configuration is reset to the original values. @@ -452,9 +452,9 @@ func TestOnRemoteConfigUpdate(t *testing.T) { // Telemetry telemetryClient.AssertNumberOfCalls(t, "ConfigChange", 2) telemetryClient.AssertCalled(t, "ConfigChange", []telemetry.Configuration{ - {Name: "trace_sample_rate", Value: 0.1, Origin: ""}, - {Name: "trace_header_tags", Value: "X-Test-Header:my-tag-from-env", Origin: ""}, - {Name: "trace_tags", Value: "ddtag:from-env," + ext.RuntimeID + ":" + globalconfig.RuntimeID(), Origin: ""}, + {Name: "trace_sample_rate", Value: 0.1, Origin: originDefault}, + {Name: "trace_header_tags", Value: "X-Test-Header:my-tag-from-env", Origin: originDefault}, + {Name: "trace_tags", Value: "ddtag:from-env," + ext.RuntimeID + ":" + globalconfig.RuntimeID(), Origin: originDefault}, }) }) From 430d8763217bcb9032675cdce8078cac740e6a08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Thu, 4 Apr 2024 14:06:11 +0200 Subject: [PATCH 02/12] .github/workflows: use specific branch to test new parametric tests --- .github/workflows/parametric-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/parametric-tests.yml b/.github/workflows/parametric-tests.yml index eea919e7ce..6d6cdb3317 100644 --- a/.github/workflows/parametric-tests.yml +++ b/.github/workflows/parametric-tests.yml @@ -33,7 +33,7 @@ jobs: uses: actions/checkout@v3 with: repository: 'DataDog/system-tests' - ref: ${{ inputs.ref }} + ref: kylev/enable-config-tests - name: Checkout dd-trace-go uses: actions/checkout@v3 From 46bb463775081de864d459955a8546c6bb67e241 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Thu, 4 Apr 2024 14:41:12 +0200 Subject: [PATCH 03/12] ddtrace/tracer: set trace_sample_rate origin to env_var if present --- ddtrace/tracer/remote_config_test.go | 30 ++++++++++++++++++++++++---- ddtrace/tracer/tracer.go | 8 ++++++++ 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/ddtrace/tracer/remote_config_test.go b/ddtrace/tracer/remote_config_test.go index 830849f0c6..2bb0b9e025 100644 --- a/ddtrace/tracer/remote_config_test.go +++ b/ddtrace/tracer/remote_config_test.go @@ -29,6 +29,8 @@ func TestOnRemoteConfigUpdate(t *testing.T) { tracer, _, _, stop := startTestTracer(t, WithService("my-service"), WithEnv("my-env")) defer stop() + require.Equal(t, originDefault, tracer.config.traceSampleRate.cfgOrigin) + // Apply RC. Assert _dd.rule_psr shows the RC sampling rate (0.5) is applied input := remoteconfig.ProductUpdate{ "path": []byte(`{"lib_config": {"tracing_sampling_rate": 0.5}, "service_target": {"service": "my-service", "env": "my-env"}}`), @@ -87,6 +89,8 @@ func TestOnRemoteConfigUpdate(t *testing.T) { tracer, _, _, stop := startTestTracer(t, WithService("my-service"), WithEnv("my-env")) defer stop() + require.Equal(t, originEnvVar, tracer.config.traceSampleRate.cfgOrigin) + // Apply RC. Assert _dd.rule_psr shows the RC sampling rate (0.2) is applied input := remoteconfig.ProductUpdate{ "path": []byte(`{"lib_config": {"tracing_sampling_rate": 0.2}, "service_target": {"service": "my-service", "env": "my-env"}}`), @@ -127,6 +131,8 @@ func TestOnRemoteConfigUpdate(t *testing.T) { tracer, _, _, stop := startTestTracer(t, WithService("my-service"), WithEnv("my-env")) defer stop() + require.Equal(t, originDefault, tracer.config.traceSampleRate.cfgOrigin) + s := tracer.StartSpan("web.request").(*span) s.Finish() require.Equal(t, 0.1, s.Metrics[keyRulesSamplerAppliedRate]) @@ -168,6 +174,8 @@ func TestOnRemoteConfigUpdate(t *testing.T) { tracer, _, _, stop := startTestTracer(t, WithService("my-service"), WithEnv("my-env")) defer stop() + require.Equal(t, originDefault, tracer.config.traceSampleRate.cfgOrigin) + // Apply RC. Assert global config shows the RC header tag is applied input := remoteconfig.ProductUpdate{ "path": []byte(`{"lib_config": {"tracing_header_tags": [{"header": "X-Test-Header", "tag_name": "my-tag-name"}]}, "service_target": {"service": "my-service", "env": "my-env"}}`), @@ -195,15 +203,15 @@ func TestOnRemoteConfigUpdate(t *testing.T) { telemetryClient := new(telemetrytest.MockClient) defer telemetry.MockGlobalClient(telemetryClient)() - Start(WithService("my-service"), WithEnv("my-env")) - defer Stop() + tr, _, _, stop := startTestTracer(t, WithService("my-service"), WithEnv("my-env")) + defer stop() + + require.Equal(t, originDefault, tr.config.traceSampleRate.cfgOrigin) input := remoteconfig.ProductUpdate{ "path": []byte(`{"lib_config": {"tracing_enabled": false}, "service_target": {"service": "my-service", "env": "my-env"}}`), } - tr, ok := internal.GetGlobalTracer().(*tracer) - require.Equal(t, true, ok) applyStatus := tr.onRemoteConfigUpdate(input) require.Equal(t, state.ApplyStateAcknowledged, applyStatus["path"].State) require.Equal(t, false, tr.config.enabled.current) @@ -250,6 +258,8 @@ func TestOnRemoteConfigUpdate(t *testing.T) { tracer, _, _, stop := startTestTracer(t, WithService("my-service"), WithEnv("my-env")) defer stop() + require.Equal(t, originDefault, tracer.config.traceSampleRate.cfgOrigin) + // Apply RC. Assert global config shows the RC header tag is applied input := remoteconfig.ProductUpdate{ "path": []byte(`{"lib_config": {"tracing_header_tags": [{"header": "X-Test-Header", "tag_name": "my-tag-name-from-rc"}]}, "service_target": {"service": "my-service", "env": "my-env"}}`), @@ -283,6 +293,8 @@ func TestOnRemoteConfigUpdate(t *testing.T) { tracer, _, _, stop := startTestTracer(t, WithService("my-service"), WithEnv("my-env"), WithHeaderTags([]string{"X-Test-Header:my-tag-name-in-code"})) defer stop() + require.Equal(t, originDefault, tracer.config.traceSampleRate.cfgOrigin) + // Apply RC. Assert global config shows the RC header tag is applied input := remoteconfig.ProductUpdate{ "path": []byte(`{"lib_config": {"tracing_header_tags": [{"header": "X-Test-Header", "tag_name": "my-tag-name-from-rc"}]}, "service_target": {"service": "my-service", "env": "my-env"}}`), @@ -315,6 +327,8 @@ func TestOnRemoteConfigUpdate(t *testing.T) { tracer, _, _, stop := startTestTracer(t, WithService("my-service"), WithEnv("my-env")) defer stop() + require.Equal(t, originDefault, tracer.config.traceSampleRate.cfgOrigin) + input := remoteconfig.ProductUpdate{ "path": []byte(`{"lib_config": {"tracing_sampling_rate": "string value", "service_target": {"service": "my-service", "env": "my-env"}}}`), } @@ -333,6 +347,8 @@ func TestOnRemoteConfigUpdate(t *testing.T) { tracer, _, _, stop := startTestTracer(t, WithServiceName("my-service"), WithEnv("my-env")) defer stop() + require.Equal(t, originDefault, tracer.config.traceSampleRate.cfgOrigin) + input := remoteconfig.ProductUpdate{ "path": []byte(`{"lib_config": {}, "service_target": {"service": "other-service", "env": "my-env"}}`), } @@ -351,6 +367,8 @@ func TestOnRemoteConfigUpdate(t *testing.T) { tracer, _, _, stop := startTestTracer(t, WithServiceName("my-service"), WithEnv("my-env")) defer stop() + require.Equal(t, originDefault, tracer.config.traceSampleRate.cfgOrigin) + input := remoteconfig.ProductUpdate{ "path": []byte(`{"lib_config": {}, "service_target": {"service": "my-service", "env": "other-env"}}`), } @@ -370,6 +388,8 @@ func TestOnRemoteConfigUpdate(t *testing.T) { tracer, _, _, stop := startTestTracer(t, WithService("my-service"), WithEnv("my-env"), WithGlobalTag("key2", "val2")) defer stop() + require.Equal(t, originDefault, tracer.config.traceSampleRate.cfgOrigin) + // Apply RC. Assert global tags have the RC tags key3:val3,key4:val4 applied + runtime ID input := remoteconfig.ProductUpdate{ "path": []byte(`{"lib_config": {"tracing_tags": ["key3:val3","key4:val4"]}, "service_target": {"service": "my-service", "env": "my-env"}}`), @@ -419,6 +439,8 @@ func TestOnRemoteConfigUpdate(t *testing.T) { tracer, _, _, stop := startTestTracer(t, WithService("my-service"), WithEnv("my-env")) defer stop() + require.Equal(t, originEnvVar, tracer.config.traceSampleRate.cfgOrigin) + // Apply RC. Assert configuration is updated to the RC values. input := remoteconfig.ProductUpdate{ "path": []byte(`{"lib_config": {"tracing_sampling_rate": 0.2,"tracing_header_tags": [{"header": "X-Test-Header", "tag_name": "my-tag-from-rc"}],"tracing_tags": ["ddtag:from-rc"]}, "service_target": {"service": "my-service", "env": "my-env"}}`), diff --git a/ddtrace/tracer/tracer.go b/ddtrace/tracer/tracer.go index d704dfaa0c..06d6c5e471 100644 --- a/ddtrace/tracer/tracer.go +++ b/ddtrace/tracer/tracer.go @@ -8,6 +8,7 @@ package tracer import ( gocontext "context" "encoding/binary" + "math" "os" "runtime/pprof" rt "runtime/trace" @@ -252,6 +253,13 @@ func newUnstartedTracer(opts ...StartOption) *tracer { globalRate := globalSampleRate() rulesSampler := newRulesSampler(c.traceRules, c.spanRules, globalRate) c.traceSampleRate = newDynamicConfig("trace_sample_rate", globalRate, 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(globalRate) { + c.traceSampleRate.cfgOrigin = originEnvVar + } c.traceSampleRules = newDynamicConfig("trace_sample_rules", c.traceRules, rulesSampler.traces.setTraceSampleRules, EqualsFalseNegative) var dataStreamsProcessor *datastreams.Processor From fab626bc4d065a6e16ef61dba1035214c559bd2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Tue, 28 May 2024 16:50:41 +0200 Subject: [PATCH 04/12] internal/telemetry: change Configuration#Origin from string to enum --- ddtrace/tracer/dynamic_config.go | 25 +++---- ddtrace/tracer/option.go | 3 +- ddtrace/tracer/remote_config_test.go | 70 +++++++++---------- ddtrace/tracer/tracer.go | 2 +- internal/appsec/config/config.go | 2 +- internal/appsec/telemetry.go | 10 +-- internal/telemetry/client.go | 4 +- internal/telemetry/message.go | 45 +++++++++++- internal/telemetry/telemetry_test.go | 14 ++-- .../telemetry/telemetrytest/telemetrytest.go | 2 +- 10 files changed, 105 insertions(+), 72 deletions(-) diff --git a/ddtrace/tracer/dynamic_config.go b/ddtrace/tracer/dynamic_config.go index 53bc449cd0..db48be5a73 100644 --- a/ddtrace/tracer/dynamic_config.go +++ b/ddtrace/tracer/dynamic_config.go @@ -11,12 +11,6 @@ import ( "gopkg.in/DataDog/dd-trace-go.v1/internal/telemetry" ) -const ( - originDefault = "default" - originEnvVar = "env_var" - originRemoteConfig = "remote_config" -) - // dynamicConfig is a thread-safe generic data structure to represent configuration fields. // It's designed to satisfy the dynamic configuration semantics (i.e reset, update, apply configuration changes). // This structure will be extended to track the origin of configuration values as well (e.g remote_config, env_var). @@ -25,19 +19,18 @@ type dynamicConfig[T any] struct { current T // holds the current configuration value startup T // holds the startup configuration value cfgName string // holds the name of the configuration, has to be compatible with telemetry.Configuration.Name - cfgOrigin string // holds the origin of the current configuration value (currently only supports remote_config, empty otherwise) + cfgOrigin telemetry.Origin // holds the origin of the current configuration value (currently only supports remote_config, empty otherwise) apply func(T) bool // executes any config-specific operations to propagate the update properly, returns whether the update was applied equal func(x, y T) bool // compares two configuration values, this is used to avoid unnecessary config and telemetry updates } func newDynamicConfig[T any](name string, val T, apply func(T) bool, equal func(x, y T) bool) dynamicConfig[T] { return dynamicConfig[T]{ - cfgName: name, - current: val, - startup: val, - apply: apply, - equal: equal, - cfgOrigin: originDefault, + cfgName: name, + current: val, + startup: val, + apply: apply, + equal: equal, } } @@ -49,7 +42,7 @@ func (dc *dynamicConfig[T]) get() T { } // update applies a new configuration value -func (dc *dynamicConfig[T]) update(val T, origin string) bool { +func (dc *dynamicConfig[T]) update(val T, origin telemetry.Origin) bool { dc.Lock() defer dc.Unlock() if dc.equal(dc.current, val) { @@ -69,7 +62,7 @@ func (dc *dynamicConfig[T]) reset() bool { } dc.current = dc.startup // TODO: set the origin to the startup value's origin - dc.cfgOrigin = originDefault + dc.cfgOrigin = telemetry.OriginDefault return dc.apply(dc.startup) } @@ -77,7 +70,7 @@ func (dc *dynamicConfig[T]) reset() bool { // Returns whether the configuration value has been updated or not func (dc *dynamicConfig[T]) handleRC(val *T) bool { if val != nil { - return dc.update(*val, originRemoteConfig) + return dc.update(*val, telemetry.OriginRemoteConfig) } return dc.reset() } diff --git a/ddtrace/tracer/option.go b/ddtrace/tracer/option.go index a92a5d0a9f..98b048330c 100644 --- a/ddtrace/tracer/option.go +++ b/ddtrace/tracer/option.go @@ -29,6 +29,7 @@ import ( "gopkg.in/DataDog/dd-trace-go.v1/internal/log" "gopkg.in/DataDog/dd-trace-go.v1/internal/namingschema" "gopkg.in/DataDog/dd-trace-go.v1/internal/normalizer" + "gopkg.in/DataDog/dd-trace-go.v1/internal/telemetry" "gopkg.in/DataDog/dd-trace-go.v1/internal/traceprof" "gopkg.in/DataDog/dd-trace-go.v1/internal/version" @@ -336,7 +337,7 @@ func newConfig(opts ...StartOption) *config { } c.headerAsTags = newDynamicConfig("trace_header_tags", nil, setHeaderTags, equalSlice[string]) if v := os.Getenv("DD_TRACE_HEADER_TAGS"); v != "" { - c.headerAsTags.update(strings.Split(v, ","), originEnvVar) + c.headerAsTags.update(strings.Split(v, ","), telemetry.OriginEnvVar) // Required to ensure that the startup header tags are set on reset. c.headerAsTags.startup = c.headerAsTags.current } diff --git a/ddtrace/tracer/remote_config_test.go b/ddtrace/tracer/remote_config_test.go index 4a3b5e5aa1..769b978b8a 100644 --- a/ddtrace/tracer/remote_config_test.go +++ b/ddtrace/tracer/remote_config_test.go @@ -30,7 +30,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { tracer, _, _, stop := startTestTracer(t, WithService("my-service"), WithEnv("my-env")) defer stop() - require.Equal(t, originDefault, tracer.config.traceSampleRate.cfgOrigin) + require.Equal(t, telemetry.OriginDefault, tracer.config.traceSampleRate.cfgOrigin) // Apply RC. Assert _dd.rule_psr shows the RC sampling rate (0.5) is applied input := remoteconfig.ProductUpdate{ @@ -49,7 +49,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { telemetryClient.AssertCalled( t, "ConfigChange", - []telemetry.Configuration{{Name: "trace_sample_rate", Value: 0.5, Origin: originRemoteConfig}}, + []telemetry.Configuration{{Name: "trace_sample_rate", Value: 0.5, Origin: telemetry.OriginRemoteConfig}}, ) //Apply RC with sampling rules. Assert _dd.rule_psr shows the corresponding rule matched rate. @@ -102,7 +102,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { tracer, _, _, stop := startTestTracer(t, WithService("my-service"), WithEnv("my-env")) defer stop() - require.Equal(t, originEnvVar, tracer.config.traceSampleRate.cfgOrigin) + require.Equal(t, telemetry.OriginEnvVar, tracer.config.traceSampleRate.cfgOrigin) // Apply RC. Assert _dd.rule_psr shows the RC sampling rate (0.2) is applied input := remoteconfig.ProductUpdate{ @@ -121,7 +121,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { telemetryClient.AssertCalled( t, "ConfigChange", - []telemetry.Configuration{{Name: "trace_sample_rate", Value: 0.2, Origin: originRemoteConfig}}, + []telemetry.Configuration{{Name: "trace_sample_rate", Value: 0.2, Origin: telemetry.OriginRemoteConfig}}, ) // Unset RC. Assert _dd.rule_psr shows the previous sampling rate (0.1) is applied @@ -139,7 +139,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { telemetryClient.AssertCalled( t, "ConfigChange", - []telemetry.Configuration{{Name: "trace_sample_rate", Value: 0.1, Origin: "default"}}, + []telemetry.Configuration{{Name: "trace_sample_rate", Value: 0.1, Origin: telemetry.OriginDefault}}, ) }) @@ -156,7 +156,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { tracer, _, _, stop := startTestTracer(t, WithService("my-service"), WithEnv("my-env")) defer stop() - require.Equal(t, originDefault, tracer.config.traceSampleRate.cfgOrigin) + require.Equal(t, telemetry.OriginDefault, tracer.config.traceSampleRate.cfgOrigin) s := tracer.StartSpan("web.request").(*span) s.Finish() @@ -194,11 +194,11 @@ func TestOnRemoteConfigUpdate(t *testing.T) { telemetryClient.AssertNumberOfCalls(t, "ConfigChange", 1) telemetryClient.AssertCalled(t, "ConfigChange", []telemetry.Configuration{ - {Name: "trace_sample_rate", Value: 0.5, Origin: originRemoteConfig}, + {Name: "trace_sample_rate", Value: 0.5, Origin: telemetry.OriginRemoteConfig}, { Name: "trace_sample_rules", Value: `[{"service":"my-service","name":"web.request","resource":"abc","sample_rate":1,"provenance":"customer"}]`, - Origin: originRemoteConfig, + Origin: telemetry.OriginRemoteConfig, }}) }) @@ -271,16 +271,16 @@ func TestOnRemoteConfigUpdate(t *testing.T) { } telemetryClient.AssertNumberOfCalls(t, "ConfigChange", 2) telemetryClient.AssertCalled(t, "ConfigChange", []telemetry.Configuration{ - {Name: "trace_sample_rate", Value: 0.5, Origin: originRemoteConfig}, + {Name: "trace_sample_rate", Value: 0.5, Origin: telemetry.OriginRemoteConfig}, {Name: "trace_sample_rules", - Value: `[{"service":"my-service","name":"web.request","resource":"abc","sample_rate":1,"provenance":"customer"} {"service":"my-service","name":"web.request","resource":"*","sample_rate":0.3,"provenance":"dynamic"}]`, Origin: originRemoteConfig}, + Value: `[{"service":"my-service","name":"web.request","resource":"abc","sample_rate":1,"provenance":"customer"} {"service":"my-service","name":"web.request","resource":"*","sample_rate":0.3,"provenance":"dynamic"}]`, Origin: telemetry.OriginRemoteConfig}, }) telemetryClient.AssertCalled(t, "ConfigChange", []telemetry.Configuration{ - {Name: "trace_sample_rate", Value: nil, Origin: "default"}, + {Name: "trace_sample_rate", Value: nil, Origin: telemetry.OriginDefault}, { Name: "trace_sample_rules", Value: `[{"service":"my-service","name":"web.request","resource":"*","sample_rate":0.1,"type":"1"}]`, - Origin: "default", + Origin: telemetry.OriginDefault, }, }) }) @@ -321,11 +321,11 @@ func TestOnRemoteConfigUpdate(t *testing.T) { telemetryClient.AssertNumberOfCalls(t, "ConfigChange", 1) telemetryClient.AssertCalled(t, "ConfigChange", []telemetry.Configuration{ - {Name: "trace_sample_rate", Value: 0.5, Origin: originRemoteConfig}, + {Name: "trace_sample_rate", Value: 0.5, Origin: telemetry.OriginRemoteConfig}, { Name: "trace_sample_rules", Value: `[{"service":"my-service","name":"web.request","resource":"*","sample_rate":1,"tags":{"tag-a":"tv-a??"},"provenance":"customer"}]`, - Origin: originRemoteConfig, + Origin: telemetry.OriginRemoteConfig, }, }) }) @@ -338,7 +338,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { tracer, _, _, stop := startTestTracer(t, WithService("my-service"), WithEnv("my-env")) defer stop() - require.Equal(t, originDefault, tracer.config.traceSampleRate.cfgOrigin) + require.Equal(t, telemetry.OriginDefault, tracer.config.traceSampleRate.cfgOrigin) // Apply RC. Assert global config shows the RC header tag is applied input := remoteconfig.ProductUpdate{ @@ -356,7 +356,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { t, "ConfigChange", []telemetry.Configuration{ - {Name: "trace_header_tags", Value: "X-Test-Header:my-tag-name", Origin: originRemoteConfig}, + {Name: "trace_header_tags", Value: "X-Test-Header:my-tag-name", Origin: telemetry.OriginRemoteConfig}, }, ) @@ -373,7 +373,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { telemetryClient.AssertCalled( t, "ConfigChange", - []telemetry.Configuration{{Name: "trace_header_tags", Value: "", Origin: "default"}}, + []telemetry.Configuration{{Name: "trace_header_tags", Value: "", Origin: telemetry.OriginDefault}}, ) }) @@ -384,7 +384,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { tr, _, _, stop := startTestTracer(t, WithService("my-service"), WithEnv("my-env")) defer stop() - require.Equal(t, originDefault, tr.config.traceSampleRate.cfgOrigin) + require.Equal(t, telemetry.OriginDefault, tr.config.traceSampleRate.cfgOrigin) input := remoteconfig.ProductUpdate{ "path": []byte( @@ -459,7 +459,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { t, "ConfigChange", []telemetry.Configuration{ - {Name: "trace_header_tags", Value: "X-Test-Header:my-tag-name-from-rc", Origin: originRemoteConfig}, + {Name: "trace_header_tags", Value: "X-Test-Header:my-tag-name-from-rc", Origin: telemetry.OriginRemoteConfig}, }, ) @@ -478,7 +478,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { t, "ConfigChange", []telemetry.Configuration{ - {Name: "trace_header_tags", Value: "X-Test-Header:my-tag-name-from-env", Origin: "default"}, + {Name: "trace_header_tags", Value: "X-Test-Header:my-tag-name-from-env", Origin: telemetry.OriginDefault}, }, ) }, @@ -516,7 +516,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { t, "ConfigChange", []telemetry.Configuration{ - {Name: "trace_header_tags", Value: "X-Test-Header:my-tag-name-from-rc", Origin: originRemoteConfig}, + {Name: "trace_header_tags", Value: "X-Test-Header:my-tag-name-from-rc", Origin: telemetry.OriginRemoteConfig}, }, ) @@ -535,7 +535,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { t, "ConfigChange", []telemetry.Configuration{ - {Name: "trace_header_tags", Value: "X-Test-Header:my-tag-name-in-code", Origin: "default"}, + {Name: "trace_header_tags", Value: "X-Test-Header:my-tag-name-in-code", Origin: telemetry.OriginDefault}, }, ) }, @@ -548,7 +548,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { tracer, _, _, stop := startTestTracer(t, WithService("my-service"), WithEnv("my-env")) defer stop() - require.Equal(t, originDefault, tracer.config.traceSampleRate.cfgOrigin) + require.Equal(t, telemetry.OriginDefault, tracer.config.traceSampleRate.cfgOrigin) input := remoteconfig.ProductUpdate{ "path": []byte( @@ -570,7 +570,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { tracer, _, _, stop := startTestTracer(t, WithServiceName("my-service"), WithEnv("my-env")) defer stop() - require.Equal(t, originDefault, tracer.config.traceSampleRate.cfgOrigin) + require.Equal(t, telemetry.OriginDefault, tracer.config.traceSampleRate.cfgOrigin) input := remoteconfig.ProductUpdate{ "path": []byte(`{"lib_config": {}, "service_target": {"service": "other-service", "env": "my-env"}}`), @@ -590,7 +590,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { tracer, _, _, stop := startTestTracer(t, WithServiceName("my-service"), WithEnv("my-env")) defer stop() - require.Equal(t, originDefault, tracer.config.traceSampleRate.cfgOrigin) + require.Equal(t, telemetry.OriginDefault, tracer.config.traceSampleRate.cfgOrigin) input := remoteconfig.ProductUpdate{ "path": []byte(`{"lib_config": {}, "service_target": {"service": "my-service", "env": "other-env"}}`), @@ -616,7 +616,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { ) defer stop() - require.Equal(t, originDefault, tracer.config.traceSampleRate.cfgOrigin) + require.Equal(t, telemetry.OriginDefault, tracer.config.traceSampleRate.cfgOrigin) // Apply RC. Assert global tags have the RC tags key3:val3,key4:val4 applied + runtime ID input := remoteconfig.ProductUpdate{ @@ -642,7 +642,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { t, "ConfigChange", []telemetry.Configuration{ - {Name: "trace_tags", Value: "key3:val3,key4:val4," + runtimeIDTag, Origin: originRemoteConfig}, + {Name: "trace_tags", Value: "key3:val3,key4:val4," + runtimeIDTag, Origin: telemetry.OriginRemoteConfig}, }, ) @@ -667,7 +667,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { t, "ConfigChange", []telemetry.Configuration{ - {Name: "trace_tags", Value: "key0:val0,key1:val1,key2:val2," + runtimeIDTag, Origin: "default"}, + {Name: "trace_tags", Value: "key0:val0,key1:val1,key2:val2," + runtimeIDTag, Origin: telemetry.OriginDefault}, }, ) }) @@ -683,7 +683,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { tracer, _, _, stop := startTestTracer(t, WithService("my-service"), WithEnv("my-env")) defer stop() - require.Equal(t, originEnvVar, tracer.config.traceSampleRate.cfgOrigin) + require.Equal(t, telemetry.OriginEnvVar, tracer.config.traceSampleRate.cfgOrigin) // Apply RC. Assert configuration is updated to the RC values. input := remoteconfig.ProductUpdate{ @@ -702,12 +702,12 @@ func TestOnRemoteConfigUpdate(t *testing.T) { // Telemetry telemetryClient.AssertNumberOfCalls(t, "ConfigChange", 1) telemetryClient.AssertCalled(t, "ConfigChange", []telemetry.Configuration{ - {Name: "trace_sample_rate", Value: 0.2, Origin: originRemoteConfig}, - {Name: "trace_header_tags", Value: "X-Test-Header:my-tag-from-rc", Origin: originRemoteConfig}, + {Name: "trace_sample_rate", Value: 0.2, Origin: telemetry.OriginRemoteConfig}, + {Name: "trace_header_tags", Value: "X-Test-Header:my-tag-from-rc", Origin: telemetry.OriginRemoteConfig}, { Name: "trace_tags", Value: "ddtag:from-rc," + ext.RuntimeID + ":" + globalconfig.RuntimeID(), - Origin: originRemoteConfig, + Origin: telemetry.OriginRemoteConfig, }, }) @@ -724,9 +724,9 @@ func TestOnRemoteConfigUpdate(t *testing.T) { // Telemetry telemetryClient.AssertNumberOfCalls(t, "ConfigChange", 2) telemetryClient.AssertCalled(t, "ConfigChange", []telemetry.Configuration{ - {Name: "trace_sample_rate", Value: 0.1, Origin: originDefault}, - {Name: "trace_header_tags", Value: "X-Test-Header:my-tag-from-env", Origin: originDefault}, - {Name: "trace_tags", Value: "ddtag:from-env," + ext.RuntimeID + ":" + globalconfig.RuntimeID(), Origin: originDefault}, + {Name: "trace_sample_rate", Value: 0.1, Origin: telemetry.OriginDefault}, + {Name: "trace_header_tags", Value: "X-Test-Header:my-tag-from-env", Origin: telemetry.OriginDefault}, + {Name: "trace_tags", Value: "ddtag:from-env," + ext.RuntimeID + ":" + globalconfig.RuntimeID(), Origin: telemetry.OriginDefault}, }) }) diff --git a/ddtrace/tracer/tracer.go b/ddtrace/tracer/tracer.go index e4f65c082d..1453f0a450 100644 --- a/ddtrace/tracer/tracer.go +++ b/ddtrace/tracer/tracer.go @@ -258,7 +258,7 @@ func newUnstartedTracer(opts ...StartOption) *tracer { // to distinguish between the case where the environment variable was not set and the case where // it default to NaN. if !math.IsNaN(globalRate) { - c.traceSampleRate.cfgOrigin = originEnvVar + c.traceSampleRate.cfgOrigin = telemetry.OriginEnvVar } c.traceSampleRules = newDynamicConfig("trace_sample_rules", c.traceRules, rulesSampler.traces.setTraceSampleRules, EqualsFalseNegative) diff --git a/internal/appsec/config/config.go b/internal/appsec/config/config.go index d7cf538881..33ca45e56b 100644 --- a/internal/appsec/config/config.go +++ b/internal/appsec/config/config.go @@ -37,7 +37,7 @@ func registerSCAAppConfigTelemetry(client telemetry.Client) { return } if defined { - client.RegisterAppConfig(EnvSCAEnabled, val, "env_var") + client.RegisterAppConfig(EnvSCAEnabled, val, telemetry.OriginEnvVar) } } diff --git a/internal/appsec/telemetry.go b/internal/appsec/telemetry.go index aeac5827d2..2b07117bd8 100644 --- a/internal/appsec/telemetry.go +++ b/internal/appsec/telemetry.go @@ -26,10 +26,10 @@ var ( wafSupported, _ = waf.SupportsTarget() wafHealthy, _ = waf.Health() staticConfigs = []telemetry.Configuration{ - {Name: "goos", Value: runtime.GOOS, Origin: "code"}, - {Name: "goarch", Value: runtime.GOARCH, Origin: "code"}, - {Name: "waf_supports_target", Value: wafSupported, Origin: "code"}, - {Name: "waf_healthy", Value: wafHealthy, Origin: "code"}, + {Name: "goos", Value: runtime.GOOS, Origin: telemetry.OriginCode}, + {Name: "goarch", Value: runtime.GOARCH, Origin: telemetry.OriginCode}, + {Name: "waf_supports_target", Value: wafSupported, Origin: telemetry.OriginCode}, + {Name: "waf_healthy", Value: wafHealthy, Origin: telemetry.OriginCode}, } ) @@ -62,7 +62,7 @@ func (a *appsecTelemetry) addEnvConfig(name string, value any) { if a == nil { return } - a.configs = append(a.configs, telemetry.Configuration{Name: name, Value: value, Origin: "env_var"}) + a.configs = append(a.configs, telemetry.Configuration{Name: name, Value: value, Origin: telemetry.OriginEnvVar}) } // setEnabled makes AppSec as having effectively been enabled. diff --git a/internal/telemetry/client.go b/internal/telemetry/client.go index 1613c5f911..1b01ced31f 100644 --- a/internal/telemetry/client.go +++ b/internal/telemetry/client.go @@ -30,7 +30,7 @@ import ( // Client buffers and sends telemetry messages to Datadog (possibly through an // agent). type Client interface { - RegisterAppConfig(name string, val interface{}, origin string) + RegisterAppConfig(name string, val interface{}, origin Origin) ProductChange(namespace Namespace, enabled bool, configuration []Configuration) ConfigChange(configuration []Configuration) Record(namespace Namespace, metric MetricKind, name string, value float64, tags []string, common bool) @@ -158,7 +158,7 @@ func log(msg string, args ...interface{}) { // RegisterAppConfig allows to register a globally-defined application configuration. // This configuration will be sent when the telemetry client is started and over related configuration updates. -func (c *client) RegisterAppConfig(name string, value interface{}, origin string) { +func (c *client) RegisterAppConfig(name string, value interface{}, origin Origin) { c.globalAppConfig = append(c.globalAppConfig, Configuration{ Name: name, Value: value, diff --git a/internal/telemetry/message.go b/internal/telemetry/message.go index 3348187596..27a72768d9 100644 --- a/internal/telemetry/message.go +++ b/internal/telemetry/message.go @@ -5,7 +5,11 @@ package telemetry -import "net/http" +import ( + "bytes" + "fmt" + "net/http" +) // Request captures all necessary information for a telemetry event submission type Request struct { @@ -132,13 +136,48 @@ type ConfigurationChange struct { RemoteConfig *RemoteConfig `json:"remote_config,omitempty"` } +type Origin int + +const ( + OriginDefault Origin = iota + OriginCode + OriginDDConfig + OriginEnvVar + OriginRemoteConfig +) + +func (o Origin) String() string { + switch o { + case OriginDefault: + return "default" + case OriginCode: + return "code" + case OriginDDConfig: + return "dd_config" + case OriginEnvVar: + return "env_var" + case OriginRemoteConfig: + return "remote_config" + default: + return fmt.Sprintf("unknown origin %d", o) + } +} + +func (o Origin) MarshalJSON() ([]byte, error) { + var b bytes.Buffer + b.WriteString(`"`) + b.WriteString(o.String()) + b.WriteString(`"`) + return b.Bytes(), nil +} + // Configuration is a library-specific configuration value // that should be initialized through StringConfig, IntConfig, FloatConfig, or BoolConfig type Configuration struct { Name string `json:"name"` Value interface{} `json:"value"` - // origin is the source of the config. It is one of {env_var, code, dd_config, remote_config} - Origin string `json:"origin"` + // origin is the source of the config. It is one of {default, env_var, code, dd_config, remote_config}. + Origin Origin `json:"origin"` Error Error `json:"error"` IsOverriden bool `json:"is_overridden"` } diff --git a/internal/telemetry/telemetry_test.go b/internal/telemetry/telemetry_test.go index e7c917100b..2666a91c44 100644 --- a/internal/telemetry/telemetry_test.go +++ b/internal/telemetry/telemetry_test.go @@ -142,27 +142,27 @@ func TestProductChange(t *testing.T) { // Test that globally registered app config is sent in telemetry requests including the configuration state. func TestRegisterAppConfig(t *testing.T) { client := new(client) - client.RegisterAppConfig("key1", "val1", "origin1") + client.RegisterAppConfig("key1", "val1", OriginDefault) // Test that globally registered app config is sent in app-started payloads - client.start([]Configuration{{Name: "key2", Value: "val2", Origin: "origin2"}}, NamespaceTracers, false) + client.start([]Configuration{{Name: "key2", Value: "val2", Origin: OriginDDConfig}}, NamespaceTracers, false) req := client.requests[0].Body require.Equal(t, RequestTypeAppStarted, req.RequestType) appStarted := req.Payload.(*AppStarted) cfg := appStarted.Configuration require.Len(t, cfg, 2) - require.Contains(t, cfg, Configuration{Name: "key1", Value: "val1", Origin: "origin1"}) - require.Contains(t, cfg, Configuration{Name: "key2", Value: "val2", Origin: "origin2"}) + require.Contains(t, cfg, Configuration{Name: "key1", Value: "val1", Origin: OriginDefault}) + require.Contains(t, cfg, Configuration{Name: "key2", Value: "val2", Origin: OriginDDConfig}) // Test that globally registered app config is sent in app-client-configuration-change payloads - client.ProductChange(NamespaceTracers, true, []Configuration{{Name: "key3", Value: "val3", Origin: "origin3"}}) + client.ProductChange(NamespaceTracers, true, []Configuration{{Name: "key3", Value: "val3", Origin: OriginCode}}) req = client.requests[2].Body require.Equal(t, RequestTypeAppClientConfigurationChange, req.RequestType) appConfigChange := req.Payload.(*ConfigurationChange) cfg = appConfigChange.Configuration require.Len(t, cfg, 2) - require.Contains(t, cfg, Configuration{Name: "key1", Value: "val1", Origin: "origin1"}) - require.Contains(t, cfg, Configuration{Name: "key3", Value: "val3", Origin: "origin3"}) + require.Contains(t, cfg, Configuration{Name: "key1", Value: "val1", Origin: OriginDefault}) + require.Contains(t, cfg, Configuration{Name: "key3", Value: "val3", Origin: OriginCode}) } diff --git a/internal/telemetry/telemetrytest/telemetrytest.go b/internal/telemetry/telemetrytest/telemetrytest.go index 9a0df1b78c..0c8b22bd32 100644 --- a/internal/telemetry/telemetrytest/telemetrytest.go +++ b/internal/telemetry/telemetrytest/telemetrytest.go @@ -27,7 +27,7 @@ type MockClient struct { Metrics map[telemetry.Namespace]map[string]float64 } -func (c *MockClient) RegisterAppConfig(name string, val interface{}, origin string) { +func (c *MockClient) RegisterAppConfig(name string, val interface{}, origin telemetry.Origin) { _ = c.Called(name, val, origin) } From db13ecdc0305ecebff93c161ca07b52d6c98e799 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Tue, 28 May 2024 16:56:48 +0200 Subject: [PATCH 05/12] internal/appsec/config: adapt mock to expect telemetry.Origin* value --- internal/appsec/config/config_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/appsec/config/config_test.go b/internal/appsec/config/config_test.go index 19dec18669..77d29e36df 100644 --- a/internal/appsec/config/config_test.go +++ b/internal/appsec/config/config_test.go @@ -8,6 +8,7 @@ package config import ( "testing" + "gopkg.in/DataDog/dd-trace-go.v1/internal/telemetry" "gopkg.in/DataDog/dd-trace-go.v1/internal/telemetry/telemetrytest" ) @@ -49,12 +50,12 @@ func TestSCAEnabled(t *testing.T) { } telemetryClient := new(telemetrytest.MockClient) - telemetryClient.On("RegisterAppConfig", EnvSCAEnabled, tc.expectedValue, "env_var").Return() + telemetryClient.On("RegisterAppConfig", EnvSCAEnabled, tc.expectedValue, telemetry.OriginEnvVar).Return() registerSCAAppConfigTelemetry(telemetryClient) if tc.telemetryExpected { - telemetryClient.AssertCalled(t, "RegisterAppConfig", EnvSCAEnabled, tc.expectedValue, "env_var") + telemetryClient.AssertCalled(t, "RegisterAppConfig", EnvSCAEnabled, tc.expectedValue, telemetry.OriginEnvVar) telemetryClient.AssertNumberOfCalls(t, "RegisterAppConfig", 1) } else { telemetryClient.AssertNumberOfCalls(t, "RegisterAppConfig", 0) From 00686ffa65064852fe8c092546246982fa5fa31e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Wed, 29 May 2024 10:21:39 +0200 Subject: [PATCH 06/12] ddtrace/tracer: set global tags origin to env_var if DD_TAGS is present --- ddtrace/tracer/option.go | 4 +++- ddtrace/tracer/remote_config_test.go | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/ddtrace/tracer/option.go b/ddtrace/tracer/option.go index 98b048330c..da9e5d7a15 100644 --- a/ddtrace/tracer/option.go +++ b/ddtrace/tracer/option.go @@ -347,6 +347,8 @@ func newConfig(opts ...StartOption) *config { for key, val := range tags { WithGlobalTag(key, val)(c) } + // TODO: track the origin of these tags individually + c.globalTags.cfgOrigin = telemetry.OriginEnvVar } if _, ok := os.LookupEnv("AWS_LAMBDA_FUNCTION_NAME"); ok { // AWS_LAMBDA_FUNCTION_NAME being set indicates that we're running in an AWS Lambda environment. @@ -916,7 +918,7 @@ func (c *config) initGlobalTags(init map[string]interface{}) { c.globalTags.current[ext.RuntimeID] = globalconfig.RuntimeID() return true } - c.globalTags = newDynamicConfig[map[string]interface{}]("trace_tags", init, apply, equalMap[string]) + c.globalTags = newDynamicConfig("trace_tags", init, apply, equalMap[string]) } // WithSampler sets the given sampler to be used with the tracer. By default diff --git a/ddtrace/tracer/remote_config_test.go b/ddtrace/tracer/remote_config_test.go index 769b978b8a..be6b1f6c7b 100644 --- a/ddtrace/tracer/remote_config_test.go +++ b/ddtrace/tracer/remote_config_test.go @@ -726,7 +726,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { telemetryClient.AssertCalled(t, "ConfigChange", []telemetry.Configuration{ {Name: "trace_sample_rate", Value: 0.1, Origin: telemetry.OriginDefault}, {Name: "trace_header_tags", Value: "X-Test-Header:my-tag-from-env", Origin: telemetry.OriginDefault}, - {Name: "trace_tags", Value: "ddtag:from-env," + ext.RuntimeID + ":" + globalconfig.RuntimeID(), Origin: telemetry.OriginDefault}, + {Name: "trace_tags", Value: "ddtag:from-env," + ext.RuntimeID + ":" + globalconfig.RuntimeID(), Origin: telemetry.OriginEnvVar}, }) }) From bda935a868cc70245dd862a379707c2fa47f924f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Wed, 29 May 2024 11:04:07 +0200 Subject: [PATCH 07/12] ddtrace/tracer: preserve global tags origin on config initialization --- ddtrace/tracer/option.go | 10 ++++++---- ddtrace/tracer/remote_config_test.go | 1 + 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/ddtrace/tracer/option.go b/ddtrace/tracer/option.go index da9e5d7a15..dcba261d5e 100644 --- a/ddtrace/tracer/option.go +++ b/ddtrace/tracer/option.go @@ -347,7 +347,7 @@ func newConfig(opts ...StartOption) *config { for key, val := range tags { WithGlobalTag(key, val)(c) } - // TODO: track the origin of these tags individually + // TODO: should we track the origin of these tags individually? c.globalTags.cfgOrigin = telemetry.OriginEnvVar } if _, ok := os.LookupEnv("AWS_LAMBDA_FUNCTION_NAME"); ok { @@ -514,7 +514,8 @@ func newConfig(opts ...StartOption) *config { } // Re-initialize the globalTags config with the value constructed from the environment and start options // This allows persisting the initial value of globalTags for future resets and updates. - c.initGlobalTags(c.globalTags.get()) + globalTagsOrigin := c.globalTags.cfgOrigin + c.initGlobalTags(c.globalTags.get(), globalTagsOrigin) return c } @@ -903,7 +904,7 @@ func WithPeerServiceMapping(from, to string) StartOption { func WithGlobalTag(k string, v interface{}) StartOption { return func(c *config) { if c.globalTags.get() == nil { - c.initGlobalTags(map[string]interface{}{}) + c.initGlobalTags(map[string]interface{}{}, telemetry.OriginDefault) } c.globalTags.Lock() defer c.globalTags.Unlock() @@ -912,13 +913,14 @@ func WithGlobalTag(k string, v interface{}) StartOption { } // initGlobalTags initializes the globalTags config with the provided init value -func (c *config) initGlobalTags(init map[string]interface{}) { +func (c *config) initGlobalTags(init map[string]interface{}, origin telemetry.Origin) { apply := func(map[string]interface{}) bool { // always set the runtime ID on updates c.globalTags.current[ext.RuntimeID] = globalconfig.RuntimeID() return true } c.globalTags = newDynamicConfig("trace_tags", init, apply, equalMap[string]) + c.globalTags.cfgOrigin = origin } // WithSampler sets the given sampler to be used with the tracer. By default diff --git a/ddtrace/tracer/remote_config_test.go b/ddtrace/tracer/remote_config_test.go index be6b1f6c7b..7caa71c978 100644 --- a/ddtrace/tracer/remote_config_test.go +++ b/ddtrace/tracer/remote_config_test.go @@ -617,6 +617,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { defer stop() require.Equal(t, telemetry.OriginDefault, tracer.config.traceSampleRate.cfgOrigin) + require.Equal(t, telemetry.OriginEnvVar, tracer.config.globalTags.cfgOrigin) // Apply RC. Assert global tags have the RC tags key3:val3,key4:val4 applied + runtime ID input := remoteconfig.ProductUpdate{ From ae27eba9cd40826225249a1f7dcb79eb13e9edb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Wed, 29 May 2024 11:20:15 +0200 Subject: [PATCH 08/12] ddtrace/tracer: fix TestOnRemoteConfigUpdate/Deleted_config test --- ddtrace/tracer/remote_config_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddtrace/tracer/remote_config_test.go b/ddtrace/tracer/remote_config_test.go index 7caa71c978..3e5e4d881e 100644 --- a/ddtrace/tracer/remote_config_test.go +++ b/ddtrace/tracer/remote_config_test.go @@ -727,7 +727,7 @@ func TestOnRemoteConfigUpdate(t *testing.T) { telemetryClient.AssertCalled(t, "ConfigChange", []telemetry.Configuration{ {Name: "trace_sample_rate", Value: 0.1, Origin: telemetry.OriginDefault}, {Name: "trace_header_tags", Value: "X-Test-Header:my-tag-from-env", Origin: telemetry.OriginDefault}, - {Name: "trace_tags", Value: "ddtag:from-env," + ext.RuntimeID + ":" + globalconfig.RuntimeID(), Origin: telemetry.OriginEnvVar}, + {Name: "trace_tags", Value: "ddtag:from-env," + ext.RuntimeID + ":" + globalconfig.RuntimeID(), Origin: telemetry.OriginDefault}, }) }) From e9c4dd8da6ae3b1ffbf33ac90c863045f502ccdc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Wed, 29 May 2024 11:49:51 +0200 Subject: [PATCH 09/12] ddtrace/tracer: set trace_enabled origin to env_var if DD_TRACE_ENABLED is present --- ddtrace/tracer/option.go | 3 +++ ddtrace/tracer/option_test.go | 3 +++ 2 files changed, 6 insertions(+) diff --git a/ddtrace/tracer/option.go b/ddtrace/tracer/option.go index dcba261d5e..30352a9ba0 100644 --- a/ddtrace/tracer/option.go +++ b/ddtrace/tracer/option.go @@ -359,6 +359,9 @@ func newConfig(opts ...StartOption) *config { c.runtimeMetrics = internal.BoolEnv("DD_RUNTIME_METRICS_ENABLED", false) c.debug = internal.BoolEnv("DD_TRACE_DEBUG", false) c.enabled = newDynamicConfig("tracing_enabled", internal.BoolEnv("DD_TRACE_ENABLED", true), func(b bool) bool { return true }, equal[bool]) + if _, ok := os.LookupEnv("DD_TRACE_ENABLED"); ok { + c.enabled.cfgOrigin = telemetry.OriginEnvVar + } c.profilerEndpoints = internal.BoolEnv(traceprof.EndpointEnvVar, true) c.profilerHotspots = internal.BoolEnv(traceprof.CodeHotspotsEnvVar, true) c.enableHostnameDetection = internal.BoolEnv("DD_CLIENT_HOSTNAME_ENABLED", true) diff --git a/ddtrace/tracer/option_test.go b/ddtrace/tracer/option_test.go index 71eaac7587..55d7a65487 100644 --- a/ddtrace/tracer/option_test.go +++ b/ddtrace/tracer/option_test.go @@ -29,6 +29,7 @@ import ( "gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext" "gopkg.in/DataDog/dd-trace-go.v1/internal/globalconfig" "gopkg.in/DataDog/dd-trace-go.v1/internal/namingschema" + "gopkg.in/DataDog/dd-trace-go.v1/internal/telemetry" "gopkg.in/DataDog/dd-trace-go.v1/internal/traceprof" "gopkg.in/DataDog/dd-trace-go.v1/internal/version" @@ -548,6 +549,7 @@ func TestTracerOptionsDefaults(t *testing.T) { defer tracer.Stop() c := tracer.config assert.True(t, c.enabled.current) + assert.Equal(t, c.enabled.cfgOrigin, telemetry.OriginDefault) }) t.Run("override", func(t *testing.T) { @@ -556,6 +558,7 @@ func TestTracerOptionsDefaults(t *testing.T) { defer tracer.Stop() c := tracer.config assert.False(t, c.enabled.current) + assert.Equal(t, c.enabled.cfgOrigin, telemetry.OriginEnvVar) }) }) From e03f61036d17c3e34f3a042e161f8044b3f2e71c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Wed, 29 May 2024 12:06:29 +0200 Subject: [PATCH 10/12] ddtrace/tracer: report trace_enabled with dynamicConfig#toTelemetry --- ddtrace/tracer/telemetry.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddtrace/tracer/telemetry.go b/ddtrace/tracer/telemetry.go index 3c9b635fb7..1978ebd55e 100644 --- a/ddtrace/tracer/telemetry.go +++ b/ddtrace/tracer/telemetry.go @@ -54,7 +54,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}, + c.enabled.toTelemetry(), c.traceSampleRate.toTelemetry(), c.headerAsTags.toTelemetry(), c.globalTags.toTelemetry(), From aaae6a72b76b31dc88c71f29cab098bb972a95ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Wed, 29 May 2024 12:22:40 +0200 Subject: [PATCH 11/12] ddtrace/tracer: revert to report trace_enabled with a telemetry.Configuration literal Our telemetry expects trace_enabled for config, but config#enabled uses tracing_enabled because it's expected in lib_config key. --- ddtrace/tracer/telemetry.go | 2 +- ddtrace/tracer/telemetry_test.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/ddtrace/tracer/telemetry.go b/ddtrace/tracer/telemetry.go index 1978ebd55e..1f112ff830 100644 --- a/ddtrace/tracer/telemetry.go +++ b/ddtrace/tracer/telemetry.go @@ -54,7 +54,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}, - c.enabled.toTelemetry(), + {Name: "trace_enabled", Value: c.enabled.current, Origin: c.enabled.cfgOrigin}, c.traceSampleRate.toTelemetry(), c.headerAsTags.toTelemetry(), c.globalTags.toTelemetry(), diff --git a/ddtrace/tracer/telemetry_test.go b/ddtrace/tracer/telemetry_test.go index f64c5e4f80..09e10cf638 100644 --- a/ddtrace/tracer/telemetry_test.go +++ b/ddtrace/tracer/telemetry_test.go @@ -47,6 +47,7 @@ func TestTelemetryEnabled(t *testing.T) { telemetry.Check(t, telemetryClient.Configuration, "env", "test-env") telemetry.Check(t, telemetryClient.Configuration, "runtime_metrics_enabled", true) telemetry.Check(t, telemetryClient.Configuration, "stats_computation_enabled", false) + telemetry.Check(t, telemetryClient.Configuration, "trace_enabled", true) telemetry.Check(t, telemetryClient.Configuration, "trace_span_attribute_schema", 0) telemetry.Check(t, telemetryClient.Configuration, "trace_peer_service_defaults_enabled", true) telemetry.Check(t, telemetryClient.Configuration, "trace_peer_service_mapping", "key:val") From 8db3fa2f343837323b903035bc10374d60c0c978 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Thu, 30 May 2024 09:22:51 +0200 Subject: [PATCH 12/12] .github/workflows: revert parametric tests to inputs.ref --- .github/workflows/parametric-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/parametric-tests.yml b/.github/workflows/parametric-tests.yml index 6d6cdb3317..eea919e7ce 100644 --- a/.github/workflows/parametric-tests.yml +++ b/.github/workflows/parametric-tests.yml @@ -33,7 +33,7 @@ jobs: uses: actions/checkout@v3 with: repository: 'DataDog/system-tests' - ref: kylev/enable-config-tests + ref: ${{ inputs.ref }} - name: Checkout dd-trace-go uses: actions/checkout@v3