Support file-based configurations (Phase 1) by mtoffl01 · Pull Request #3438 · DataDog/dd-trace-go

mtoffl01 · 2025-04-15T20:11:54Z

What does this PR do?

Support reading product-enablement configurations from user-managed (/etc/datadog-agent/application_monitoring.yaml) and fleet-managed (/etc/datadog-agent/managed/datadog-agent/stable/application_monitoring.yaml) yaml files for dd-trace-go. In-scope configurations include:

DD_APM_TRACING_ENABLED
DD_RUNTIME_METRICS_ENABLED
DD_LOGS_INJECTION
DD_PROFILING_ENABLED
DD_DATA_STREAMS_ENABLED
DD_APPSEC_ENABLED
DD_IAST_ENABLED
DD_DYNAMIC_INSTRUMENTATION_ENABLED
DD_DATA_JOBS_ENABLED
DD_APPSEC_SCA_ENABLED
DD_TRACE_DEBUG

RFC: Stable Configuration Phase 1

Notes

All TODOs for APMAPI-1358 are just markers for the next phase of work, where we report Origin to telemetry.
Orchestrion support/tests are out of scope for this PR.

Testing

This PR passes all system-tests as per: DataDog/system-tests#4604. To pass the system-tests, I had to update the startup log to include data streams info.

Motivation

Reviewer's Checklist

Changed code has unit tests for its functionality at or near 100% coverage.
System-Tests covering this feature have been added and enabled with the va.b.c-dev version tag.
There is a benchmark for any new code, or changes to existing code.
If this interacts with the agent in a new way, a system test has been added.
New code is free of linting errors. You can check this by running golangci-lint run locally.
Add an appropriate team label so this PR gets put in the right place for the release notes.
Non-trivial go.mod changes, e.g. adding new modules, are reviewed by @DataDog/dd-trace-go-guild.

Unsure? Have a question? Request a review!

…nfigsource types, fileContentsToConfig tests

datadog-datadog-prod-us1 · 2025-04-15T20:19:27Z

Datadog Report

Branch report: mtoff/v2-stable_cfg_1
Commit report: 2809bc1
Test service: dd-trace-go

✅ 0 Failed, 4562 Passed, 65 Skipped, 3m 16.41s Total Time

pr-commenter · 2025-04-15T20:41:40Z

Benchmarks

Benchmark execution time: 2025-05-27 20:29:50

Comparing candidate commit 631d655 in PR branch mtoff/v2-stable_cfg_1 with baseline commit 1d6075b in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 21 metrics, 0 unstable metrics.

…es. Plus, apply stable config check to all phase 1 env vars, except for DD_APSEC_ENABLED

…bleConfig

…an configured

mtoffl01 · 2025-04-28T17:53:38Z

+	"github.com/DataDog/dd-trace-go/v2/internal/telemetry"
+)
+
+func BoolStableConfig(env string, def bool) (value bool, origin telemetry.Origin, err error) {


Should BoolStableConfig and StringStableConfig live in internal/env.go instead?

nsrip-dd

Approving for the profiling portion of the change. Let me know if you want me to take a closer look at the rest.

…#3520)

eliottness

Awesome work. Excited to test it 👍

eliottness · 2025-05-20T14:07:06Z

+	localFilePath   = "/etc/datadog-agent/application_monitoring.yaml"
+	managedFilePath = "/etc/datadog-agent/managed/datadog-agent/stable/application_monitoring.yaml"


Will this work on Windows ?

No. Support is for linux, only (ref). Thanks to your comment, I've now called this out on line 19.

This would probably warrant a test or at least a manual check in our windows CI to make we dont spit any uncomprehensible errors/logs for windows users. If it is the case then adding a if runtime.GOOS == "linux" somewhere could be something. WDYT ?

eliottness · 2025-05-20T14:30:16Z

+		if val, err := strconv.ParseBool(v); err == nil {
+			return val, o, nil
+		}
+		err = fmt.Errorf("non-boolean value for %s: '%s' in %s configuration, dropping", env, v, o)


I would say it makes sense to use errors.Join if there is multiple errors here.
Also maybe adding a special case to not report an error if val is an empty string can also make sense I believe

eliottness · 2025-05-20T14:32:13Z

Would it be possible to have functions for duration, for int, for float and for net.IP to cover all cases that where previously convered by internal/env.go?

I am also wondering if we should call telemetry.RegisterAppConfig directly here. What do you think?

@eliottness ,

Would it be possible to have functions for duration, for int, for float and for net.IP...

Yes, I will add this in a separate PR, so that this work is not blocked! #3537

I am also wondering if we should call telemetry.RegisterAppConfig directly here. What do you think?

Good idea. Telemetry will be added in a separate PR, as part of Phase "1.5," also scheduled for this quarter (by contrast, sourcing stable configurations is "Phase 1").

…in stableconfigsource, and add test for windows

mtoffl01 added 4 commits April 15, 2025 16:09

Initial implementation: stableconfig package, stableconfig & stableco…

fa3275e

…nfigsource types, fileContentsToConfig tests

Add TestParseFile tests

2eef6d3

Apply stable config to dd_runtime_metrics_enabled

1f174c8

Apply stable config to DD_APM_TRACING_ENABLED

8c37ce7

mtoffl01 added 20 commits April 16, 2025 11:11

fix stableconfig isEmpty call

6c5cf1f

Create helper fn to resolve config from fleet, envvar and local sourc…

aa3a58a

…es. Plus, apply stable config check to all phase 1 env vars, except for DD_APSEC_ENABLED

Revert stable config on SCAEnabled

2039b95

Fix inverted err nil condition in BoolStableConfig

b732414

Fix inverted err nil condition in BoolStableConfig

55c32ef

Apply stableconfig to dd_profiling_enabled=auto condition

40ed9f6

Apply stable config to DD_APPSEC_SCA_ENABLED

db2ff5c

Modify BoolStableConfig to return whether configuration was set or not

101aa9e

initialize logger in stableconfig

c41b2fc

nits: remove test logs and fix variable assignment on call to BoolSta…

d0dc618

…bleConfig

enforce file size limits

d30acac

nits: Rename Fleet -> Managed

d8a47ad

Nits: clean up initialization of stableConfigSource types

862ca0c

Add tests for BoolStableConfig

b972cfc

Write tests for StringStableConfig

c0cfbf6

nits: remove testing log

9c39056

use origin != default to determine status for DD_APPSEC_SCA_ENABLED

511881f

change BoolStableConfig fn signature to return error instead of boole…

a65c4d2

…an configured

remove test files

dcb8bb3

Make DD_APPSEC_ENABLED use BoolStableConfig

394eddd

mtoffl01 commented Apr 28, 2025

View reviewed changes

mtoffl01 added 3 commits April 28, 2025 15:25

Improve godocs and add more test cases to stableconfigsource_test

f36ba46

gofmt

7a1f7a6

nits: cleanup

f980ad3

mtoffl01 force-pushed the mtoff/v2-stable_cfg_1 branch from 0435678 to d8fb6c4 Compare May 7, 2025 16:41

Include data_streams_monitoring in startup log

0b05149

mtoffl01 marked this pull request as ready for review May 7, 2025 20:28

mtoffl01 requested review from a team as code owners May 7, 2025 20:28

mtoffl01 requested a review from nikita-tkachenko-datadog May 7, 2025 20:28

nsrip-dd reviewed May 8, 2025

View reviewed changes

Comment thread profiler/options.go

optimize otelDDConfig for handsoff

1ecd8e5

nsrip-dd approved these changes May 13, 2025

View reviewed changes

Comment thread profiler/options.go

felixge mentioned this pull request May 13, 2025

chore(internal/stableconfig): remove config priority code duplication #3520

Merged

felixge reviewed May 13, 2025

View reviewed changes

Comment thread internal/stableconfig/api.go

chore(internal/stableconfig): remove config priority code duplication (…

d20bb4e

…#3520)

eliottness approved these changes May 20, 2025

View reviewed changes

mtoffl01 added 3 commits May 21, 2025 12:52

Annotate file paths linux support

eca5697

pull from remote

fd6c518

Join errors when multple in stableconfig.Bool

4c07803

mtoffl01 requested a review from eliottness May 21, 2025 11:12

eliottness approved these changes May 22, 2025

View reviewed changes

mtoffl01 added 4 commits May 27, 2025 12:19

Change log import from telemetry/log to correct internal/log package …

ad3b68d

…in stableconfigsource, and add test for windows

resolve merge conflicts with main in civisibility.go

0e84328

reintroduce deleted fmt import in appsec/config/config.go

c358a34

Fix TestParseFileNonLinux

a49337b

genesor approved these changes May 27, 2025

View reviewed changes

mtoffl01 force-pushed the mtoff/v2-stable_cfg_1 branch from 3076429 to a49337b Compare May 27, 2025 19:54

update error msg for TestSCAEnabled 'parsing error' case

631d655

mtoffl01 merged commit 0c7fa4b into main May 27, 2025

mtoffl01 deleted the mtoff/v2-stable_cfg_1 branch May 27, 2025 20:38

		localFilePath = "/etc/datadog-agent/application_monitoring.yaml"
		managedFilePath = "/etc/datadog-agent/managed/datadog-agent/stable/application_monitoring.yaml"

Uh oh!

Conversation

mtoffl01 commented Apr 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Notes

Testing

Motivation

Reviewer's Checklist

Uh oh!

datadog-datadog-prod-us1 Bot commented Apr 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Datadog Report

Uh oh!

pr-commenter Bot commented Apr 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks

Uh oh!

mtoffl01 Apr 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nsrip-dd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

eliottness left a comment

Choose a reason for hiding this comment

Uh oh!

eliottness May 20, 2025

Choose a reason for hiding this comment

Uh oh!

mtoffl01 May 21, 2025

Choose a reason for hiding this comment

Uh oh!

eliottness May 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

eliottness May 20, 2025

Choose a reason for hiding this comment

Uh oh!

eliottness May 20, 2025

Choose a reason for hiding this comment

Uh oh!

eliottness May 20, 2025

Choose a reason for hiding this comment

Uh oh!

mtoffl01 May 21, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mtoffl01 commented Apr 15, 2025 •

edited

Loading

datadog-datadog-prod-us1 Bot commented Apr 15, 2025 •

edited

Loading

pr-commenter Bot commented Apr 15, 2025 •

edited

Loading

mtoffl01 Apr 28, 2025 •

edited

Loading