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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 19 commits into from
May 30, 2024
Merged

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

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
c65f534
ddtrace/tracer: support default origin on dynamic config to support A…
darccio Mar 20, 2024
40be10c
Merge branch 'main' into dario.castane/AIT-9990-default-origin
darccio Apr 3, 2024
430d876
.github/workflows: use specific branch to test new parametric tests
darccio Apr 4, 2024
46bb463
ddtrace/tracer: set trace_sample_rate origin to env_var if present
darccio Apr 4, 2024
a08e194
Merge branch 'main' into dario.castane/AIT-9990-default-origin
darccio May 3, 2024
8eb8c40
Merge remote-tracking branch 'origin' into dario.castane/AIT-9990-def…
darccio May 28, 2024
fab626b
internal/telemetry: change Configuration#Origin from string to enum
darccio May 28, 2024
db13ecd
internal/appsec/config: adapt mock to expect telemetry.Origin* value
darccio May 28, 2024
a4cc1da
Merge branch 'main' into dario.castane/AIT-9990-default-origin
darccio May 28, 2024
00686ff
ddtrace/tracer: set global tags origin to env_var if DD_TAGS is present
darccio May 29, 2024
54960d3
Merge branch 'dario.castane/AIT-9990-default-origin' of github.com:Da…
darccio May 29, 2024
bda935a
ddtrace/tracer: preserve global tags origin on config initialization
darccio May 29, 2024
ae27eba
ddtrace/tracer: fix TestOnRemoteConfigUpdate/Deleted_config test
darccio May 29, 2024
e9c4dd8
ddtrace/tracer: set trace_enabled origin to env_var if DD_TRACE_ENABL…
darccio May 29, 2024
e03f610
ddtrace/tracer: report trace_enabled with dynamicConfig#toTelemetry
darccio May 29, 2024
aaae6a7
ddtrace/tracer: revert to report trace_enabled with a telemetry.Confi…
darccio May 29, 2024
a4bdef2
Merge branch 'main' into dario.castane/AIT-9990-default-origin
darccio May 30, 2024
8db3fa2
.github/workflows: revert parametric tests to inputs.ref
darccio May 30, 2024
81d4078
Merge branch 'dario.castane/AIT-9990-default-origin' of github.com:Da…
darccio May 30, 2024
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
22 changes: 15 additions & 7 deletions ddtrace/tracer/dynamic_config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,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 •
edited

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
38 changes: 19 additions & 19 deletions ddtrace/tracer/remote_config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}})
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 @@ -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"}}`)}
Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand All @@ -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"}}`)}
Expand All @@ -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) {
Expand Down Expand Up @@ -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"}}`)}
Expand All @@ -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) {
Expand All @@ -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"}}`)}
Expand All @@ -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) {
Expand Down Expand Up @@ -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"}}`)}
Expand All @@ -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) {
Expand Down Expand Up @@ -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.
Expand All @@ -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},
})
})

Expand Down