From 16e3b75381d2ca842e74706f0b864def790c8267 Mon Sep 17 00:00:00 2001 From: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> Date: Sun, 9 Aug 2026 12:25:33 +0000 Subject: [PATCH 1/6] fix(metrics): prevent negative gauge values for buffer metrics LocalMetrics sub/dec could drive buffer size gauges below zero when racey buffer accounting under/over-subtracted (issues #5303, #2712). That produced negative Prometheus series such as fluentd_output_status_buffer_total_bytes. Clamp gauge sub/dec at zero in LocalMetrics, and clamp stage/queue sizes when exporting buffer statistics. Fixes #5303 Signed-off-by: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> --- lib/fluent/plugin/buffer.rb | 6 +++++- lib/fluent/plugin/metrics_local.rb | 6 ++++++ test/plugin/test_metrics_local.rb | 22 ++++++++++++++++++++++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/lib/fluent/plugin/buffer.rb b/lib/fluent/plugin/buffer.rb index ea50eb3ecb..2b03c26ebd 100644 --- a/lib/fluent/plugin/buffer.rb +++ b/lib/fluent/plugin/buffer.rb @@ -917,7 +917,11 @@ def write_step_by_step(metadata, data, format, splits_count, &block) ] def statistics - stage_size, queue_size = @stage_size_metrics.get, @queue_size_metrics.get + # Clamp to non-negative: historical races can leave counters slightly + # negative (issues #5303, #2712). Exported Prometheus gauges must not + # report negative buffer sizes. + stage_size = [@stage_size_metrics.get, 0].max + queue_size = [@queue_size_metrics.get, 0].max buffer_space = 1.0 - ((stage_size + queue_size * 1.0) / @total_limit_size) @stage_length_metrics.set(@stage.size) @queue_length_metrics.set(@queue.size) diff --git a/lib/fluent/plugin/metrics_local.rb b/lib/fluent/plugin/metrics_local.rb index 8c8b0969b5..b00f094f59 100644 --- a/lib/fluent/plugin/metrics_local.rb +++ b/lib/fluent/plugin/metrics_local.rb @@ -62,7 +62,10 @@ def inc def dec_gauge @monitor.synchronize do + # Buffer size / length gauges must never go negative even if a race + # causes sub/dec to run more times than add/inc (see #5303). @store -= 1 + @store = 0 if @store < 0 end end @@ -74,7 +77,10 @@ def add(value) def sub_gauge(value) @monitor.synchronize do + # Prevent negative values that leak into Prometheus buffer metrics + # (fluentd_output_status_buffer_total_bytes, etc.). See #5303 / #2712. @store -= value + @store = 0 if @store < 0 end end diff --git a/test/plugin/test_metrics_local.rb b/test/plugin/test_metrics_local.rb index c140f18646..6c6ec8bb07 100644 --- a/test/plugin/test_metrics_local.rb +++ b/test/plugin/test_metrics_local.rb @@ -91,6 +91,28 @@ class LocalMetricsTest < ::Test::Unit::TestCase @m.set(10) assert_equal 10, @m.get # On gauge, value always should be overwritten. end + + # Prevents negative buffer size metrics exported to Prometheus (#5303) + test 'gauge sub does not go below zero' do + @m.set(5) + @m.sub(10) + assert_equal 0, @m.get + + @m.sub(1) + assert_equal 0, @m.get + end + + test 'gauge dec does not go below zero' do + assert_equal 0, @m.get + @m.dec + assert_equal 0, @m.get + + @m.set(1) + @m.dec + assert_equal 0, @m.get + @m.dec + assert_equal 0, @m.get + end end end end From ce7211a501b82a03edf2572e809d9ee36a44b8fb Mon Sep 17 00:00:00 2001 From: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> Date: Thu, 13 Aug 2026 09:08:31 +0000 Subject: [PATCH 2/6] test(buffer): assert statistics clamps negative gauges to zero Cover export-path clamping for stage_byte_size/queue_byte_size and non-negative total_queued_size when underlying gauges are negative. Signed-off-by: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> --- test/plugin/test_buffer.rb | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/plugin/test_buffer.rb b/test/plugin/test_buffer.rb index 9cb803f2f7..82e3f19bb9 100644 --- a/test/plugin/test_buffer.rb +++ b/test/plugin/test_buffer.rb @@ -1536,5 +1536,19 @@ def create_chunk_es(metadata, es) test 'returns available_buffer_space_ratios' do assert_equal 10.0, @p.statistics['buffer']['available_buffer_space_ratios'] end + + # Export path clamps independently of LocalMetrics sub/dec (#5303). + # set_gauge can still hold a negative value (e.g. external metrics backends). + test 'exports non-negative stage/queue byte sizes when gauges are negative' do + @p.stage_size_metrics.set(-50) + @p.queue_size_metrics.set(-100) + + stats = @p.statistics['buffer'] + assert_equal 0, stats['stage_byte_size'] + assert_equal 0, stats['queue_byte_size'] + assert_equal 0, stats['total_queued_size'] + assert stats['total_queued_size'] >= 0 + assert stats['available_buffer_space_ratios'] >= 0 + end end end From 9afb98cbc11d5e8a44bc93bca9fe639406ad69d2 Mon Sep 17 00:00:00 2001 From: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> Date: Sun, 23 Aug 2026 20:01:07 +0530 Subject: [PATCH 3/6] fix(metrics): clamp available_buffer_space_ratios and tighten tests When stage/queue counters overshoot total_limit_size the free-space ratio could go negative (or above 100). Clamp the ratio to [0, 1] after non-negative size export. Strengthen #statistics unit tests: - negative gauges export stage/queue/total as 0 and ratio 100.0 - overshoot beyond total_limit_size clamps ratio to 0.0 Refs #5303 Signed-off-by: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> --- lib/fluent/plugin/buffer.rb | 2 ++ test/plugin/test_buffer.rb | 17 ++++++++++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/lib/fluent/plugin/buffer.rb b/lib/fluent/plugin/buffer.rb index 2b03c26ebd..fa20faa02d 100644 --- a/lib/fluent/plugin/buffer.rb +++ b/lib/fluent/plugin/buffer.rb @@ -922,7 +922,9 @@ def statistics # report negative buffer sizes. stage_size = [@stage_size_metrics.get, 0].max queue_size = [@queue_size_metrics.get, 0].max + # Keep available-space ratio in [0, 1] even if counters overshoot total_limit_size. buffer_space = 1.0 - ((stage_size + queue_size * 1.0) / @total_limit_size) + buffer_space = [[buffer_space, 0.0].max, 1.0].min @stage_length_metrics.set(@stage.size) @queue_length_metrics.set(@queue.size) @available_buffer_space_ratios_metrics.set(buffer_space * 100) diff --git a/test/plugin/test_buffer.rb b/test/plugin/test_buffer.rb index 82e3f19bb9..ec208a50d7 100644 --- a/test/plugin/test_buffer.rb +++ b/test/plugin/test_buffer.rb @@ -1548,7 +1548,22 @@ def create_chunk_es(metadata, es) assert_equal 0, stats['queue_byte_size'] assert_equal 0, stats['total_queued_size'] assert stats['total_queued_size'] >= 0 - assert stats['available_buffer_space_ratios'] >= 0 + # Negative gauges floor to 0 usage => full free space (100.0% with total_limit_size=1024) + assert_equal 100.0, stats['available_buffer_space_ratios'] + end + + test 'clamps available_buffer_space_ratios when usage exceeds total_limit_size' do + # Simulate counter overshoot past configured limit + @p.stage_size_metrics.set(2000) + @p.queue_size_metrics.set(2000) + + stats = @p.statistics['buffer'] + assert_equal 2000, stats['stage_byte_size'] + assert_equal 2000, stats['queue_byte_size'] + assert_equal 4000, stats['total_queued_size'] + assert stats['available_buffer_space_ratios'] >= 0.0 + assert stats['available_buffer_space_ratios'] <= 100.0 + assert_equal 0.0, stats['available_buffer_space_ratios'] end end end From fedb9ff2fa80700f6cb8e5e901adcff3ba616a92 Mon Sep 17 00:00:00 2001 From: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> Date: Mon, 24 Aug 2026 08:35:58 +0530 Subject: [PATCH 4/6] fix(metrics): avoid NaN crash in available_buffer_space_ratios clamp Signed-off-by: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> --- lib/fluent/plugin/buffer.rb | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/fluent/plugin/buffer.rb b/lib/fluent/plugin/buffer.rb index fa20faa02d..cda7dc4363 100644 --- a/lib/fluent/plugin/buffer.rb +++ b/lib/fluent/plugin/buffer.rb @@ -923,8 +923,15 @@ def statistics stage_size = [@stage_size_metrics.get, 0].max queue_size = [@queue_size_metrics.get, 0].max # Keep available-space ratio in [0, 1] even if counters overshoot total_limit_size. - buffer_space = 1.0 - ((stage_size + queue_size * 1.0) / @total_limit_size) - buffer_space = [[buffer_space, 0.0].max, 1.0].min + # Avoid Array#max/min on NaN (e.g. 0/0 when total_limit_size is 0), which raises. + denom = @total_limit_size.to_f + if denom > 0.0 + buffer_space = 1.0 - ((stage_size + queue_size).to_f / denom) + buffer_space = 0.0 if buffer_space.nan? || buffer_space < 0.0 + buffer_space = 1.0 if buffer_space > 1.0 + else + buffer_space = 0.0 + end @stage_length_metrics.set(@stage.size) @queue_length_metrics.set(@queue.size) @available_buffer_space_ratios_metrics.set(buffer_space * 100) From 966b9ef7152dab7fae870955440a4283afa6a047 Mon Sep 17 00:00:00 2001 From: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> Date: Mon, 24 Aug 2026 08:36:15 +0530 Subject: [PATCH 5/6] test(buffer): cover available_buffer_space_ratios when total_limit_size is 0 Signed-off-by: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> --- test/plugin/test_buffer.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/plugin/test_buffer.rb b/test/plugin/test_buffer.rb index ec208a50d7..3819695510 100644 --- a/test/plugin/test_buffer.rb +++ b/test/plugin/test_buffer.rb @@ -1565,5 +1565,18 @@ def create_chunk_es(metadata, es) assert stats['available_buffer_space_ratios'] <= 100.0 assert_equal 0.0, stats['available_buffer_space_ratios'] end + + test 'available_buffer_space_ratios is safe when total_limit_size is zero' do + # 0/0 would be NaN; Array#max/min on NaN raises — must not crash export. + @p.instance_variable_set(:@total_limit_size, 0) + @p.stage_size_metrics.set(0) + @p.queue_size_metrics.set(0) + stats = @p.statistics['buffer'] + assert_equal 0, stats['stage_byte_size'] + assert_equal 0, stats['queue_byte_size'] + assert stats['available_buffer_space_ratios'] >= 0.0 + assert stats['available_buffer_space_ratios'] <= 100.0 + refute stats['available_buffer_space_ratios'].to_f.nan? + end end end From 3fe8d1892f6d606c16c46713b5fa962a2cf7233f Mon Sep 17 00:00:00 2001 From: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> Date: Thu, 27 Aug 2026 05:32:38 +0530 Subject: [PATCH 6/6] fix(metrics): export-only clamp for buffer stats (#5303) Address Watson1978 review on #5467: - Revert LocalMetrics sub/dec zero-clamping. Flooring the gauge store turns a transient deferred-add vs enqueue_chunk-sub race into a permanent stage_size over-count; Buffer#storable? then refuses writes. Keep self-correcting raw gauge semantics. - Keep export-path [get, 0].max for stage_byte_size/queue_byte_size/ total_queued_size so Prometheus consumers never see negatives. - Simplify available_buffer_space_ratios: denom > 0 ? ratio.clamp(0,1) : 0 (drop dead NaN guard inside the positive-denom branch). - Drop LocalMetrics clamp unit tests; keep statistics export tests. Signed-off-by: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> --- lib/fluent/plugin/buffer.rb | 18 ++++++------------ lib/fluent/plugin/metrics_local.rb | 6 ------ test/plugin/test_buffer.rb | 4 ++-- test/plugin/test_metrics_local.rb | 22 ---------------------- 4 files changed, 8 insertions(+), 42 deletions(-) diff --git a/lib/fluent/plugin/buffer.rb b/lib/fluent/plugin/buffer.rb index cda7dc4363..a183362553 100644 --- a/lib/fluent/plugin/buffer.rb +++ b/lib/fluent/plugin/buffer.rb @@ -917,21 +917,15 @@ def write_step_by_step(metadata, data, format, splits_count, &block) ] def statistics - # Clamp to non-negative: historical races can leave counters slightly - # negative (issues #5303, #2712). Exported Prometheus gauges must not - # report negative buffer sizes. + # Export-only clamp: internal gauges may go transiently negative during + # the deferred stage_size add vs enqueue_chunk sub race (#5303, #2712). + # Clamping the gauge store itself would turn that into a permanent + # over-count and break Buffer#storable? -- keep raw gauge semantics. stage_size = [@stage_size_metrics.get, 0].max queue_size = [@queue_size_metrics.get, 0].max - # Keep available-space ratio in [0, 1] even if counters overshoot total_limit_size. - # Avoid Array#max/min on NaN (e.g. 0/0 when total_limit_size is 0), which raises. denom = @total_limit_size.to_f - if denom > 0.0 - buffer_space = 1.0 - ((stage_size + queue_size).to_f / denom) - buffer_space = 0.0 if buffer_space.nan? || buffer_space < 0.0 - buffer_space = 1.0 if buffer_space > 1.0 - else - buffer_space = 0.0 - end + # denom > 0 already excludes 0/0 NaN; stage/queue are floored above. + buffer_space = denom > 0.0 ? (1.0 - (stage_size + queue_size).to_f / denom).clamp(0.0, 1.0) : 0.0 @stage_length_metrics.set(@stage.size) @queue_length_metrics.set(@queue.size) @available_buffer_space_ratios_metrics.set(buffer_space * 100) diff --git a/lib/fluent/plugin/metrics_local.rb b/lib/fluent/plugin/metrics_local.rb index b00f094f59..8c8b0969b5 100644 --- a/lib/fluent/plugin/metrics_local.rb +++ b/lib/fluent/plugin/metrics_local.rb @@ -62,10 +62,7 @@ def inc def dec_gauge @monitor.synchronize do - # Buffer size / length gauges must never go negative even if a race - # causes sub/dec to run more times than add/inc (see #5303). @store -= 1 - @store = 0 if @store < 0 end end @@ -77,10 +74,7 @@ def add(value) def sub_gauge(value) @monitor.synchronize do - # Prevent negative values that leak into Prometheus buffer metrics - # (fluentd_output_status_buffer_total_bytes, etc.). See #5303 / #2712. @store -= value - @store = 0 if @store < 0 end end diff --git a/test/plugin/test_buffer.rb b/test/plugin/test_buffer.rb index 3819695510..230b5f08cf 100644 --- a/test/plugin/test_buffer.rb +++ b/test/plugin/test_buffer.rb @@ -1537,8 +1537,8 @@ def create_chunk_es(metadata, es) assert_equal 10.0, @p.statistics['buffer']['available_buffer_space_ratios'] end - # Export path clamps independently of LocalMetrics sub/dec (#5303). - # set_gauge can still hold a negative value (e.g. external metrics backends). + # Export-only clamp: internal gauges may be negative (self-correcting race + # or set_gauge); statistics must still publish non-negative sizes (#5303). test 'exports non-negative stage/queue byte sizes when gauges are negative' do @p.stage_size_metrics.set(-50) @p.queue_size_metrics.set(-100) diff --git a/test/plugin/test_metrics_local.rb b/test/plugin/test_metrics_local.rb index 6c6ec8bb07..c140f18646 100644 --- a/test/plugin/test_metrics_local.rb +++ b/test/plugin/test_metrics_local.rb @@ -91,28 +91,6 @@ class LocalMetricsTest < ::Test::Unit::TestCase @m.set(10) assert_equal 10, @m.get # On gauge, value always should be overwritten. end - - # Prevents negative buffer size metrics exported to Prometheus (#5303) - test 'gauge sub does not go below zero' do - @m.set(5) - @m.sub(10) - assert_equal 0, @m.get - - @m.sub(1) - assert_equal 0, @m.get - end - - test 'gauge dec does not go below zero' do - assert_equal 0, @m.get - @m.dec - assert_equal 0, @m.get - - @m.set(1) - @m.dec - assert_equal 0, @m.get - @m.dec - assert_equal 0, @m.get - end end end end