fix(netty-4.1): Treat Netty native client aborts as non-error responses - #12295
Conversation
|
🎯 Code Coverage (details) 🔗 Commit SHA: f51a702 | Docs | View more details | Give us feedback! |
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
…ror(-32): Broken pipe` resulting in an error span with 500 status
b798fda to
b7970a5
Compare
writevAddresses(..) failed with error(-32): Broken pipe resulting in an error span with 500 statusb7970a5 to
f3e5c74
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3e5c74504
ℹ️ 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".
f3e5c74 to
1a87253
Compare
1a87253 to
d8c6f8f
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Assert and drain expected traces in AppSec pipelining scenarios so spans are consumed by the tests that create them. The failure appeared now because this branch adds native Netty latest-dep dependencies/tests, changing the `latestDepTest` classpath and execution order enough for previously unasserted AppSec traces to be written after the next test’s writer reset. Add method-aware server trace matching for the HEAD response case.
from DDSpanTest.java
Replace brittle exact-message matching with a scoped matcher for Netty NativeIoException writev failures ending in known client-abort errno messages. This covers another observed Linux ECONNRESET format: writevAddresses(..) failed with error(-104): Connection reset by peer while still limiting the non-error treatment to native Netty write-side client aborts, such as Broken pipe and Connection reset by peer, instead of broadly suppressing unrelated native I/O failures.
There was a problem hiding this comment.
More details
The change limits special handling to Netty native write failures with known client-abort messages. Other write failures still mark the span as an error, and the application response status stays unchanged.
🤖 Datadog Autotest · Commit f51a702 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
|
/merge |
|
View all feedbacks in Devflow UI.
It will be processed automatically as soon as GitHub reports it as mergeable. View in MergeQueue UI.
The expected merge time in
The merge request has been interrupted because the build 6264861321344271299 took longer than expected. The current limit for the base branch 'master' is 120 minutes. |
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
042bde8
into
master
What Does This Do
Solves #12197, a regression introduced by #11941 and released in 1.65.0.
Updates Netty 4.1 broken-pipe handling so Netty native transport write failures such as:
writevAddresses(..) failed with error(-32): Broken pipewritevAddresses(..) failed: Connection reset by peersyscall:writev(..) failed: Broken pipeare treated as client-aborted responses instead of server errors.
This PR:
io.netty.channel.unix.Errors$NativeIoException.Broken pipeandConnection reset by peersuffixes, while preserving the existing shared exactBroken pipebehavior.500when the terminal write promise fails.Motivation
#11941 correctly moved Netty server span completion to terminal response write completion so asynchronous write failures can be observed.
However, cancelled client responses can surface through Netty native transport as
NativeIoExceptions whose messages include syscall context beforeBroken pipeorConnection reset by peer. The existing shared suppression only handled the exactBroken pipecase, so expected client disconnects from Netty native transport started appearing as errored server spans with HTTP500status in Datadog.Additional Notes
1.65.xonce merged.Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issueJira ticket: APMS-20399