Skip to content

Send APM_TRACING configs as sdk_config when the library advertises SDK_CONFIGURATION - #7628

Draft
rachelyangdog wants to merge 2 commits into
mainfrom
rachel.yang/rc-sdk-configuration-payload
Draft

Send APM_TRACING configs as sdk_config when the library advertises SDK_CONFIGURATION#7628
rachelyangdog wants to merge 2 commits into
mainfrom
rachel.yang/rc-sdk-configuration-payload

Conversation

@rachelyangdog

Copy link
Copy Markdown
Contributor

Motivation

dd-trace-js#9392 is the first library to move to the unified Config at Runtime contract, and it fails 7 system-tests jobs:

  • DEBUGGER_INPRODUCT_ENABLEMENT × 5 weblogs — Test_Debugger_InProduct_Enablement_Dynamic_Instrumentation::test_inproduct_enablement_di
  • PARAMETRIC × 2 — 10 behaviour tests + 7 capability tests in test_dynamic_configuration.py

That PR swaps the whole APM_TRACING contract:

before after
capabilities APM_TRACING_SAMPLE_RATE, _LOGS_INJECTION, _HTTP_HEADER_TAGS, _CUSTOM_TAGS, _ENABLED, _SAMPLE_RULES, _ENABLE_DYNAMIC_INSTRUMENTATION, _ENABLE_CODE_ORIGIN, _ENABLE_LIVE_DEBUGGING SDK_CONFIGURATION (bit 49)
payload lib_config: {dynamic_instrumentation_enabled: true} sdk_config: {service_name, env, config: [{key: "DD_DYNAMIC_INSTRUMENTATION_ENABLED", value: "true"}]}

The lib_config branch is removed, not kept as a fallback — RCClientManager.addConfig only reads conf.sdk_config?.config. system-tests only ever emits lib_config, so every APM_TRACING config becomes a no-op: DI is never enabled, probes never reach EMITTING, and the test fails.

What does this PR do?

The RC helpers now read the capability bit the library advertises on /v0.7/config and, when SDK_CONFIGURATION is set, send the same settings in the new shape. service_target is untouched — it still drives matching and priority.

  • utils/_remote_config.pyto_sdk_config_payload() rewrites an APM_TRACING config from lib_config to sdk_config: each setting keyed by its canonical DD_* name, serialized to its environment variable form (booleans, sample rate, tracing_header_tags"h:tag,…", tracing_tags"k:v,…", tracing_service_mapping"a:b,…", tracing_sampling_rules → JSON with the [{key, value_glob}] tag clauses folded into a map). library_supports_sdk_configuration() reads the bit.
  • The two APM_TRACING builders take use_sdk_config; the send_* wrappers detect it. lib_config stays the internal representation and prev_payloads keeps it, so the "empty config keeps the previous value" inheritance the debugger helpers rely on is unchanged.
  • tests/parametric/test_dynamic_configuration.py — translation happens in _set_rc, the single choke point that owns test_agent. assert_rc_capability() accepts the legacy per-setting bit or SDK_CONFIGURATION, since one bit now covers them all.
  • tests/parametric/capabilities.yml — nodejs's per-setting bits scoped to <7.0.0-0.
  • tests/test_library_conf.pyTest_HeaderTags_DynamicConfig routed through the same translation (missing_feature for nodejs today, but it builds its own APM_TRACING payload).
  • tests/test_the_test/test_remote_config.py — 7 unit tests covering the translation, the value serializers, and that the mapping stays exhaustive over what the builders can emit.

Nothing changes for any library that does not advertise the bit — today that is every library, including nodejs on main.

Notes for reviewers

  • capabilities.yml: SDK_CONFIGURATION is deliberately not listed as expected yet. dd-trace-js main is 7.0.0-pre and does not advertise it until #9392 lands; a pre-release is allowed to report capabilities beyond the expected set, so the <7.0.0-0 bound alone covers both sides of the transition without a chicken-and-egg break. A follow-up should add '>=7.0.0-0': [SDK_CONFIGURATION] once #9392 merges. The bound assumes the removal ships in 7.0.0; if a 7.0.0 is cut before #9392 lands, the bound needs moving.
  • dynamic_sampling_enabled and live_debugging_enabled are remote-config-only settings with no environment variable, so they cannot be expressed as sdk_config and are dropped for libraries on the new contract. Both are already ignored by dd-trace-js, so nothing regresses; the mapping records them explicitly as None rather than guessing a name.
  • provenance on sampling rules is carried through in the DD_TRACE_SAMPLING_RULES JSON. dd-trace-js's SamplingRule reads it, so _dd.p.dm -11/-12 should still be produced through the env-var path — worth confirming on the #9392 run, as it is a library-side behaviour, not a payload one.

Testing

  • ./run.sh TEST_THE_TEST — 445 passed
  • ./format.sh — clean
  • Replayed the 4-step RC sequence of test_inproduct_enablement_di through the js reader's allowlist/merge logic: unset → {}, enable → DD_DYNAMIC_INSTRUMENTATION_ENABLED=true, empty → inherits true, disable → false.

🤖 Generated with Claude Code

Libraries advertising the SDK_CONFIGURATION remote config capability no
longer read the lib_config object of an APM_TRACING config. They read a
sdk_config field carrying the same settings keyed by their canonical
environment variable name, with values in environment variable form.

system-tests only ever emitted lib_config, so every APM_TRACING config
was silently ignored by such a library.

The RC helpers now read the capability bit off the /v0.7/config requests
and translate the payload when it is advertised. lib_config stays the
internal representation, so the "empty config keeps the previous value"
semantics of the debugger helpers are untouched, and libraries that do
not advertise the bit keep receiving exactly what they receive today.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

CODEOWNERS have been resolved as:

tests/parametric/capabilities.yml                                       @DataDog/system-tests-core @DataDog/apm-sdk-capabilities
tests/parametric/test_dynamic_configuration.py                          @DataDog/system-tests-core @DataDog/apm-sdk-capabilities
tests/test_library_conf.py                                              @DataDog/system-tests-core
tests/test_the_test/test_remote_config.py                               @DataDog/system-tests-core
utils/_remote_config.py                                                 @DataDog/system-tests-core

@datadog-prod-us1-4

datadog-prod-us1-4 Bot commented Sep 1, 2026

Copy link
Copy Markdown

Pipelines  Tests

⚠️ Warnings

Your PR has failed checks. Please review the issues below and take necessary action before merging.

🚦 2 Pipeline jobs failed

Testing the test | System Tests (ruby, prod) / End-to-end #1 / uds-sinatra 1 — ❌ 10 tests failed · 🔧 Needs a code fix, caused by this PR

View more details · View in GitHub Actions

10 failed tests due to assertion errors when expected spans were not created for API Gateway requests.

❌ tests.appsec.test_inferred_spans.Test_Proxy_Inferred_Span_Tags.test_proxy_inferred_span[uds-sinatra] from system_tests_suite
AssertionError: Expected non empty appsec data on the weblog entry span
assert None

self = &lt;tests.appsec.test_inferred_spans.Test_Proxy_Inferred_Span_Tags object at 0x7fd3e98202f0&gt;

    def test_proxy_inferred_span(self) -&gt; None:
        service_entry_span_appsec_data = None
        for _, _, span, appsec_data in interfaces.library.get_appsec_events(self.r, full_trace=True):
            if span.get(&#34;service&#34;) == &#34;weblog&#34;:
                service_entry_span_appsec_data = appsec_data
...
❌ tests.integrations.test_inferred_proxy.Test_AWS_API_Gateway_Inferred_Span_Creation.test_api_gateway_inferred_span_creation[uds-sinatra] from system_tests_suite
AssertionError: API Gateway inferred span should have been created but was not found!
assert None is not None

self = &lt;tests.integrations.test_inferred_proxy.Test_AWS_API_Gateway_Inferred_Span_Creation object at 0x7fd3e8f15cd0&gt;

    def test_api_gateway_inferred_span_creation(self):
        assert self.r.text == &#34;ok&#34;
    
        span = get_span(interfaces.library, &#34;GET /api/data&#34;)
    
...
❌ tests.integrations.test_inferred_proxy.Test_AWS_API_Gateway_Inferred_Span_Creation_With_Distributed_Context.test_api_gateway_inferred_span_creation_with_dist... from system_tests_suite
AssertionError: API Gateway inferred span should have been created but was not found!
assert None is not None

self = &lt;tests.integrations.test_inferred_proxy.Test_AWS_API_Gateway_Inferred_Span_Creation_With_Distributed_Context object at 0x7fd3e8f15e50&gt;

    def test_api_gateway_inferred_span_creation_with_distributed_context(self):
        assert self.r.text == &#34;ok&#34;
    
        span = get_span(interfaces.library, &#34;GET /api/data/distributed&#34;)
    
...
↳ and 7 more — View all
Testing the test | all-jobs-are-green

View more details · View in GitHub Actions

Job did not run due to failed check: System Tests (ruby, prod) / End-to-end #1 / uds-sinatra.

📋 Copy fix prompt
CI on my pull request is failing. Help me find and fix the root cause of each failing job below — they were flagged as caused by changes in this PR, so focus on the diff. For each job, explain the failure and propose a fix.

Before you start, set up the Datadog software-delivery tooling so you can
query the CI data yourself:

1. Check whether you already have the Datadog software-delivery MCP tools
   (e.g. a `search_datadog_ci_pipeline_events` tool) and the `unblock-pr` skill.
2. If either is missing, STOP and ask me for permission before installing
   anything. Do not install or run anything until I have said yes.
3. Only with my explicit approval, set up the Datadog software-delivery MCP
   server and skills by following:
     https://docs.datadoghq.com/getting_started/software_delivery_mcp_tools/
   then restart so the skill is picked up.
4. If I decline, skip all of the above and work from the context below alone.

Then run /unblock-pr — it will pull the CI data itself. The job context below is what we already know.

If /unblock-pr is not available — because I declined the setup above, or it did not install — work from the context below instead.

Datadog has already classified this failure as caused by changes in this PR.
Take that as given and work the fix:

1. Locate the change. Diff this branch against its base and find the change
   that produces this error. Explain the mechanism, don't just name a file:
     git fetch origin && git diff $(git merge-base origin/main HEAD)...HEAD
2. Reproduce it locally. Run the failing job's command or test before
   proposing anything.
3. Propose the smallest fix that addresses the root cause — not a workaround,
   not a broadened assertion, not a disabled or skipped test.
4. Re-run the same command to confirm, and say exactly what you ran.
5. If the failure turns out to be intermittent rather than deterministic, say
   so plainly instead of "fixing" it — that is a flaky test, and patching it
   hides the problem.

If the right move is to re-run the job rather than change code, use the job
link in the context below. For GitHub Actions: `gh run rerun <run-id> --failed`,
where the run ID is the number after `/runs/` in that URL (not the trailing
number, which is the job ID).

Branch: rachel.yang/rc-sdk-configuration-payload

Testing the test | System Tests (ruby, prod) / End-to-end #1 / uds-sinatra 1
Commit: c45e00e14c3da50e16e6fc271134d8fe585c4c32
Error (code / test):
10 failed tests due to assertion errors when expected spans were not created for API Gateway requests.
CI job: https://github.com/DataDog/system-tests/actions/runs/33566239975/job/100051591361

ℹ️ Info

No other issues found (see more)

❄️ No new flaky tests detected

Useful? React with 👍 / 👎

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 79eb84b | Docs | View more details | Give us feedback!

Bit 49 is SDK_CONFIGURATION in the remote config source of truth
(dd-source remote-config/shared/libs/rc/capabilities.go), but libdatadog
gives the same bit to ASM_RAW_RESPONSE_BODY. dd-trace-php therefore
advertises it while still reading lib_config, and got sent a payload it
does not understand: DEBUGGER_INPRODUCT_ENABLEMENT failed on every php
weblog.

Require the per-setting APM_TRACING capabilities to be absent as well.
Dropping them is the point of the unified bit, and a library still
advertising them still understands lib_config, so the two are told apart
without relying on a bit whose meaning is currently ambiguous.

Also resolve the payload shape before set_and_wait_rc clears the
recorded requests. Reading the capabilities waits for the next remote
config request, and one recorded between the clear and the update is
exactly the stale acknowledgement the clear exists to discard, which
wait_for_rc_apply_state then took for the acknowledgement of the update.
That returned before the tracer had applied it and failed
test_trace_sampling_rate_override_env on every language.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@rachelyangdog

Copy link
Copy Markdown
Contributor Author

Update: two bugs found in the first CI run, both fixed in 79eb84b

1. Capability bit 49 is ambiguous today (this is the interesting one)

SDK_CONFIGURATION is bit 49 per the remote config source of truth — dd-source remote-config/shared/libs/rc/capabilities.go:

// CapabilitySDKConfiguration is the capability for remotely configuring any in-scope
// SDK setting at runtime via a single, env-var-keyed capability bit
CapabilitySDKConfiguration = Capability{
	Name:     "SDK_CONFIGURATION",
	bitIndex: 49,
}

But libdatadog gives the same bit to a different capability:

DDOG_REMOTE_CONFIG_CAPABILITIES_ASM_RAW_RESPONSE_BODY = 49,

dd-trace-php ships that enum, so it advertises bit 49 today while still reading lib_config. Captured from the failing job's artifact (php 1.24.2, apache-mod-7.4-zts), the client capabilities [2, 44, 15, 160, 235, 247, 252] decode to bits …, 42, 43, 45, 49. The first revision read that as "php supports SDK_CONFIGURATION", sent it sdk_config, and DI never enabled — test_inproduct_enablement_di failed on all 40 php weblog jobs. Java (highest bit 45) was unaffected, which is why only php broke.

Fix: the switch now also requires the per-setting APM_TRACING capabilities to be gone. Dropping APM_TRACING_SAMPLE_RATE/_LOGS_INJECTION/_HTTP_HEADER_TAGS/_CUSTOM_TAGS/_ENABLED/_SAMPLE_RULES is the entire point of the unified bit, and a library still advertising them still understands lib_config — so the two are told apart without depending on a bit whose meaning is currently contested. Verified against the real bitmasks captured from this run:

library bit 49 resolved shape
php 1.24.2 (real) set lib_config
java prod (real) unset lib_config
nodejs main (real) unset lib_config
nodejs + dd-trace-js#9392 set sdk_config

assert_rc_capability uses the same rule, so bit 49 alone can no longer let a libdatadog-based tracer skip a per-setting capability assertion.

Important

The bit collision itself still needs an owner. libdatadog and dd-source disagree about bit 49, and dd-trace-js#9392 is about to start using it for real. One of the two has to move before SDK_CONFIGURATION can be trusted on its own. The guard here keeps system-tests correct meanwhile, but it is a workaround, not the fix.

2. Stale-ACK race in the parametric helper

set_and_wait_rc calls test_agent.clear() before reusing a config_id, to discard the ACKs of the previous config at that path. The capability read sat after that clear and blocks until the next RC request is recorded — and that request still carries the tracer's ACK of the old config. wait_for_rc_apply_state matched it and returned before the update had been applied, so the test read a trace still sampled at the old rate:

E       assert 0.5 == 0.6 ± 6.0e-07

That failed TestDynamicConfigV1::test_trace_sampling_rate_override_env on rust, golang, nodejs, ruby, java and cpp — language-independent, because it is purely a helper-ordering bug.

Fix: resolve the payload shape at the top of set_and_wait_rc, before the clear, and memoize it once the capabilities are conclusive so _set_rc never polls inside the clear/update window.

Not caused by this PR

test_config_consistency.py and test_128_bit_traceids.py on cpp, and the golang net-http-orchestrion build job — unrelated to remote config, present independently of this change.

Testing

./run.sh TEST_THE_TEST — 446 passed. New unit test pins the resolution against the real php / java / nodejs bitmasks captured above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant