refactor(config): peer service revamp by funyjane · Pull Request #4653 · DataDog/dd-trace-go

funyjane · 2026-04-10T14:18:10Z

What does this PR do? Coninues the following PR: #4483

PR was created due to permission issues, I was unable to merge the fork branch initially.

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 make lint locally.
New code doesn't break existing tests. You can check this by running make test locally.
Add an appropriate team label so this PR gets put in the right place for the release notes.
All generated files are up to date. You can check this by running make generate locally.
Non-trivial go.mod changes, e.g. adding new modules, are reviewed by @DataDog/dd-trace-go-guild. Make sure all nested modules are up to date by running make fix-modules locally.

Unsure? Have a question? Request a review!

Move peerServiceDefaultsEnabled and peerServiceMappings from the tracer's local config struct to internal/config.Config. These fields already existed in the global config singleton but lacked getter/setter methods. The tracer now reads/writes peer service configuration through the global config accessors instead of maintaining local copies.

…e defaults - Fix gofmt: remove extra blank line left from field removal - Keep schema-dependent peerServiceDefaultsEnabled logic in the tracer since spanAttributeSchemaVersion uses namingschema (understands "v1") while loadConfig uses getInt (can't parse "v1"). Add TODO to move once spanAttributeSchemaVersion is fully migrated.

SetPeerServiceMappings and SetPeerServiceMapping have non-standard signatures (map param / three value params) that the reflective test helper cannot auto-generate values for. Register them as special cases.

Address PR review comments: move the peer service defaults enabled logic (schema-dependent calculation) from tracer's newConfig() to internal config's loadConfig(). Add parseSpanAttributeSchema parser that handles "v0"/"v1" string values (matching namingschema behavior), replacing the previous getInt call that couldn't parse these formats.

…okup - Add getIntWithParser to configprovider instead of calling get() directly - Keep all provider.getX calls together in loadConfig; move the schema-dependent peer service defaults override to the derived evaluations section below - Replace PeerServiceMappings map in TracerConf with a PeerServiceMapping func for single-key lookups, avoiding lock contention and per-call map copies on the hot path (setPeerService is called for every span) - Update setPeerService to use the function-based lookup with nil guard - Fix CiVisibility TracerConf test to compare fields individually since functions cannot be compared with reflect.DeepEqual

…al mutation Made-with: Cursor

Follow the existing configprovider pattern: use getString to read the raw DD_TRACE_SPAN_ATTRIBUTE_SCHEMA value and parse it in the derived evaluations section with parseSpanAttributeSchema, matching how featureFlags and logToStdout are handled. Remove the getIntWithParser method that was just a passthrough to get().

Resolve merge conflict in internal/config/config.go: adopt main's refactored provider API (p.GetX exported methods) while preserving our peer service and span attribute schema migrations. Update reportTelemetry calls to configtelemetry.Report to match main's rename. Co-Authored-By: Claude Opus 4.6 <[email protected]>

…rvice # Conflicts: # internal/config/config_helpers.go

…rvice # Conflicts: # ddtrace/tracer/api.txt

…race-go into config-revamp/peer-service

Co-Authored-By: Claude Opus 4.6 <[email protected]>

…race-go into config-revamp/peer-service

…rvice

… map Avoid a breaking API change in this PR. The TracerConf cleanup (removing PeerServiceMappings entirely) will be handled in a follow-up PR. Co-Authored-By: Claude Opus 4.6 <[email protected]>

Co-Authored-By: Claude Opus 4.6 <[email protected]>

…rvice

…race-go into config-revamp/peer-service

datadog-official · 2026-04-10T14:21:39Z

✅ Tests

🎉 All green!

❄️ No new flaky tests detected
🧪 All tests passed

🎯 Code Coverage (details)
• Patch Coverage: 50.79%
• Overall Coverage: 60.08% (-0.03%)

_{This comment will be updated automatically if new data arrives.

🔗 Commit SHA: e62b453 | Docs | Datadog PR Page | Was this helpful? React with 👍/👎 or give us feedback!}

pr-commenter · 2026-04-10T14:46:18Z

Benchmarks

Benchmark execution time: 2026-04-10 14:46:13

Comparing candidate commit e62b453 in PR branch config-revamp/peer-service with baseline commit 747b2e5 in branch main.

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

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

🟩 = significantly better candidate vs. baseline
🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

scenario:BenchmarkPayloadVersions/simple_1000spans/v0_4-25

🟥 execution_time [+16.949µs; +17.461µs] or [+7.871%; +8.109%]

scenario:BenchmarkPayloadVersions/simple_100spans/v0_4-25

🟥 execution_time [+1.625µs; +1.727µs] or [+6.603%; +7.016%]

scenario:BenchmarkPayloadVersions/simple_10spans/v0_4-25

🟥 execution_time [+115.357ns; +125.043ns] or [+3.736%; +4.050%]

hannahkm

🚀

## Summary - Adds a public `Config.SpanAttributeSchemaVersion()` getter on `internal/config.Config`. The field itself was already declared and initialised by `loadConfig`; this just exposes a public accessor. - Removes the duplicate `spanAttributeSchemaVersion` field from the legacy tracer `config` struct and the `c.spanAttributeSchemaVersion = int(namingschema.GetVersion())` assignment in `newConfig`. - Rewires the two readers (`telemetry.go` startup config and `tracer.go` `StartSpan` hot path) to read through the new getter. - Getter avoids `defer` because it is read once per local-root span — matches the hot-path pattern from the peer-service migration (#4653) and the README guidance. - `namingschema.LoadFromEnv()` is preserved: contrib packages still consume the namingschema global via `GetVersion()`. Jira: [APMAPI-1887](https://datadoghq.atlassian.net/browse/APMAPI-1887) EPIC: [APMAPI-1881](https://datadoghq.atlassian.net/browse/APMAPI-1881) — Go Config Revamp: Migrate tracer ## Test plan - [x] `go build ./...` - [x] `go vet ./ddtrace/tracer/... ./internal/config/...` - [x] `go test ./internal/config/...` - [x] `go test ./ddtrace/tracer/...` (full suite) - [x] Existing `attribute_schema_is_set_v0/v1/wrong_value` (tracer_test.go) and `defaults-with-schema-v1` (option_test.go) pass without modification. [APMAPI-1887]: https://datadoghq.atlassian.net/browse/APMAPI-1887?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [APMAPI-1881]: https://datadoghq.atlassian.net/browse/APMAPI-1881?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ Co-authored-by: daniil.stasiuk <[email protected]>

### What does this PR do? - Adds a public `Config.MaxTagsHeaderLen()` getter on `internal/config.Config`. - Initializes the field in `loadConfig` from `DD_TRACE_X_DATADOG_TAGS_MAX_LENGTH`, preserving the existing clamping semantics (negative → 0; > `MaxPropagatedTagsLength` → clamped down to that value). - Adds `DefaultMaxTagsHeaderLen` and `MaxPropagatedTagsLength` exported constants in `internal/config`. - Removes the duplicate env resolution from `tracer/option.go` and the `defaultMaxTagsHeaderLen` / `maxPropagatedTagsLength` constants there. - The propagator is now constructed using `c.internalConfig.MaxTagsHeaderLen()` instead of locally re-reading the env var. ### Motivation Continued config-migration work: move env resolution of `DD_TRACE_X_DATADOG_TAGS_MAX_LENGTH` into `internal/config` to match the pattern established by recent migrations (e.g. #4711, #4653). --------- Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>

funyjane and others added 26 commits February 27, 2026 10:39

Merge branch 'main' into config-revamp/peer-service

f11ae64

fix: add peer service map setters to specialCaseSetters in test

4712800

SetPeerServiceMappings and SetPeerServiceMapping have non-standard signatures (map param / three value params) that the reflective test helper cannot auto-generate values for. Register them as special cases.

fix: defensively copy map in SetPeerServiceMappings to prevent extern…

49922cf

…al mutation Made-with: Cursor

fix: regenerate api.txt with apiextractor tool

55bf4cf

Merge remote-tracking branch 'origin/main' into config-revamp/peer-se…

0011c24

…rvice # Conflicts: # internal/config/config_helpers.go

Merge branch 'main' into config-revamp/peer-service

dff4ff0

Merge branch 'main' into config-revamp/peer-service

17669a3

Merge remote-tracking branch 'origin/main' into config-revamp/peer-se…

3a2868a

…rvice # Conflicts: # ddtrace/tracer/api.txt

Merge branch 'config-revamp/peer-service' of github.com:funyjane/dd-t…

fdd1104

…race-go into config-revamp/peer-service

Merge branch 'main' into config-revamp/peer-service

8515ed1

Remove noisy inline comments in loadConfig

876ceb1

Co-Authored-By: Claude Opus 4.6 <[email protected]>

Merge branch 'config-revamp/peer-service' of github.com:funyjane/dd-t…

0d76d3e

…race-go into config-revamp/peer-service

Merge remote-tracking branch 'origin/main' into config-revamp/peer-se…

554e735

…rvice

Revert TracerConf.PeerServiceMapping func back to PeerServiceMappings…

e8ed300

… map Avoid a breaking API change in this PR. The TracerConf cleanup (removing PeerServiceMappings entirely) will be handled in a follow-up PR. Co-Authored-By: Claude Opus 4.6 <[email protected]>

Fix gofmt alignment in TracerConf struct

bf588da

Co-Authored-By: Claude Opus 4.6 <[email protected]>

Merge remote-tracking branch 'origin/main' into config-revamp/peer-se…

f39930c

…rvice

Merge branch 'main' into config-revamp/peer-service

610583c

Merge branch 'main' into config-revamp/peer-service

b5626d3

Merge remote-tracking branch 'origin/main' into config-revamp/peer-se…

05c4fcd

…rvice

Merge branch 'config-revamp/peer-service' of github.com:funyjane/dd-t…

e62b453

…race-go into config-revamp/peer-service

funyjane requested review from a team as code owners April 10, 2026 14:18

funyjane changed the title ~~Config revamp/peer service~~ feat(config): peer service revamp Apr 10, 2026

funyjane changed the title ~~feat(config): peer service revamp~~ refactor(config): peer service revamp Apr 10, 2026

genesor approved these changes Apr 10, 2026

View reviewed changes

hannahkm approved these changes Apr 10, 2026

View reviewed changes

funyjane merged commit 0029919 into main Apr 10, 2026
298 of 306 checks passed

funyjane deleted the config-revamp/peer-service branch April 10, 2026 16:09

darccio mentioned this pull request Apr 20, 2026

feat(ddtrace/tracer): add span lifecycle benchmarks #4682

Merged

7 tasks

funyjane mentioned this pull request May 1, 2026

refactor(config): migrate spanAttributeSchemaVersion #4711

Merged

5 tasks

mtoffl01 mentioned this pull request May 5, 2026

refactor(config): migrate maxTagsHeaderLen #4730

Merged

funyjane mentioned this pull request May 8, 2026

refactor(config): migrate dogstatsdAddr #4741

Closed

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor(config): peer service revamp#4653

refactor(config): peer service revamp#4653
funyjane merged 26 commits into
mainfrom
config-revamp/peer-service

funyjane commented Apr 10, 2026 •

edited

Loading

Uh oh!

datadog-official Bot commented Apr 10, 2026 •

edited by datadog-prod-us1-6 Bot

Loading

Uh oh!

pr-commenter Bot commented Apr 10, 2026

Explanation

More details about the CI and significant changes

Uh oh!

hannahkm left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

funyjane commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do? Coninues the following PR: #4483

Motivation

Reviewer's Checklist

Uh oh!

datadog-official Bot commented Apr 10, 2026 • edited by datadog-prod-us1-6 Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pr-commenter Bot commented Apr 10, 2026

Benchmarks

Explanation

More details about the CI and significant changes

scenario:BenchmarkPayloadVersions/simple_1000spans/v0_4-25

scenario:BenchmarkPayloadVersions/simple_100spans/v0_4-25

scenario:BenchmarkPayloadVersions/simple_10spans/v0_4-25

Uh oh!

hannahkm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

funyjane commented Apr 10, 2026 •

edited

Loading

datadog-official Bot commented Apr 10, 2026 •

edited by datadog-prod-us1-6 Bot

Loading