fix(ddtrace/tracer): instrument payload queue-full trace drops - #5136
Merged
Conversation
Drops in pushChunk when t.out is full were only visible via a 1s-throttled log.Error, so operators had no metric to detect or size around the loss. Add datadog.tracer.traces_dropped and datadog.tracer.spans_dropped counters tagged reason:queue_full, matching the reason: tag convention already used in writer.go, plus a datadog.tracer.queue.length gauge sampled on each scheduled flush tick. Part of #5135
Contributor
Config Audit |
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 25db7b9 | Docs | Datadog PR Page | Give us feedback! |
![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: 5a03f9a6f7
ℹ️ 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".
BenchmarksBenchmark execution time: 2026-08-06 15:26:21 Comparing candidate commit 25db7b9 in PR branch Found 0 performance improvements and 7 performance regressions! Performance is the same for 319 metrics, 0 unstable metrics, 1 flaky benchmarks without significant changes.
|
5 tasks
kakkoyun
approved these changes
Aug 6, 2026
kakkoyun
left a comment
kakkoyun
left a comment
Member
There was a problem hiding this comment.
LGTM.
Accept the codex comment.
darccio
added
the
AI Generated
… drops submitChunk already records reason:trace_filter for filter-rejected chunks before unconditionally calling pushChunk, so a filter-rejected chunk that also overflows the payload queue was being counted twice under two different reasons. Gate the queue_full counters on !filterRejected. Addresses Codex review feedback on #5136.
Member
Author
|
/code blockers |
|
View all feedbacks in Devflow UI.
Checking merge blockers for #5136...
Detected 5 merge blocker(s) to address:
|
…merge blockingTransport's endpoint() lost sync with the ddTransport interface change in #5147 (endpoint now takes the trace protocol), which only surfaced once GitHub's PR merge-ref checks combined this branch with current main.
gh-worker-dd-devflow-36fce6
Bot
added
mergequeue-status: removed
and removed
mergequeue-status: waiting
labels
9 tasks
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
2 participants
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.
Summary
datadog.tracer.traces_dropped/datadog.tracer.spans_droppedcounters taggedreason:queue_fullinpushChunk, so drops caused by the internal payload queue (t.out) filling up are finally visible in metrics instead of only via a 1s-throttledlog.Error.datadog.tracer.queue.lengthgauge (len(t.out)) sampled on every scheduled flush tick, giving visibility into queue backlog before it overflows.reason:tag convention already used inwriter.go(reason:send_failed,reason:+dropReason).Part of #5135 (P1 of that issue's proposed remediation) — a pure bug fix finishing the metric that #533 asked for back in 2019 and that never got wired up for this specific drop path. The remaining items in #5135 (configurable queue size, non-blocking flush, dynamic protocol re-evaluation, widening the drain) change runtime behavior and are left for discussion on that issue before any PRs.
Test plan
go build ./...go vet ./...go test .(fullddtrace/tracerpackage suite)TestSubmitChunkQueueFullto assert both new counters fire withreason:queue_fullTestQueueLengthMetricconfirming the gauge fires on tickgofmt -lclean on all touched files