Skip to content

fix(ddtrace/tracer): classify ENOTCONN as transient and raise stale-conn retry budget#4908

Merged
gh-worker-dd-mergequeue-cf854d[bot] merged 2 commits into
mainfrom
dario.castane/thirsty-bartik-a94f7f
Jun 18, 2026
Merged

fix(ddtrace/tracer): classify ENOTCONN as transient and raise stale-conn retry budget#4908
gh-worker-dd-mergequeue-cf854d[bot] merged 2 commits into
mainfrom
dario.castane/thirsty-bartik-a94f7f

Conversation

@darccio

@darccio darccio commented Jun 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

What does this PR do

  • Add syscall.ENOTCONN to isTransientConnError (alongside EPIPE/ECONNRESET/ECONNABORTED/net.ErrClosed), and add a "socket is not connected" string fallback for Windows WSAENOTCONN parity.
  • Raise staleConnRetryAttempts from 3 → 6 (7 total attempts) to absorb the Linux net.ErrClosed burst-exhaustion path.

Motivation

TestUDSTransportRecoversFromStaleIdleConn was flaking at ~1.25%/run on macOS CI (#4809 introduced the test and the retry fix). The flake had two distinct failure modes:

macOS (dominant, reproduced): On Darwin, the same stale-idle UDS write race that produces EPIPE on Linux surfaces instead as ENOTCONN ("socket is not connected"). isTransientConnError did not recognise ENOTCONN, so doWithStaleConnRetry returned immediately without retrying and the request was lost.

Linux (rarer, the originally-reported CI error): net.ErrClosed is correctly classified and retried, but under the 400-request synthetic burst the idle pool can keep handing out stale conns, occasionally exhausting all 4 original attempts before landing on a fresh connection.

Verification

Reproduced baseline flake with a compiled test binary: 10/80 batches failed (count=10 each), 100% ENOTCONN. After the fix: 0/80 batches failed.

# baseline (main): 10/80 batches failed
# after fix:        0/80 batches failed

@darccio
fix(ddtrace/tracer): classify ENOTCONN as transient and raise retry b…
06573ca 
…udget

On Darwin, the stale-idle UDS write race surfaces as syscall.ENOTCONN
("socket is not connected") rather than the Linux-typical EPIPE or
ECONNRESET.  isTransientConnError did not recognise ENOTCONN, so
doWithStaleConnRetry returned the error immediately without retrying,
causing TestUDSTransportRecoversFromStaleIdleConn to flake at ~1.25%/run
on macOS CI.

Two changes:
- Add errors.Is(err, syscall.ENOTCONN) to isTransientConnError alongside
  EPIPE/ECONNRESET/ECONNABORTED, and add the "socket is not connected"
  string fallback for Windows WSAENOTCONN parity.
- Raise staleConnRetryAttempts from 3 to 6 (7 total attempts) to absorb
  the Linux net.ErrClosed burst-exhaustion that was the originally-
  reported CI failure mode.

Verified: 0/80 batches failed (count=10 each) after the fix vs 10/80
before; race detector clean; full transport regression set green.
@darccio darccio added team:apm-go AI Generated Largely based on code generated by an AI or LLM. This label is the same across all dd-trace-* repos labels Jun 18, 2026
@darccio
darccio marked this pull request as ready for review June 18, 2026 14:44
@darccio
darccio requested a review from a team as a code owner June 18, 2026 14:44
@datadog-prod-us1-6

datadog-prod-us1-6 Bot commented Jun 18, 2026 •
edited by datadog-datadog-prod-us1-2 Bot
Loading

Copy link
Copy Markdown

Tests

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

🎯 Code Coverage (details)
• Patch Coverage: 100.00%
• Overall Coverage: 62.68% (+4.27%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 4442e9b | Docs | Datadog PR Page | Give us feedback!

@pr-commenter

pr-commenter Bot commented Jun 18, 2026 •
edited
Loading

Copy link
Copy Markdown

Benchmarks

Benchmark execution time: 2026-06-18 15:43:57

Comparing candidate commit 4442e9b in PR branch dario.castane/thirsty-bartik-a94f7f with baseline commit cc90400 in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 297 metrics, 2 unstable metrics, 1 flaky benchmarks without significant changes.

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

  • 🟩 = significantly better candidate vs. baseline
  • 🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

Known flaky benchmarks

These benchmarks are marked as flaky and will not trigger a failure. Modify FLAKY_BENCHMARKS_REGEX to control which benchmarks are marked as flaky.

Known flaky benchmarks without significant changes:

  • scenario:BenchmarkOTLPTraceWriterFlush

@darccio

darccio commented Jun 18, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Jun 18, 2026

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Can't wait for the next one!

Reviewed commit: 4442e9b0f8

ℹ️ 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".

@gh-worker-dd-mergequeue-cf854d
gh-worker-dd-mergequeue-cf854d Bot merged commit 6043488 into main Jun 18, 2026
206 checks passed
@gh-worker-dd-mergequeue-cf854d
gh-worker-dd-mergequeue-cf854d Bot deleted the dario.castane/thirsty-bartik-a94f7f branch June 18, 2026 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kakkoyun kakkoyun kakkoyun approved these changes

Assignees

No one assigned

Labels

AI Generated Largely based on code generated by an AI or LLM. This label is the same across all dd-trace-* repos mergequeue-status: done team:apm-go

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@darccio @kakkoyun