fix(metrics): prevent negative gauge values for buffer metrics

[VedantMadane]

Which issue(s) this PR fixes:
Fixes #5303

What this PR does / why we need it:
Buffer size gauges (stage_byte_size, queue_byte_size, and derived total_queued_size) can go negative when Fluentd core under/over-subtracts during concurrent stage/queue transitions. Those values are mirrored by the Prometheus plugin as fluentd_output_status_buffer_total_bytes / fluentd_output_status_buffer_stage_byte_size, which is what #5303 reports.

@Watson1978 noted on the issue that the Prometheus plugin only mirrors Fluentd core counters; the drift lives in core (related to the class of races addressed in #2712 / #2734).

This PR:

1. Clamps LocalMetrics gauge sub/dec at zero so gauges never go negative under local metrics (the default).
2. Clamps stage/queue sizes when exporting buffer statistics so exporters never publish negative sizes even if an alternate metrics backend returns a negative intermediate value.
3. Adds unit tests for non-negative gauge sub/dec.

This does not attempt a full re-audit of every buffer lock path; it stops the user-visible bad export and prevents counter corruption from cascading.

Docs Changes:
N/A

Release Note:

• Fix negative buffer size metrics (stage_byte_size / queue_byte_size / total_queued_size) under concurrent buffer operations

[Watson1978]

Thanks for digging into #5303. However, I think clamping at zero is the wrong direction for the buffer counters: it converts a transient, self-healing negative into a permanent over-count.

Buffer#write intentionally defers the add. As part of the #2712 fix, staged_bytesizes_by_chunk is accumulated while each chunk is locked, but @stage_size_metrics.add(bytesize) only runs after the chunk locks are released (chunk.mon_exit at lib/fluent/plugin/buffer.rb:386 / :400, and the deferred add at :416).

That leaves a window in which the enqueue thread can run enqueue_chunk → @stage_size_metrics.sub(chunk.bytesize) (buffer.rb:504) while stage_size is still 0.

Take a 100-byte chunk in that window:

• Before this PR: the store goes to -100, then the pending add(100) brings it back to the correct 0. The negative reading is transient and self-correcting — ugly in the export, but the accounting converges.
• After this PR: sub clamps the store to 0, then the pending add(100) leaves stage_size == 100 while the real staged size is 0. The +100 never decays.

Under load this repeats on every flush cycle and accumulates. Note that storable? (buffer.rb:300) reads the raw .get, not the clamped statistics value:

@total_limit_size > @stage_size_metrics.get + @queue_size_metrics.get

So an effectively empty buffer eventually fails this check and the output starts raising BufferOverflowError permanently. That would turn a cosmetic metrics problem into a back-pressure / data-loss problem.

Two related points:

1. The negative value is currently the only signal operators have that add/sub pairing broke. Clamping silently removes the alert without removing the drift — an over-subtraction that does not cross zero was never visible to begin with, and after this change nothing is left to alert on.
2. set_gauge is not clamped, so buffer.stage_size = -1 still stores a negative. statistics would report 0 while storable? computes with -1, i.e. the exported number and the number the buffer actually acts on disagree.

I would suggest fixing the add/sub pairing in buffer.rb (the deferred add at :416 versus enqueue_chunk's sub at :504) rather than clamping the generic gauge type. If a defensive clamp for alternate metrics backends is still wanted, doing it once in the stage_size / queue_size accessors (buffer.rb:204-218) would cover statistics, storable? and in_monitor_agent in one place, and it should emit a log.warn so the underlying race stays diagnosable.

One more thing unrelated to the design question: @store = 0 if @store < 0 is now the last expression of the synchronize block, so dec_gauge / sub_gauge return nil on the non-clamped path. m.set(10); m.dec returned 9 before and returns nil now, while inc / add / set still return the value (and the existing tests assert that contract, e.g. assert_equal 1, @m.inc). Worth an explicit @store at the end of the block, plus a return-value assertion in the new tests.

[Copilot]

Pull request overview

This PR addresses negative buffer size gauge values that can surface under concurrent buffer stage/queue transitions, ensuring exported buffer metrics do not report negative byte sizes (notably impacting Prometheus consumers).

Changes:

• Clamp LocalMetrics gauge sub/dec operations so gauge values do not fall below zero.
• Clamp buffer statistics export for stage_byte_size/queue_byte_size to non-negative values.
• Add unit tests to verify gauge sub/dec never produce negative values.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
lib/fluent/plugin/metrics_local.rb	Floors gauge decrements/subtractions at zero to prevent negative gauges in the default local metrics backend.
lib/fluent/plugin/buffer.rb	Ensures exported buffer statistics never publish negative stage/queue byte sizes by clamping at export time.
test/plugin/test_metrics_local.rb	Adds unit tests validating non-negative behavior for gauge sub/dec.

Suppressed comments (1)

lib/fluent/plugin/buffer.rb:925

• available_buffer_space_ratios is described as a ratio of available space, but buffer_space can still go below 0.0 (or above 1.0) if stage/queue counters drift upward beyond @total_limit_size. This would export negative (or >100) ratios; consider clamping the computed ratio to [0.0, 1.0] before publishing.

        buffer_space = 1.0 - ((stage_size + queue_size * 1.0) / @total_limit_size)

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.