Repository navigation
fix(id): drop stale data on MicroVM identity refresh - #9814
litianningdatadog wants to merge 8 commits into
Conversation
Overall package sizeSelf size: 8.98 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.5.1 | 127.66 kB | 513.61 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | dc-polyfill | 0.1.11 | 25.74 kB | 25.74 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
There was a problem hiding this comment.
Pull request overview
This PR follows up on MicroVM clone-resume identity refresh behavior in dd-trace-js, ensuring pre-snapshot buffered/aggregated telemetry is dropped (instead of being flushed under each clone’s refreshed identity) and fixing a remote-config client-id tag regression after identity refresh.
Changes:
- Reset/discard pending state on
datadog:identity:refreshacross span-stats buckets, agentless trace batches, OTel metrics measurement queues/cumulative baselines, and OTel logs queued records. - Rebase runtime-metrics sampler baselines (CPU/ELU/event-loop delay) on identity refresh to avoid deltas spanning the snapshot pause.
- Ensure
_dd.rc.client_idis written back intoconfig.tagswhenever an RC client exists (even if tags were rebuilt by an RC lib-config update).
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 |
|---|---|
| packages/dd-trace/test/span_stats.spec.js | Adds coverage for clearing pending span-stats buckets on identity refresh. |
| packages/dd-trace/test/runtime_metrics.spec.js | Adds coverage for rebasing event-loop-delay baseline on identity refresh (runtime + OTLP variants). |
| packages/dd-trace/test/remote_config/index.spec.js | Ensures RC client-id tag is restored on identity refresh when an RC client exists. |
| packages/dd-trace/test/opentelemetry/metrics.spec.js | Adds coverage for dropping pre-refresh sync Counter measurements on identity refresh. |
| packages/dd-trace/test/opentelemetry/logs.spec.js | Adds coverage for dropping queued pre-refresh log records on identity refresh. |
| packages/dd-trace/test/exporters/common/writer.spec.js | Adds unit test for discarding a pending encoded batch via resetPendingBatch(). |
| packages/dd-trace/test/exporters/agentless/exporter.spec.js | Adds coverage that agentless exporter drops pending trace batch on identity refresh. |
| packages/dd-trace/test/dogstatsd.spec.js | Adds coverage for dropping pending aggregated metrics when identity refresh changes tags, and preserving when unchanged. |
| packages/dd-trace/src/span_stats.js | Subscribes to identity refresh to drop pre-snapshot span-stats buckets. |
| packages/dd-trace/src/runtime_metrics/runtime_metrics.js | Routes identity refresh through subscribeToIdentityRefresh(..., resetSamplerBaselines) and implements baseline rebase. |
| packages/dd-trace/src/runtime_metrics/otlp_runtime_metrics.js | Rebases OTLP runtime-metrics baselines (ELU + event-loop histogram) on identity refresh. |
| packages/dd-trace/src/runtime_metrics/client.js | Extends subscribeToIdentityRefresh to accept an optional post-refresh callback. |
| packages/dd-trace/src/remote_config/index.js | Writes _dd.rc.client_id back to config.tags whenever an RC client exists. |
| packages/dd-trace/src/opentelemetry/metrics/periodic_metric_reader.js | Adds resetPendingState() to discard queued measurements and sync cumulative state. |
| packages/dd-trace/src/opentelemetry/metrics/index.js | Subscribes to identity refresh to drop pending OTel metrics state (with restart-safe unsubscribe handling). |
| packages/dd-trace/src/opentelemetry/logs/index.js | Subscribes to identity refresh to drop pending OTel logs state (with restart-safe unsubscribe handling). |
| packages/dd-trace/src/opentelemetry/logs/batch_log_processor.js | Adds resetPendingState() to discard queued log records and clear the timer. |
| packages/dd-trace/src/exporters/common/writer.js | Adds resetPendingBatch() to drop pending encoded trace batches. |
| packages/dd-trace/src/exporters/agentless/index.js | Subscribes to identity refresh to drop pending agentless trace batches. |
| packages/dd-trace/src/dogstatsd.js | Makes tag updates report whether tags changed; resets aggregation only when the underlying tag prefix changed. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
BenchmarksBenchmark execution time: 2026-09-14 19:33:11 Comparing candidate commit 3075325 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 2306 metrics, 7 unstable metrics.
|
|
✅ All CI checks and tests passed. 🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 3075325 | Docs | View more details | Give us feedback! |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #9814 +/- ##
========================================
Coverage 98.71% 98.71%
========================================
Files 1025 1025
Lines 160626 161126 +500
Branches 74 74
========================================
+ Hits 158563 159060 +497
- Misses 2063 2066 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
4f85c4a to
016848d
Compare
litianningdatadog
changed the title
016848d to
579c97e
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.
Suppressed comments (3)
packages/dd-trace/src/dogstatsd.js:255
- The aggregation maps also need to be discarded on every identity refresh, not only when the rendered tag prefix changes. Otherwise configurations that omit runtime-id and RC tags preserve identical pre-snapshot counters/gauges/histograms in every clone; the refresh event itself is the signal to drop them.
if (this._client.updateTags(tags)) {
this.reset()
packages/dd-trace/src/dogstatsd.js:71
- An identity refresh still represents a clone resume when the generated DogStatsD tags are unchanged—for example, with runtime-id tagging disabled and Remote Config disabled. Returning here keeps the pre-snapshot encoded buffer, so every clone can flush the same metrics. Clear the buffer on every refresh while retaining the boolean only as an indication that the prefix changed.
This issue also appears on line 254 of the same file.
if (tagsPrefix === this.#tagsPrefix) return false
packages/dd-trace/src/runtime_metrics/runtime_metrics.js:186
- On supported Node versions before the new per-iteration sampler, the default path uses
@datadog/native-metrics; itsstats()call owns and drains the CPU, event-loop, and GC accumulators. This reset only rebases JavaScript state, so the first post-resume capture still exports the native pre-snapshot accumulations from every clone. DrainnativeMetrics.stats()during refresh as well.
function resetSamplerBaselines () {
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 579c97eb74
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (this._client.updateTags(tags)) { | ||
| this.reset() | ||
| } |
There was a problem hiding this comment.
On a MicroVM resume, the generated DogStatsD tags commonly remain unchanged: DogStatsDClient.generateClientConfig() excludes runtime-id unless runtimeMetricsRuntimeId is enabled, which defaults to false, and serverless configuration disables Remote Config so there may be no changing RC client-id tag either. In that default case updateTags() returns false and this branch retains counters, gauges, and histograms accumulated in the snapshot, causing every clone to flush duplicate pre-snapshot values; the identity-refresh subscriber should reset aggregation regardless of whether the serialized tag prefix changes. The added tests only exercise the non-default runtime-id-enabled case and explicitly preserve the faulty sibling case.
AGENTS.md reference: AGENTS.md:L127-L129
Useful? React with 👍 / 👎.
| function resetSamplerBaselines () { | ||
| lastTime = performance.now() | ||
| lastElu = performance.eventLoopUtilization() | ||
|
|
||
| if (lastCpuUsage !== null) { | ||
| lastCpuUsage = process.cpuUsage() |
There was a problem hiding this comment.
When the @datadog/native-metrics branch is active, this rebases lastTime but does not consume or reset the addon's accumulated CPU, event-loop, and GC statistics. The next captureNativeMetrics() therefore reads pre-snapshot values from nativeMetrics.stats() while dividing CPU usage by only the post-refresh elapsed time, producing an inflated first sample and exporting the stale event-loop/GC data the change intends to discard. Resetting this branch needs to drain the native statistics as well; the new observer-reset test explicitly skips this supported sibling path.
AGENTS.md reference: AGENTS.md:L127-L129
Useful? React with 👍 / 👎.
| for (const key of this.#cumulativeState.keys()) { | ||
| this.#lastExportedState.delete(key) | ||
| } | ||
| this.#cumulativeState.clear() |
There was a problem hiding this comment.
Clearing #cumulativeState starts a new value series for synchronous cumulative counters, histograms, and up/down counters, but MetricAggregator.#startTime remains the timestamp captured when the snapshot image initialized. After a clone resumes—potentially days later—the first cumulative point therefore contains only post-resume measurements while claiming an interval beginning before the snapshot, which yields incorrect rates and temporal metadata under the clone's new resource identity. The reset must also advance the aggregator start time; the added test covers only the default delta-counter sibling.
AGENTS.md reference: AGENTS.md:L127-L129
Useful? React with 👍 / 👎.
![datadog-prod-us1-4[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
Native runtime metrics still retain pre-snapshot CPU/event-loop/GC accumulators, while span stats can retain the old tags object after an RC tag update. Both paths can make clones emit telemetry associated with snapshot-era state or identity.
🤖 Datadog Autotest · Commit 579c97e · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
| lastTime = performance.now() | ||
| lastElu = performance.eventLoopUtilization() |
There was a problem hiding this comment.
Every clone using native runtime metrics can report the same pre-snapshot event-loop/GC activity and inflated CPU usage under its fresh identity.
Assertion details
- Input: A MicroVM snapshot taken after @datadog/native-metrics accumulates activity but before its periodic stats() collection, then resumed into one or more clones.
- Expected:
Identity refresh should drain the native addon's CPU, event-loop, and GC state before establishing post-resume baselines. - Actual:
The new refresh callback only rebases JavaScript timestamps. On the native path, the addon's CPU baseline and event-loop/GC histograms remain populated until nativeMetrics.stats() is called, so the next periodic collection includes snapshot-era activity.
| lastTime = performance.now() | |
| lastElu = performance.eventLoopUtilization() | |
| nativeMetrics?.stats() | |
| lastTime = performance.now() | |
| lastElu = performance.eventLoopUtilization() |
Was this helpful? React 👍 or 👎
🤖 Datadog Autotest · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
| const onIdentityRefresh = () => { | ||
| this.buckets = new TimeBuckets() |
There was a problem hiding this comment.
Agent span stats from multiple clones can share the snapshot runtime ID, causing cross-clone identity collisions and incorrect aggregation.
Assertion details
- Input: Remote Config applies tracing_tags after SpanStatsProcessor construction, replacing config.tags, followed by a MicroVM identity refresh and agent-format span-stats export.
- Expected:
The span-stats processor should adopt the current config.tags object when identity refresh fires, before exporting new buckets. - Actual:
The callback clears buckets but leaves this.tags pointing to the object captured during construction. An RC tracing_tags update replaces config.tags, so identity refresh mutates a different object and subsequent v0.6 stats retain the snapshot runtime ID.
| const onIdentityRefresh = () => { | |
| this.buckets = new TimeBuckets() | |
| const onIdentityRefresh = (config) => { | |
| this.tags = config?.tags ?? this.tags | |
| this.buckets = new TimeBuckets() |
Was this helpful? React 👍 or 👎
🤖 Datadog Autotest · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
6cac3ff to
31556cd
Compare
aa18e4e to
dcf016d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e427594c00
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| // A clone resume shouldn't flush spans buffered before the snapshot under its own identity. | ||
| unsubscribeBatchReset?.() | ||
| const onIdentityRefresh = () => this._writer.resetPendingBatch() |
There was a problem hiding this comment.
When an agentless batch has already reached AgentlessWriter._sendPayload() before the MicroVM snapshot, this reset only clears the encoder; the active @datadog/libdatadog exporter and its sendV04() operation remain live, so every clone can complete the same pre-snapshot trace delivery. Fresh evidence in this revision is that the agentless transport never receives the new common-request reset controller, despite caching an exporter that can be closed; the refresh path needs to invalidate the active pipeline export as well as the queued encoder batch.
AGENTS.md reference: AGENTS.md:L79-L79
Useful? React with 👍 / 👎.
| const onIdentityRefresh = () => processor.resetPendingState() | ||
| identityRefreshChannel.subscribe(onIdentityRefresh) |
There was a problem hiding this comment.
When an OTLP log or metric batch has already been transformed and passed to OtlpHttpExporterBase.sendPayload() before the snapshot, resetting only the processor queue or metric reader cannot reach it: the base exporter retains the HTTP request and serialized payload and exposes no reset/cancellation hook. Each resumed clone can therefore finish the same pre-snapshot OTLP export, so both newly added reset subscriptions need to invalidate active exporter requests as well as stored measurements or log records.
AGENTS.md reference: AGENTS.md:L79-L79
Useful? React with 👍 / 👎.
| this.tags = config?.tags ?? this.tags | ||
| this.buckets = new TimeBuckets() | ||
| this.writer.resetPendingRequests() |
There was a problem hiding this comment.
When Avro or Protobuf schema sampling has accumulated calls in DataStreamsProcessor._schemaSamplers before the snapshot, this handler clears only buckets, leaving every clone with the same SchemaSampler.weight and lastSampleMs. The next eligible post-resume schema span in each clone therefore reports a schema.weight that includes the identical pre-snapshot message count (and may retain the old sampling window); reset the schema samplers along with the other inherited Data Streams aggregation state.
AGENTS.md reference: AGENTS.md:L79-L79
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7af4bdaa12
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (const retry of controller.pendingRetryTimers) { | ||
| retry.cancel() | ||
| } |
There was a problem hiding this comment.
When an agent, span-stats, or Data Streams request is still in an asynchronous DNS lookup or socket connection at clone resume, reset() only cancels entries in pendingRetryTimers; the ClientRequest already created by attempt() remains live and can transmit its pre-snapshot buffer from every restored clone. Track and abort active requests as well as retry timers, settling their callbacks with the refresh cancellation error.
AGENTS.md reference: AGENTS.md:L73-L79
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40b95a8f35
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const { type } = measurement | ||
| if (type !== METRIC_TYPES.OBSERVABLECOUNTER || this.#getTemporality(type) !== TEMPORALITY.DELTA) { | ||
| continue |
There was a problem hiding this comment.
When an application uses an ObservableUpDownCounter, its callback can report a process-lifetime total, but these lines only rebase delta ObservableCounter values. Because observable up/down counters are always cumulative, the first export after a MicroVM resume therefore reports the entire pre-snapshot value under the clone's refreshed runtime ID while using the newly reset start time. Preserve and subtract a refresh offset for this sibling instrument type as well.
AGENTS.md reference: AGENTS.md:L79-L79
Useful? React with 👍 / 👎.
BridgeAR
left a comment
BridgeAR
left a comment
There was a problem hiding this comment.
Could we split this into smaller chunks? Now that the main functionality exists, we could change smaller pieces at a time
Copilot/Codex review of #9814 found several reset paths that still leaked pre-snapshot state across MicroVM clones. DogStatsD only reset buffered lines/telemetry counters when the serialized tag prefix changed, which a clone resume with runtime-id tagging disabled skips entirely. Runtime metrics didn't drain the native-metrics addon or queued GC observer entries. Span stats dropped the OTLP trace-root flag on reset. The OTel periodic metric reader didn't rebase its cumulative start time. The default (non-agentless) agent exporter never reset its pending batch at all. Fixes each with regression tests. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Closes the last three gaps from PR review: Data Streams span-stats buckets, in-flight trace-transport retries, and OTel metrics' dropped- measurement count and ObservableCounter delta baselines all survived a MicroVM identity refresh and could be re-flushed under a clone's new identity. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
40b95a8 to
3075325
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3075325e07
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| controller.pendingRetryTimers.clear() | ||
|
|
||
| const activeRequests = [...controller.activeRequests] |
There was a problem hiding this comment.
When a MicroVM refresh cancels a scheduled retry, retry.cancel() synchronously invokes the original flush callback before activeRequests is snapshotted. If that callback records post-refresh data and starts another flush, its new request uses the current generation but is added to activeRequests in time to be included here and is then aborted as though it were stale. Capture the active-request set before invoking any cancellation callbacks so reentrant post-refresh exports are preserved.
Useful? React with 👍 / 👎.
https://datadoghq.atlassian.net/browse/SVLS-9768
What does this PR do?
Follow-up to #9709. When a MicroVM clone resumes, telemetry buffered or aggregated before the snapshot can be flushed again under the clone's refreshed identity. This resets or rebases that state on the identity-refresh path instead of only retagging it:
@datadog/libdatadogexports remain out of scope.ObservableCounterdelta baselines.@datadog/native-metrics's own accumulators and any GC entries already queued but not yet delivered.Motivation
Without these resets, clones can duplicate pre-snapshot telemetry, traces, and metrics under fresh MicroVM identities. They can also send a pre-refresh request after the reset, so the same activity may be reported more than once.
Tests
Additional Notes
profiler.js, not the per-processor subscription pattern used here.telemetry.js'sretryDataandsentIntegrations) can duplicate or drop pre-snapshot telemetry across clones on resume. This needs separate follow-up work.@datadog/libdatadogexports and OTLP HTTP requests are not cancelled on identity refresh. Their transport-specific cancellation paths need separate changes.Regression tests cover each reset above, including detached sends, pre-refresh compression jobs, active common HTTP request cancellation, and callback-once behavior. Verified locally with the full spec for each affected module (DogStatsD, writer, common request, agent/agentless exporters, OTel logs/metrics, runtime metrics, span stats, Data Streams), plus
node --expose-gcagainstruntime_metrics.spec.js.