Skip to content

Fix flaky DatadogMetricsTest (broken wait helper, origin detection, UDP races)#5384

Merged
adamw merged 1 commit into
masterfrom
fix/datadog-metrics-test-flakiness
Jul 7, 2026
Merged

Fix flaky DatadogMetricsTest (broken wait helper, origin detection, UDP races)#5384
adamw merged 1 commit into
masterfrom
fix/datadog-metrics-test-flakiness

Conversation

@adamw

@adamw adamw commented Jul 7, 2026

Copy link
Copy Markdown
Member

Problem

DatadogMetricsTest was flaky on CI (e.g. the failure on #5369, where the 4xx counter assertion ran before the datagram arrived) and failed 5/5 when run on a dev machine. Four root causes:

  1. The waitReceiveMessage helper was broken. Its m.startsWith(expected) check could never match — the default sentinel was "method" and others were "4xx"/"0.3", while every datagram starts with tapir. or datadog.. Its polarity was also inverted (it kept looping while the expected message was present, rather than until it appeared), and its first clause was unbounded (a potential infinite hang if no message ever arrived). The effective behaviour degraded to "return as soon as ANY message arrives", so the assertions that followed raced in-flight UDP datagrams. The "0.3" sentinel in the duration test was additionally wrong — the last histogram sent is 3.3.
  2. Origin detection breaks exact-match assertions on dev machines. java-dogstatsd-client enables origin detection by default; on machines where a container id resolves, it appends |c:<container-id> to every datagram, so all exact-string assertions fail (hence 5/5 local failures). Now disabled via .originDetectionEnabled(false) on every client.
  3. Client-side aggregation delays and reshapes counters. The client aggregates metrics client-side and flushes every ~2 seconds, so datagrams arrive late, and counter increments can be split or merged across flush windows.
  4. Unsynchronized cross-thread access. MockStatsDServer.receivedMessages was a plain var written by a daemon receiver thread and read by the test thread, with no @volatile.

Fix

  • Replaced all waitReceiveMessage calls with eventually-wrapped assertions, using the same idiom as ServerMetricsTest (class-level Eventually.PatienceConfig, 15s timeout / 150ms interval for slow CI); deleted the broken helper.
  • Added .originDetectionEnabled(false) to all five client builders.
  • Tagged "collect requests active" and "collect requests total" as Retryable: aggregation can in rare cases split or merge counter increments across 2-second flush windows, which no amount of waiting can heal — the suite's existing retry mechanism handles this.
  • Made receivedMessages @volatile.

Verification

On a machine where origin detection resolves a container id:

  • Baseline (before fix): datadogMetrics/testOnly ...DatadogMetricsTest0/5 passed (all 5 fail on the appended |c:<container-id>).
  • After fix: 3 consecutive runs — 5/5 passed, 3/3 green.
  • datadogMetrics2_12/Test/compile and datadogMetrics3/Test/compile — success.
  • datadogMetrics/Test/scalafmtCheck — success.

🤖 Generated with Claude Code

The suite was flaky on CI and failed 5/5 on dev machines, for four
reasons:

1. The waitReceiveMessage helper was broken: its startsWith(expected)
   check never matched (all datagrams start with "tapir." or
   "datadog."), its polarity was inverted (it looped WHILE the expected
   message was present), and its first clause was unbounded (a
   potential infinite hang). Its effective behaviour degraded to
   "return once ANY message arrives", so assertions raced in-flight UDP
   datagrams. Replaced by eventually-wrapped assertions (idiom from
   ServerMetricsTest, 15s timeout / 150ms interval).

2. java-dogstatsd-client enables origin detection by default; on
   machines where a container id resolves it appends |c:<container-id>
   to every datagram, breaking all exact-string assertions (the local
   5/5 failure). Disabled via originDetectionEnabled(false).

3. Client-side aggregation flushes every 2 seconds, so datagrams arrive
   late and counters can be split or merged across flush windows; the
   counter tests are now tagged Retryable, as no amount of waiting can
   heal a split/merged counter.

4. MockStatsDServer.receivedMessages was a plain var written by a
   daemon thread and read by the test thread; now @volatile.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@adamw adamw merged commit e983817 into master Jul 7, 2026
22 checks passed
@adamw adamw deleted the fix/datadog-metrics-test-flakiness branch July 7, 2026 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant