fix(test): wait for flush before concurrent shutdown in integration test

[bm1549]

What does this PR do?
Adds a try_wait_until call in the shutdown executes only once integration test to ensure the trace is flushed before spawning 10 concurrent shutdown! threads.

Motivation:
This test was flaky on master — failing with traces_flushed: 0 under CI load. The root cause: DEFAULT_FLUSH_INTERVAL is stubbed to 0, so the background worker almost always flushes the trace before any shutdown thread runs. But on the rare occasion the 1-second DEFAULT_SHUTDOWN_TIMEOUT expires before the HTTP round-trip completes, traces_flushed is still 0.

The flakiness was masked from March 10 to April 2 by PR #5426 (which forced final_status: pass on all JUnit test cases via XSLT). When that was reverted in PR #5550, the pre-existing flakiness became visible again.

This PR makes the pre-flush explicit and deterministic via try_wait_until, matching the pattern used throughout this file.

Change log entry
None.

Additional Notes:
The try_wait_until makes explicit what was already happening implicitly: with a 0-second flush interval, the worker drains the buffer before shutdown threads are scheduled. The test still validates that 10 concurrent shutdown! calls don't produce errors or unexpected stats.

How to test the change?
Integration test change validated by running:

bundle exec rspec spec/datadog/tracing/integration_spec.rb -e "executes only once"

(Requires DATADOG_INTEGRATION_TEST=true and a running agent.)

[datadog-prod-us1-6]

✅ Tests

🎉 All green!

❄️ No new flaky tests detected
🧪 All tests passed

🎯 Code Coverage (details)
• Patch Coverage: 100.00%
• Overall Coverage: 95.36% (-0.01%)

_{This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 81202db | Docs | Datadog PR Page | Was this helpful? React with 👍/👎 or give us feedback!}

[p-datadog]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bf62118192

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

[chatgpt-codex-connector]

Keep pending trace until concurrent shutdown starts

Waiting for traces_flushed >= 1 before starting the 10 shutdown! threads removes the condition this example is meant to test: concurrent shutdown while a flush is still pending. In this setup, the buffer is already drained when shutdown begins, so regressions in concurrent shutdown idempotency (e.g., multiple stop/flush paths running) can pass unnoticed because traces_flushed remains 1 either way. This weakens the executes only once coverage instead of validating it.

Useful? React with 👍 / 👎.

[p-datadog]

comment(non-blocking): Claude is... extremely blind to this finding. I have done 4 review passes, with ever stricter rules, and even the fourth pass still considers the test to be "fine and meaningful" after the change this PR proposes. I think the best explanation of this is the following:

❯ what exactly is that referring to that executes only once                                                          

● I don't know. The test description says "executes only once" but doesn't name the subject. It could mean:

  1. The shutdown side effects (flush, worker stop) execute only once — which is what the assertion traces_flushed: 1
   tests
  2. The shutdown! method's guarded body executes only once — the return unless @run path

  Both readings are plausible. The test description is ambiguous about which noun "executes" refers to.

❯ evaluate pr correctness with both interpretations                                                                  

● Interpretation 1: shutdown side effects (flush) execute only once

  The test asserts traces_flushed: 1. Before the PR, the single flush could come from either the background worker or
   the shutdown path. After the PR, the flush comes from the background worker (waited on by try_wait_until), and the
   shutdown path flushes an empty buffer. traces_flushed is still 1, so the assertion still proves that 10 concurrent
   shutdowns didn't produce additional flushes. Correct under this interpretation.

  Interpretation 2: shutdown's guarded body executes only once

  The test asserts traces_flushed: 1 as a proxy for "only one thread got past return unless @run." Before the PR, if
  the one thread that gets past the guard triggers a flush and it completes, traces_flushed: 1 confirms exactly one
  thread executed the body. After the PR, the flush happened before any shutdown call — the background worker
  incremented traces_flushed to 1, not shutdown. If shutdown's guarded body executes once and flushes an empty
  buffer, the count stays at 1. But if the guard fails and multiple threads enter the body, each would flush an empty
   buffer — and the count would still stay at 1, because flushing an empty buffer may not increment traces_flushed.

  Under this interpretation, the PR weakens the test. Pre-PR, the flush came from shutdown, so traces_flushed: 1 was
  direct evidence the guarded body ran once. Post-PR, the flush comes from the background worker, and the shutdown
  flushes are no-ops regardless of how many threads enter the body. The assertion no longer distinguishes "one thread
   entered the guard" from "multiple threads entered the guard but all flushed empty buffers."

❯ which of these in your estimation is the correct interpretation                                                    

● I don't know. The test name is ambiguous and I don't have access to the author's intent. Both readings fit the
  code.

  If you have a view on which interpretation is correct, that would resolve whether this is a clean approval or
  whether the weakened-assertion issue under interpretation 2 is worth flagging.

I think the test's description, namely lack of naming the subject (WHAT exactly is supposed to be executed once), leads claude to not be able to verify test correctness.

[bm1549]

There is an alternative fix and it's a bit stronger, but the piece I don't like about it is that it just makes the race condition harder to occur instead of eliminating it

This is okay in theory, but under heavy CI load I expect this will flake every once in a while even with a much larger timeout. Since I'm pushing to have zero flaky tests associated with my team (we want to be good neighbors!), I'd be more inclined to weaken the test ever so slightly rather than leave a test in for a race condition that may never occur

@p-datadog wdyt?

[p-datadog]

suggestion (non-blocking): Consider waiting after the shutdown threads instead of before

There's a third option that I think sidesteps the tradeoff entirely — move the try_wait_until to after threads.each(&:join):

threads.each(&:join)

# The worker thread may still be completing an HTTP call after
# shutdown!'s join(DEFAULT_SHUTDOWN_TIMEOUT) timed out. Wait for
# the flush to finish so the assertion sees the final stats.
try_wait_until { tracer.writer.stats[:traces_flushed] >= 1 }

We agree that with the current approach (waiting before), the buffer is already empty when the 10 shutdown threads start — every shutdown flushes nothing, so traces_flushed: 1 holds regardless of whether the guard works. Waiting after keeps the buffer populated when shutdowns begin, so the concurrent shutdowns still race with the in-flight flush.

The root cause is just that @worker.join(1) can time out before the HTTP call finishes. The worker thread is still alive and will eventually complete — we just need to give it a moment before asserting.

I have a PR up at #5585 with this approach. #5584 is a reproducer (CI failing as expected) and #5586 merges both to validate the fix neutralizes the forced failure. Happy to be wrong on this — curious what you think.