Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support :all as an aggregation mode in DirectFileStore #127

Merged
merged 3 commits into from
Jun 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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