Repository navigation
fix(id): reset DogStatsD buffers on identity refresh - #10339
litianningdatadog wants to merge 1 commit into
Conversation
Overall package sizeSelf size: 8.96 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #10339 +/- ##
=======================================
Coverage 98.71% 98.71%
=======================================
Files 1025 1025
Lines 160626 160695 +69
Branches 74 74
=======================================
+ Hits 158563 158632 +69
Misses 2063 2063
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:
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
🟡 Changes recommended
Active HTTP requests can still deliver stale data after refresh; abort handling and in-flight coverage are needed.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Resets DogStatsD state during MicroVM identity refreshes to prevent stale data from crossing identities.
Changes:
- Reset buffered, telemetry, and aggregation state on refresh.
- Add generation checks for detached deliveries.
- Add identity-refresh regression coverage.
File summaries
| File | Summary |
|---|---|
packages/dd-trace/test/dogstatsd.spec.js |
Tests reset and stale-send behavior. |
packages/dd-trace/src/dogstatsd.js |
Implements state resets and stale-delivery guards. |
Review details
Suppressed comments (1)
packages/dd-trace/src/dogstatsd.js:384
- The new generation guard is covered only on the UDP/DNS paths in this spec. The HTTP callback also suppresses stale completion and, importantly, prevents the error path from falling back to UDP, but no test holds the mocked HTTP request open across
updateTags; a regression here could resend the old queue or record its transport telemetry. Add an HTTP case that flushes, refreshes before invoking the callback with an error, and asserts no UDP fallback or stale telemetry.
if (!this.#isCurrentIdentity(identityRefreshGeneration)) {
done?.()
return
}
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const buffer = Buffer.concat(queue) | ||
| request(buffer, this.#httpOptions, (error, _result, _statusCode, _headers, dropped) => { | ||
| if (!this.#isCurrentIdentity(identityRefreshGeneration)) { |
This comment has been minimized.
This comment has been minimized.
![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: 24c0e95350
ℹ️ 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 buffer = Buffer.concat(queue) | ||
| request(buffer, this.#httpOptions, (error, _result, _statusCode, _headers, dropped) => { | ||
| if (!this.#isCurrentIdentity(identityRefreshGeneration)) { |
There was a problem hiding this comment.
When an identity refresh occurs after request() starts but before its DNS/connect/write completes, this generation check runs only in the response callback, after the old buffer may already have been transmitted. Consequently, each resumed MicroVM clone can still submit the pre-snapshot HTTP payload; the guard merely suppresses its telemetry and UDP fallback. Track and abort active HTTP requests during updateTags(), and cover this sibling of the UDP/DNS case.
AGENTS.md reference: AGENTS.md:L79-L79
Useful? React with 👍 / 👎.
BenchmarksBenchmark execution time: 2026-09-16 20:42:43 Comparing candidate commit b8e2848 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 2303 metrics, 10 unstable metrics.
|
24c0e95 to
b8e2848
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b8e2848166
ℹ️ 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".
| socket.send(buffer, 0, buffer.length, this.#port, address, (error) => { | ||
| if (error) { | ||
| if (!this.#isCurrentIdentity(identityRefreshGeneration)) { | ||
| complete() |
There was a problem hiding this comment.
When a MicroVM snapshot is taken after socket.send() is queued but before it completes, this generation check runs only from the completion callback, after libuv has already accepted the old buffer; each resumed clone can therefore still transmit the stale datagram. The new pending-callback test confirms only that stale completions are ignored, not that the packets are canceled. Track and invalidate the outstanding UDP transport at refresh, and cover the sibling case by asserting that a receiver gets no pre-refresh packet.
AGENTS.md reference: AGENTS.md:L78-L79
Useful? React with 👍 / 👎.
What does this PR do?
Prevent DogStatsD buffered data from crossing MicroVM identity boundaries.
Motivation
This is a reviewable slice from the breakdown of monolithic PR #9814, tracked by SVLS-9768. A MicroVM identity refresh must not carry buffered, baseline, or pending exporter state into the next identity.
Changes
Dependency tree
origin/master01-common-exporter-agent-spanstats-reset-and-cancel02-agentless-exporter-identity06-datastream-reset-and-cancel03-dogstatsd-buffer-reset← current PR04-otel-runtime-baseline05-otel-runtime-metric-resetsThis PR is intentionally draft and is part of the stacked review sequence above.