Skip to content

Commit dddba2f

Browse files
author
Daniel Magliola
committed
Do now allow DirectFileStore#increment when using :most_recent aggregation
If we do this, we'd be incrementing the value for *this* process, not the global one, which is almost certainly not what the user wants to do. This is not very pretty because we may end up raising an exception in production (as test/dev tend to not use DirectFileStore), but we consider it better than letting the user mangle their numbers and end up with incorrect metrics. Signed-off-by: Daniel Magliola <danielmagliola@gocardless.com>
1 parent f40a541 commit dddba2f

3 files changed

Lines changed: 21 additions & 0 deletions

File tree

README.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,9 @@ Counters, Histograms and Summaries are `SUM`med, and Gauges report all their val
351351
for each process), tagged with a `pid` label. You can also select `SUM`, `MAX`, `MIN`, or
352352
`MOST_RECENT` for your gauges, depending on your use case.
353353

354+
Please note that that the `MOST_RECENT` aggregation only works for gauges, and it does not
355+
allow the use of `increment` / `decrement`, you can only use `set`.
356+
354357
**Memory Usage**: When scraped by Prometheus, this store will read all these files, get all
355358
the values and aggregate them. We have notice this can have a noticeable effect on memory
356359
usage for your app. We recommend you test this in a realistic usage scenario to make sure

lib/prometheus/client/data_stores/direct_file_store.rb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,12 @@ def set(labels:, val:)
105105
end
106106

107107
def increment(labels:, by: 1)
108+
if @values_aggregation_mode == DirectFileStore::MOST_RECENT
109+
raise InvalidStoreSettingsError,
110+
"The :most_recent aggregation does not support the use of increment"\
111+
"/decrement"
112+
end
113+
108114
key = store_key(labels)
109115
in_process_sync do
110116
value = internal_store.read_value(key)

spec/prometheus/client/data_stores/direct_file_store_spec.rb

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,18 @@
329329
# Both processes should return the same value
330330
expect(metric_store1.all_values).to eq(metric_store2.all_values)
331331
end
332+
333+
it "does now allow `increment`, only `set`" do
334+
metric_store1 = subject.for_metric(
335+
:metric_name,
336+
metric_type: :gauge,
337+
metric_settings: { aggregation: :most_recent }
338+
)
339+
340+
expect do
341+
metric_store1.increment(labels: {})
342+
end.to raise_error(Prometheus::Client::DataStores::DirectFileStore::InvalidStoreSettingsError)
343+
end
332344
end
333345

334346
it "resizes the File if metrics get too big" do

0 commit comments

Comments
 (0)