Repository navigation
fix(id): reset remaining exporter buffers on refresh - #10327
Draft
litianningdatadog wants to merge 1 commit into
Draft
litianningdatadog wants to merge 1 commit into
litianningdatadog wants to merge 1 commit into
Conversation
This was referenced Sep 14, 2026
Contributor
Overall package sizeSelf size: 8.97 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 |
|
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. |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Critical runtime and test defects remain, and an agent retry path is not reset-aware.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Resets pending exporter buffers and retry state during identity refresh to prevent stale payloads from being retried.
Changes:
- Adds per-writer reset controllers and retry-generation checks.
- Propagates reset controllers through common and agent exporters.
- Adds request, writer, and propagation tests.
File summaries
| File | Summary |
|---|---|
packages/dd-trace/test/exporters/common/writer.spec.js |
Adds writer reset behavior tests. |
packages/dd-trace/test/exporters/common/request.spec.js |
Adds retry reset tests; the log stub lacks NoTransmitError. |
packages/dd-trace/test/exporters/agent/writer.spec.js |
Verifies controller propagation. |
packages/dd-trace/src/exporters/common/writer.js |
Integrates reset behavior for encoder and request state. |
packages/dd-trace/src/exporters/common/request.js |
Adds retry reset control; contains a connectionOptions initialization error. |
packages/dd-trace/src/exporters/agent/writer.js |
Propagates reset state, but test-optimization retry paths remain outside the controller. |
Review details
Suppressed comments (1)
packages/dd-trace/src/exporters/agent/writer.js:155
- When
isTestOptimizationis true, this request isci-visibility/exporters/request, notcommonRequest. That implementation owns its ownretryTimerand backpressure retry queue and never registers them in this controller, so a refresh while one is pending still retries the captured pre-refresh payload. Integrate the controller into that retry lifecycle (including backpressure) or use a controller-aware transport.
resetController,
- Files reviewed: 6/6 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+140
to
142
| delete connectionOptions.resetController | ||
|
|
||
| const connectionOptions = { ...options, agent } |
| }, | ||
| http: httpStub, | ||
| './docker': docker, | ||
| '../../log': log, |
![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: c21a740364
ℹ️ 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".
| return | ||
| } | ||
| } | ||
| delete connectionOptions.resetController |
There was a problem hiding this comment.
Every normal Buffer/string/array request reaches this statement before the const connectionOptions declaration on line 142, so evaluating the binding in its temporal dead zone throws a synchronous ReferenceError. As a result, common exporter requests never reach client.request, callbacks are not invoked, and default agent trace delivery is broken; create connectionOptions first and then delete its internal resetController property.
AGENTS.md reference: AGENTS.md:L107-L109
Useful? React with 👍 / 👎.
litianningdatadog
marked this pull request as draft
September 14, 2026 20:25
litianningdatadog
marked this pull request as ready for review
September 14, 2026 20:29
litianningdatadog
marked this pull request as draft
September 14, 2026 21:41
This branch has not been deployed
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.
Stacked PRs
tianning.li/1-dogstatsd-exporter-refresh-resetstianning.li/2-otel-runtime-refresh-resetstianning.li/3-exporter-otel-identity-gapstianning.li/4-runtime-datastream-identity-gapstianning.li/5-exporter-buffer-state— current PRtianning.li/6-otel-datastream-counter-statetianning.li/7-spanstats-otel-retry-statetianning.li/8-agent-otel-counter-baselinestianning.li/9-datastream-dogstatsd-exporter-requestsOrigin
This change set was split from the monolithic PR #9814 to keep the identity-refresh fixes smaller and easier to review.
What does this PR do?
Reset remaining pending exporter buffers on identity refresh.
Motivation
Pending encoded batches and request-buffer state may contain pre-snapshot payloads. Discarding those buffers prevents stale data from crossing the identity boundary.
Logical change
This is the 5-exporter-buffer-state layer of the stacked MicroVM identity-refresh fix. It is based on
tianning.li/4-runtime-datastream-identity-gapsand keeps the subsystem-specific reset behavior isolated for review.Jira
SVLS-9768
Tests
Additional Notes
This PR is intentionally opened as a draft and is part of a linear stack.