Skip to content

fix: bound observation buffering during collection - #2336

Open
zeitlinger wants to merge 7 commits into
mainfrom
agent/bound-observation-buffer
Open

fix: bound observation buffering during collection#2336
zeitlinger wants to merge 7 commits into
mainfrom
agent/bound-observation-buffer

Conversation

@zeitlinger

@zeitlinger zeitlinger commented Jul 23, 2026

Copy link
Copy Markdown
Member

Summary

  • bound the collection spin wait and observation buffer
  • coordinate collection through explicit buffer generations and phase transitions
  • replay buffered observations before surfacing a collection timeout
  • skip timed-out native-histogram maintenance rather than throwing from observe()
  • document the generation handoff and striped-count protocol

This is stacked on the focused #2282 fix so the two Buffer concerns can be reviewed separately. It replaces the #2287 portion of #2297.

Fixes #2287

Ongoing discussion

  • Should the five-second collection deadline remain an internal default, or become publicly configurable? See the earlier discussion.
  • The bounded buffer currently applies backpressure rather than dropping observations. The earlier review suggested drop-on-overflow; this version avoids silent data loss but can block an observer while the generation remains active. See the earlier discussion.
  • The generation/phase handoff is a substantial concurrency change and should be reviewed independently of the simple stripe-index fix.

The earlier concerns about an unbounded second wait and exceptions escaping from native-histogram maintenance are addressed by this implementation.

Validation

  • mise run lint:fix
  • mise run build
  • ./mvnw test -pl prometheus-metrics-core -Dcoverage.skip=true -Dcheckstyle.skip=true
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ API changes detected — maintainer review required

This PR modifies the published API diff for the following module(s):

  • prometheus-metrics-core

Please review the changes in docs/apidiffs/current_vs_latest/ carefully before approving.

Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
Base automatically changed from agent/fix-buffer-stripe-index to main July 31, 2026 12:47
@zeitlinger
zeitlinger marked this pull request as ready for review August 18, 2026 08:38

@jaydeluca jaydeluca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two things I think block merge:

  1. The ==>= change is hiding a real accounting bug. The phase
    gate in append() created a class of observation that increments its stripe after the
    collector's getAndAdd — so it is excluded from expectedCount — yet still increments count.
    main could not produce that, which is why == was exact there and hangs here. Normally >=
    absorbs it, but when maybeReset() calls count.reset() the offset absorbs only the snapshot,
    not that observation, so expectedCount stays permanently one too high. I reproduced a
    permanently failing collect() (details inline on Buffer.java).
  2. observeDirect() takes a lock on every observation in steady state. JMH from
    HistogramBenchmark, both trees built from source (-f 1 -wi 3 -i 5 -r 2s -w 2s):
    prometheusClassicSingleThread −38%, prometheusClassicPerThread −44% (uncontended!),
    prometheusClassic −72%, prometheusNative −73%. The exclusion is only needed while a
    collection is in flight.
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Benchmark results

Benchmark run succeeded for a1657911077e0905b4281eec8e6d4c3a466ac779.

Prometheus Java Client Benchmarks

Run Information

  • Date: 2026-08-28T14:05:30Z
  • Commit: a165791
  • JDK: 25.0.3 (OpenJDK 64-Bit Server VM)
  • Benchmark config: 3 fork(s), 3 warmup, 5 measurement, 4 threads
  • Hardware: AMD EPYC 7763 64-Core Processor, 4 cores, 16 GB RAM
  • OS: Linux 6.17.0-1022-azure

Comparison with base

  • Head: a165791
  • Base: 6f88666
  • Metric direction: Throughput scores are higher-is-better; positive Head vs base deltas indicate faster performance.
  • Note: Base and head are compared on the same runner within each topic.
Benchmark PR Base Head vs base Regression verdict
CounterBenchmark.codahaleIncNoLabels 26.19K 25.68K +2.0% within noise
CounterBenchmark.openTelemetryAdd 12.66K 12.84K -1.4% within noise
CounterBenchmark.openTelemetryInc 14.42K 14.85K -2.9% within noise
CounterBenchmark.openTelemetryIncNoLabels 16.06K 16.56K -3.0% within noise
CounterBenchmark.prometheusAdd 24.78K 25.20K -1.7% within noise
CounterBenchmark.prometheusInc 26.08K 25.92K +0.6% within noise
CounterBenchmark.prometheusNoLabelsInc 25.46K 25.46K +0.0% within noise
CounterBenchmark.simpleclientAdd 6.25K 6.20K +0.7% within noise
CounterBenchmark.simpleclientInc 6.34K 6.45K -1.7% within noise
CounterBenchmark.simpleclientNoLabelsInc 6.34K 6.27K +1.1% within noise
HistogramBenchmark.openTelemetryClassic 827.18 802.27 +3.1% within noise
HistogramBenchmark.openTelemetryExponential 745.96 715.89 +4.2% within noise
HistogramBenchmark.prometheusClassic 5.86K 4.57K +28.3% within noise
HistogramBenchmark.prometheusClassicPerThread 12.99K 13.80K -5.9% meaningful regression
HistogramBenchmark.prometheusClassicSingleThread 5.38K 5.29K +1.8% within noise
HistogramBenchmark.prometheusNative 2.87K 3.09K -7.0% within noise
HistogramBenchmark.simpleclient 4.55K 4.49K +1.3% within noise
HistogramTextFormatBenchmark.openMetricsWriteToNull 27.06K 27.07K -0.0% within noise
HistogramTextFormatBenchmark.prometheusWriteToNull 27.58K 27.63K -0.2% within noise
TextFormatUtilBenchmark.openMetricsWriteToByteArray 528.04K 533.07K -0.9% within noise
TextFormatUtilBenchmark.openMetricsWriteToNull 548.86K 547.71K +0.2% within noise
TextFormatUtilBenchmark.prometheusWriteToByteArray 565.81K 570.75K -0.9% within noise
TextFormatUtilBenchmark.prometheusWriteToNull 574.29K 577.72K -0.6% within noise

Results for PR head

CounterBenchmark

Benchmark Score Error Units
codahaleIncNoLabels 26.19K ± 627.28 ops/s
prometheusInc 26.08K ± 961.70 ops/s
prometheusNoLabelsInc 25.46K ± 278.59 ops/s
prometheusAdd 24.78K ± 451.12 ops/s
openTelemetryIncNoLabels 16.06K ± 302.54 ops/s
openTelemetryInc 14.42K ± 243.32 ops/s
openTelemetryAdd 12.66K ± 240.95 ops/s
simpleclientInc 6.34K ± 104.28 ops/s
simpleclientNoLabelsInc 6.34K ± 79.81 ops/s
simpleclientAdd 6.25K ± 65.61 ops/s

HistogramBenchmark

Benchmark Score Error Units
prometheusClassicPerThread 12.99K ± 28.57 ops/s
prometheusClassic 5.86K ± 2.29K ops/s
prometheusClassicSingleThread 5.38K ± 28.15 ops/s
simpleclient 4.55K ± 72.63 ops/s
prometheusNative 2.87K ± 185.56 ops/s
openTelemetryClassic 827.18 ± 11.29 ops/s
openTelemetryExponential 745.96 ± 19.02 ops/s

HistogramTextFormatBenchmark

Benchmark Score Error Units
prometheusWriteToNull 27.58K ± 285.64 ops/s
openMetricsWriteToNull 27.06K ± 292.28 ops/s

TextFormatUtilBenchmark

Benchmark Score Error Units
prometheusWriteToNull 574.29K ± 11.57K ops/s
prometheusWriteToByteArray 565.81K ± 8.33K ops/s
openMetricsWriteToNull 548.86K ± 5.61K ops/s
openMetricsWriteToByteArray 528.04K ± 2.78K ops/s

Raw Results

Benchmark                                            Mode  Cnt          Score        Error  Units
CounterBenchmark.codahaleIncNoLabels                thrpt   15      26187.219    ± 627.282  ops/s
CounterBenchmark.openTelemetryAdd                   thrpt   15      12655.997    ± 240.951  ops/s
CounterBenchmark.openTelemetryInc                   thrpt   15      14421.717    ± 243.318  ops/s
CounterBenchmark.openTelemetryIncNoLabels           thrpt   15      16056.137    ± 302.537  ops/s
CounterBenchmark.prometheusAdd                      thrpt   15      24775.796    ± 451.120  ops/s
CounterBenchmark.prometheusInc                      thrpt   15      26081.724    ± 961.700  ops/s
CounterBenchmark.prometheusNoLabelsInc              thrpt   15      25461.465    ± 278.585  ops/s
CounterBenchmark.simpleclientAdd                    thrpt   15       6246.013     ± 65.613  ops/s
CounterBenchmark.simpleclientInc                    thrpt   15       6342.061    ± 104.278  ops/s
CounterBenchmark.simpleclientNoLabelsInc            thrpt   15       6340.728     ± 79.808  ops/s
HistogramBenchmark.openTelemetryClassic             thrpt   15        827.185     ± 11.292  ops/s
HistogramBenchmark.openTelemetryExponential         thrpt   15        745.963     ± 19.017  ops/s
HistogramBenchmark.prometheusClassic                thrpt   15       5858.828   ± 2292.348  ops/s
HistogramBenchmark.prometheusClassicPerThread       thrpt   15      12985.067     ± 28.568  ops/s
HistogramBenchmark.prometheusClassicSingleThread    thrpt   15       5384.465     ± 28.147  ops/s
HistogramBenchmark.prometheusNative                 thrpt   15       2873.003    ± 185.562  ops/s
HistogramBenchmark.simpleclient                     thrpt   15       4550.964     ± 72.627  ops/s
HistogramTextFormatBenchmark.openMetricsWriteToNull  thrpt   15      27060.099    ± 292.277  ops/s
HistogramTextFormatBenchmark.prometheusWriteToNull  thrpt   15      27577.921    ± 285.638  ops/s
TextFormatUtilBenchmark.openMetricsWriteToByteArray  thrpt   15     528036.007   ± 2778.380  ops/s
TextFormatUtilBenchmark.openMetricsWriteToNull      thrpt   15     548861.655   ± 5612.671  ops/s
TextFormatUtilBenchmark.prometheusWriteToByteArray  thrpt   15     565814.788   ± 8329.161  ops/s
TextFormatUtilBenchmark.prometheusWriteToNull       thrpt   15     574286.644  ± 11570.037  ops/s

Notes

  • Score = the JMH primary metric; throughput is higher-is-better and latency is lower-is-better.
  • Error = 99.9% confidence interval
  • Regression verdict requires comparable benchmark metadata, non-overlapping JMH confidence intervals, and a change of at least 5%; otherwise it is marked "within noise" or "inconclusive".
  • Scores for different benchmark methods are not ranked against one another; they may measure different workloads.

Benchmark Descriptions

Benchmark Description
CounterBenchmark Counter increment performance: Prometheus, OpenTelemetry, simpleclient, Codahale
HistogramBenchmark Histogram observation performance (classic vs native/exponential)
TextFormatUtilBenchmark Metric exposition format writing speed
Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

2 participants