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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

# Security Report: Unbounded Observation Buffer Growth During Stalled Scrape in prometheus/client_java

2 participants