Skip to content

Commit

Permalink
Merge pull request #127 from prometheus/sinjo-unaggregated-gauges
Browse files Browse the repository at this point in the history
Support :all as an aggregation mode in DirectFileStore
  • Loading branch information
Chris Sinjakli committed Jun 13, 2019
2 parents 6ea675e + e66ab1b commit 3652cf7
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 4 deletions.
12 changes: 11 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,15 @@ class MyComponent
end
```

### Reserved labels

The following labels are reserved by the client library, and attempting to use them in a
metric definition will result in a
`Prometheus::Client::LabelSetValidator::ReservedLabelError` being raised:

- `:job`
- `:instance`
- `:pid`

## Data Stores

Expand Down Expand Up @@ -362,7 +371,8 @@ summing the values of each process.

For Gauges, however, this may not be the right thing to do, depending on what they're
measuring. You might want to take the maximum or minimum value observed in any process,
rather than the sum of all of them.
rather than the sum of all of them. You may also want to export each process's individual
value.

In those cases, you should use the `store_settings` parameter when registering the
metric, to specify an `:aggregation` setting.
Expand Down
8 changes: 7 additions & 1 deletion lib/prometheus/client/data_stores/direct_file_store.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ module DataStores

class DirectFileStore
class InvalidStoreSettingsError < StandardError; end
AGGREGATION_MODES = [MAX = :max, MIN = :min, SUM = :sum]
AGGREGATION_MODES = [MAX = :max, MIN = :min, SUM = :sum, ALL = :all]
DEFAULT_METRIC_SETTINGS = { aggregation: SUM }

def initialize(dir:)
Expand Down Expand Up @@ -140,6 +140,10 @@ def in_process_sync
end

def store_key(labels)
if @values_aggregation_mode == ALL
labels[:pid] = process_id
end

labels.map{|k,v| "#{CGI::escape(k.to_s)}=#{CGI::escape(v.to_s)}"}.join('&')
end

Expand Down Expand Up @@ -168,6 +172,8 @@ def aggregate_values(values)
values.max
elsif @values_aggregation_mode == MIN
values.min
elsif @values_aggregation_mode == ALL
values.first
else
raise InvalidStoreSettingsError,
"Invalid Aggregation Mode: #{ @values_aggregation_mode }"
Expand Down
2 changes: 1 addition & 1 deletion lib/prometheus/client/label_set_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Client
# Prometheus specification.
class LabelSetValidator
# TODO: we might allow setting :instance in the future
BASE_RESERVED_LABELS = [:job, :instance].freeze
BASE_RESERVED_LABELS = [:job, :instance, :pid].freeze

class LabelSetError < StandardError; end
class InvalidLabelSetError < LabelSetError; end
Expand Down
36 changes: 36 additions & 0 deletions spec/prometheus/client/data_stores/direct_file_store_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,42 @@
end
end

context "with a metric that takes ALL instead of SUM" do
it "reports all the values from different processes" do
allow(Process).to receive(:pid).and_return(12345)
metric_store1 = subject.for_metric(
:metric_name,
metric_type: :gauge,
metric_settings: { aggregation: :all }
)
metric_store1.set(labels: { foo: "bar" }, val: 1)
metric_store1.set(labels: { foo: "baz" }, val: 7)
metric_store1.set(labels: { foo: "yyy" }, val: 3)

allow(Process).to receive(:pid).and_return(23456)
metric_store2 = subject.for_metric(
:metric_name,
metric_type: :gauge,
metric_settings: { aggregation: :all }
)
metric_store2.set(labels: { foo: "bar" }, val: 3)
metric_store2.set(labels: { foo: "baz" }, val: 2)
metric_store2.set(labels: { foo: "zzz" }, val: 1)

expect(metric_store1.all_values).to eq(
{ foo: "bar", pid: "12345" } => 1.0,
{ foo: "bar", pid: "23456" } => 3.0,
{ foo: "baz", pid: "12345" } => 7.0,
{ foo: "baz", pid: "23456" } => 2.0,
{ foo: "yyy", pid: "12345" } => 3.0,
{ foo: "zzz", pid: "23456" } => 1.0,
)

# Both processes should return the same value
expect(metric_store1.all_values).to eq(metric_store2.all_values)
end
end

it "resizes the File if metrics get too big" do
truncate_calls_count = 0
allow_any_instance_of(Prometheus::Client::DataStores::DirectFileStore::FileMappedDict).
Expand Down
2 changes: 1 addition & 1 deletion spec/prometheus/client/label_set_validator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
end

it 'raises ReservedLabelError if a label key is reserved' do
[:job, :instance].each do |label|
[:job, :instance, :pid].each do |label|
expect do
validator.validate_symbols!(label => 'value')
end.to raise_exception(described_class::ReservedLabelError)
Expand Down

0 comments on commit 3652cf7

Please sign in to comment.