Improve explicit histogram contention performance - #8717
Conversation
Pull request dashboard statusWaiting on reviewers · refreshed 2026-08-10 21:20 UTC Review the latest changes. Status above doesn't look right?
|
| @ParameterizedTest | ||
| @MethodSource("stressTestArgs") | ||
| @Timeout(value = 10, unit = TimeUnit.SECONDS, threadMode = Timeout.ThreadMode.SEPARATE_THREAD) | ||
| void partialWriteStressTest( |
There was a problem hiding this comment.
This new test checks that the intermediate state of every collect is correct and doesn't include partial writes. Asserting this was particularly important with the complex lock free implementations I experimented with. The current test asserts that the aggregate state after a bunch of records and collects is correct, which ensures no lost writes or double writes.
But we had no tests asserting no partial writes, and with a lock based implementation it was easier to ignore this lack of coverage.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8717 +/- ##
============================================
+ Coverage 91.62% 91.65% +0.02%
- Complexity 10327 10353 +26
============================================
Files 1003 1003
Lines 27138 27267 +129
Branches 3187 3220 +33
============================================
+ Hits 24866 24991 +125
- Misses 1566 1567 +1
- Partials 706 709 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
My goal is to have OpenTelemetry metrics equal or exceed performance of other metrics systems, in an apples-to-apples comparison.
In this PR, I address the poor performance of explicit histograms under contention, as embodied by this prometheus/client_java benchmark.
I have issues with this benchmark. The real performance characteristic is more nuanced than can be captured by recording to a single bound series. To get the complete picture, I extended MetricRecordBenchmark to add a
prometheus=false|trueparameter, allowing comparison against prometheus alongside the parameters we've deemed valuable.But there's no denying it: otel java's current simple lock based approach (predictably) gets steamrolled by a lock free design under contention.
The problem with fixing this is that lock based approach performs maximally well without contention. I've been thinking / iterating on designs on and off for months and there is no change I've found or am aware of that improves contended performance without regressing uncontended performance. This has been a sticking point in the past. So the approach I've taken with this PR:
prometheus/client_java, with modifications to avoid the complexity of having a buffer and to also optionally record min and max.Context.current()when exemplars are disabled is unnecessary and represents a significant chunk of work (at least on a relative basis).So how do we compare to prometheus/client_java? There are a lot of params that make it a bit overwhelming to interpret the data. Many of the param combinations tell variations of the same overall story. I've compressed the story into one graph which I think is fair and understandable. Notes:
Below is a before and after of the unmodified
MetricRecordBenchmark. This shows clearly shows the contended gains and uncontended regressions.Details
Takeaways from a performance standpoint: