Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ddtrace/tracer: support default origin on dynamic config to support Active Tracing telemetry spec #2623

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/parametric-tests.yml
Expand Up @@ -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
Expand Down
22 changes: 15 additions & 7 deletions ddtrace/tracer/dynamic_config.go
Expand Up @@ -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).
Expand All @@ -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,
}
}

Expand Down Expand Up @@ -61,15 +68,16 @@ 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)
}

// handleRC processes a new configuration value from remote config
// 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()
}
Expand Down
4 changes: 3 additions & 1 deletion ddtrace/tracer/option.go
Expand Up @@ -336,7 +336,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)
Copy link
Contributor

@mtoffl01 mtoffl01 Mar 22, 2024

Choose a reason for hiding this comment

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

This just made me wonder whether the other settings that are available through RC should also get origin: originEnvVar on their dynamicConfig when enabled via env var, e.g, DD_TRACE_ENABLED
Maybe newDynamicConfig() should accept a value for origin, or maybe these we should also call dynamicConfig.update(val, originEnvVar) for these other options

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mtoffl01 You are right. I left this one as example and reminder to add more in a separate PR. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with this. Ideally the constructor should set the origin.

If for some reason we can't use the constructor to do so, the generic dynamicConfig type should provide a method to update the startup and current values and their origins instead of manipulating the fields directly in ddtrace/tracer/option.go.

// 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)
Expand Down
68 changes: 45 additions & 23 deletions ddtrace/tracer/remote_config_test.go
Expand Up @@ -30,6 +30,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"}}`),
Expand All @@ -42,7 +44,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}})
darccio marked this conversation as resolved.
Show resolved Hide resolved

//Apply RC with sampling rules. Assert _dd.rule_psr shows the corresponding rule matched rate.
input = remoteconfig.ProductUpdate{
Expand Down Expand Up @@ -92,6 +94,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"}}`),
Expand All @@ -104,7 +108,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"}}`)}
Expand All @@ -116,7 +120,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 = 1.0", func(t *testing.T) {
Expand All @@ -132,6 +136,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])
Expand Down Expand Up @@ -168,8 +174,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":1,"provenance":"customer"}]`, 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":1,"provenance":"customer"}]`, Origin: originRemoteConfig}})
})

t.Run("DD_TRACE_SAMPLING_RULES=0.1 and RC rule rate=1.0 and revert", func(t *testing.T) {
Expand Down Expand Up @@ -256,6 +262,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"}}`),
Expand All @@ -266,7 +274,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"}}`)}
Expand All @@ -276,22 +284,22 @@ 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) {
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)
Expand Down Expand Up @@ -338,6 +346,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"}}`),
Expand All @@ -349,7 +359,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"}}`)}
Expand All @@ -360,7 +370,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) {
Expand All @@ -371,6 +381,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"}}`),
Expand All @@ -382,7 +394,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"}}`)}
Expand All @@ -393,7 +405,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) {
Expand All @@ -403,6 +415,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"}}}`),
}
Expand All @@ -421,6 +435,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"}}`),
}
Expand All @@ -439,6 +455,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"}}`),
}
Expand All @@ -458,6 +476,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"}}`),
Expand All @@ -476,7 +496,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"}}`)}
Expand All @@ -493,7 +513,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) {
Expand All @@ -507,6 +527,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"}}`),
Expand All @@ -522,9 +544,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.
Expand All @@ -540,9 +562,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},
})
})

Expand Down
8 changes: 8 additions & 0 deletions ddtrace/tracer/tracer.go
Expand Up @@ -8,6 +8,7 @@ package tracer
import (
gocontext "context"
"encoding/binary"
"math"
"os"
"runtime/pprof"
rt "runtime/trace"
Expand Down Expand Up @@ -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
Expand Down