fix(runtime): refresh identity-bound consumers - #19780
Closed
litianningdatadog wants to merge 1 commit into
Closed
Conversation
Codeowners resolved asResolved from the full PR diff against |
Dependency direction analysis📈 Existing violations got worse10 pre-existing violation(s) increased in severity (e.g. their target became more depended-on, or got pulled into an import cycle), though the edge itself isn't new: Show violations that got worse (showing 5 of 10 highest severity)
|
|
Circular import analysis
|
| if asm_config._rc_client_id is not None: | ||
| entry_span.set_tag(APPSEC.RC_CLIENT_ID, asm_config._rc_client_id) | ||
| if asm_config._rc_client_id_enabled: | ||
| rc_client_id = remoteconfig_poller._client.id |
Contributor
Author
There was a problem hiding this comment.
We use remoteconfig_poller as client id's SOT instead of asm_config as we refresh remoteconfig_poller
BenchmarksBenchmark execution time: 2026-08-23 21:52:57 Comparing candidate commit 230002b in PR branch Found 0 performance improvements and 10 performance regressions! Performance is the same for 577 metrics, 1 unstable metrics.
|
| _rc_client_id: Optional[str] = None | ||
| # Set by enable_appsec_rc()/disable_appsec_rc(); gates _dd.rc.client_id span tagging so it's | ||
| # only emitted while AppSec RC is actually enabled, not just whenever a live RC client exists. | ||
| _rc_client_id_enabled: bool = False |
Contributor
Author
There was a problem hiding this comment.
use _rc_client_id_enabled to replace the existence check of _rc_client_id to accurately identify whether to access client id
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends runtime.refresh_identity() to actively refresh/rebuild long-lived components that cache runtime-identity at construction time (e.g., Remote Config native client, telemetry worker, trace exporter, runtime-metrics tags, Symbol DB context, crashtracking metadata, and AppSec RC tagging), ensuring identity changes propagate without fork-style buffer loss.
Changes:
- Introduces a
on_runtime_id_change()subscription mechanism and wires multiple consumers to rebuild/refresh identity-bound state onrefresh_identity(). - Adds a native crashtracker
reconfigurebinding and uses it to refresh crashtracking metadata on identity refresh. - Adjusts AppSec RC client-id span tagging to read the client id live (and gates tagging to only when AppSec RC is enabled), plus adds focused test coverage for the new refresh behavior.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/tracer/test_writer.py | Adds coverage that identity refresh rebuilds the trace exporter without recreating the writer/buffer. |
| tests/telemetry/test_writer.py | Adds coverage for telemetry worker teardown/rebuild behavior on identity refresh (including live-worker stop). |
| tests/runtime/test_runtime_metrics_api.py | Adds coverage that runtime-id metric tags refresh after refresh_identity(). |
| tests/internal/symbol_db/test_symbols.py | Adds coverage that Symbol DB context runtimeId/uploadId refresh on identity refresh. |
| tests/internal/remoteconfig/test_remoteconfig_native.py | Adds coverage that RC client-id is renewed and native client dropped/rebuilt on identity refresh. |
| tests/crashtracker/test_crashtracker.py | Adds coverage that crashtracking reconfigures metadata on identity refresh (Linux-only). |
| tests/appsec/appsec/test_remoteconfiguration.py | Adds coverage that AppSec RC client-id tagging is live and correctly gated when RC is disabled. |
| src/native/lib.rs | Exposes crashtracker_reconfigure to the Python native module. |
| src/native/crashtracker.rs | Implements the crashtracker_reconfigure PyO3 binding calling libdd’s reconfigure. |
| ddtrace/internal/writer/writer.py | Subscribes the trace writer to runtime-id changes and rebuilds the exporter on identity refresh. |
| ddtrace/internal/telemetry/writer.py | Subscribes telemetry writer to runtime-id changes; stops/drops worker for lazy rebuild on identity refresh. |
| ddtrace/internal/symbol_db/symbols.py | Subscribes Symbol DB uploader context to runtime-id changes and refreshes cached event identity fields. |
| ddtrace/internal/settings/asm.py | Replaces cached RC client-id storage with an enablement gate flag for tagging. |
| ddtrace/internal/runtime/runtime_metrics.py | Adds runtime-id change subscription to refresh platform tags when runtime-id tagging is enabled. |
| ddtrace/internal/remoteconfig/client.py | Subscribes RC client to runtime-id changes to renew client id and drop native client for rebuild. |
| ddtrace/internal/native/_native.pyi | Updates typing stubs to include crashtracker_reconfigure. |
| ddtrace/internal/core/crashtracking.py | Subscribes crashtracking to runtime-id changes and reconfigures native crashtracking metadata on identity refresh. |
| ddtrace/debugging/_probe/status.py | Switches debugging probe status payload to import runtime-id helpers directly (import hygiene). |
| ddtrace/appsec/_remoteconfiguration.py | Sets/clears the AppSec RC client-id tagging enablement flag on enable/disable. |
| ddtrace/appsec/_asm_request_context.py | Emits _dd.rc.client_id from the live RC client id when AppSec RC is enabled (instead of a cached value). |
Suppressed comments (1)
ddtrace/appsec/_asm_request_context.py:407
- This code path only needs Remote Config state when AppSec RC is enabled. If you switch to a lazy import (see earlier comment), add the import inside this branch so RemoteConfigPoller is only instantiated when the tag is actually being emitted.
if asm_config._rc_client_id_enabled:
rc_client_id = remoteconfig_poller._client.id
if rc_client_id is not None:
entry_span.set_tag(APPSEC.RC_CLIENT_ID, rc_client_id)
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| from ddtrace.internal.constants import Constant_Class | ||
| from ddtrace.internal.core.events import Event | ||
| import ddtrace.internal.logger as ddlogger | ||
| from ddtrace.internal.remoteconfig.worker import remoteconfig_poller |
litianningdatadog
force-pushed
the
tianning.li/2-web-request-starting-event
branch
2 times, most recently
from
August 20, 2026 10:10
2724ad8 to
2df658b
Compare
litianningdatadog
force-pushed
the
tianning.li/3-runtime-id-refresh-consumers
branch
2 times, most recently
from
August 20, 2026 11:22
c992c42 to
d424d55
Compare
litianningdatadog
force-pushed
the
tianning.li/2-web-request-starting-event
branch
5 times, most recently
from
August 20, 2026 19:02
4c72681 to
4e70948
Compare
litianningdatadog
force-pushed
the
tianning.li/2-web-request-starting-event
branch
2 times, most recently
from
August 23, 2026 20:47
55acf22 to
d59e112
Compare
litianningdatadog
force-pushed
the
tianning.li/3-runtime-id-refresh-consumers
branch
from
August 23, 2026 21:07
d424d55 to
230002b
Compare
litianningdatadog
added
the
changelog/no-changelog
Rotating the runtime id is not enough by itself. Several long-lived components bake runtime or Remote Config client identity into native clients, workers, upload metadata, or tag caches. Have those components subscribe to explicit identity refreshes and rebuild only the state that captures those ids. Fork-specific cleanup remains on the existing fork hooks so this path does not drop buffers or invent fork lineage.
litianningdatadog
force-pushed
the
tianning.li/3-runtime-id-refresh-consumers
branch
from
August 23, 2026 21:26
230002b to
8faa264
Compare
This was referenced Aug 23, 2026
This was referenced Aug 23, 2026
Contributor
Author
|
Closed as it has been splitted into smaller independent PRs |
Contributor
Author
|
Close it as it has been replaced by a collection of smaller independent PRs |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Refreshing the runtime id alone leaves stale identity in components that cache it at construction time. Remote Config, telemetry, trace writer, runtime metrics, Symbol DB, crashtracking, and AppSec RC tagging each have some runtime or client identity baked into long-lived state.
This wires those components to explicit identity refreshes and rebuilds only the state that captures those ids. It keeps fork cleanup on the existing fork hooks so this path does not drop buffers or report fake fork lineage.
Testing
Added direct
refresh_identity()coverage for the affected consumers, including RC client renewal, telemetry worker rebuild, trace exporter rebuild, runtime metric tag refresh, Symbol DB metadata refresh, crashtracking reconfiguration, and live AppSec RC client-id tagging.Risks
Medium. This touches several long-lived product components, but the behavior is only reached through explicit identity refresh and is not activated from request traffic until the next PR.
Additional Notes
Stacked on #19816.