From d85f7dccd54e21aa2faab3af855e52862a32f5a9 Mon Sep 17 00:00:00 2001 From: Daniel Magliola Date: Mon, 10 Sep 2018 14:13:35 +0100 Subject: [PATCH 01/18] Remove deprecation notice from 2 years ago Signed-off-by: Daniel Magliola --- lib/prometheus/client/summary.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/prometheus/client/summary.rb b/lib/prometheus/client/summary.rb index 7b78610e..0af0397c 100644 --- a/lib/prometheus/client/summary.rb +++ b/lib/prometheus/client/summary.rb @@ -8,8 +8,6 @@ module Client # Summary is an accumulator for samples. It captures Numeric data and # provides an efficient quantile calculation mechanism. class Summary < Metric - extend Gem::Deprecate - # Value represents the state of a Summary at a given point. class Value < Hash attr_accessor :sum, :total @@ -33,8 +31,6 @@ def observe(labels, value) label_set = label_set_for(labels) synchronize { @values[label_set].observe(value) } end - alias add observe - deprecate :add, :observe, 2016, 10 # Returns the value for the given label set def get(labels = {}) From 457d6747dbdb4a45a32e8c804f2219c7eebb048c Mon Sep 17 00:00:00 2001 From: Daniel Magliola Date: Thu, 18 Oct 2018 17:36:29 +0100 Subject: [PATCH 02/18] Remove support for Ruby 1.9.3 and update current versions Ruby < 2.0 has been EOL'd over 3 years ago, and we want to use kwargs, so we're dropping support for it. Signed-off-by: Daniel Magliola --- .travis.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 6ebf1139..b602a263 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,7 +5,6 @@ before_install: - | if [[ "$(ruby -e 'puts RUBY_VERSION')" != 1.* ]]; then gem update --system; fi rvm: - - 1.9.3 - 2.3.8 - 2.4.5 - 2.5.3 From 160c25e855dd6bb7918c92718bef85d216d60a46 Mon Sep 17 00:00:00 2001 From: Daniel Magliola Date: Mon, 10 Sep 2018 14:59:50 +0100 Subject: [PATCH 03/18] Add keyword arguments to the API where it makes sense This should make it cleaner and more obvious to call into these methods. Signed-off-by: Daniel Magliola --- README.md | 34 ++++++++++---------- lib/prometheus/client/counter.rb | 2 +- lib/prometheus/client/gauge.rb | 6 ++-- lib/prometheus/client/histogram.rb | 11 +++---- lib/prometheus/client/metric.rb | 4 +-- lib/prometheus/client/registry.rb | 21 ++++++------ lib/prometheus/client/summary.rb | 10 +++--- lib/prometheus/middleware/collector.rb | 13 ++++---- spec/examples/metric_example.rb | 14 ++++---- spec/prometheus/client/counter_spec.rb | 12 ++++--- spec/prometheus/client/gauge_spec.rb | 30 ++++++++--------- spec/prometheus/client/histogram_spec.rb | 27 ++++++++-------- spec/prometheus/client/registry_spec.rb | 8 ++--- spec/prometheus/client/summary_spec.rb | 26 ++++++++------- spec/prometheus/middleware/collector_spec.rb | 16 ++++----- spec/prometheus/middleware/exporter_spec.rb | 2 +- 16 files changed, 124 insertions(+), 112 deletions(-) diff --git a/README.md b/README.md index bd6e963a..80580f84 100644 --- a/README.md +++ b/README.md @@ -19,12 +19,12 @@ require 'prometheus/client' prometheus = Prometheus::Client.registry # create a new counter metric -http_requests = Prometheus::Client::Counter.new(:http_requests, 'A counter of HTTP requests made') +http_requests = Prometheus::Client::Counter.new(:http_requests, docstring: 'A counter of HTTP requests made') # register the metric prometheus.register(http_requests) # equivalent helper function -http_requests = prometheus.counter(:http_requests, 'A counter of HTTP requests made') +http_requests = prometheus.counter(:http_requests, docstring: 'A counter of HTTP requests made') # start using the counter http_requests.increment @@ -99,16 +99,16 @@ The following metric types are currently supported. Counter is a metric that exposes merely a sum or tally of things. ```ruby -counter = Prometheus::Client::Counter.new(:service_requests_total, '...') +counter = Prometheus::Client::Counter.new(:service_requests_total, docstring: '...') # increment the counter for a given label set -counter.increment({ service: 'foo' }) +counter.increment(labels: { service: 'foo' }) # increment by a given value -counter.increment({ service: 'bar' }, 5) +counter.increment(by: 5, labels: { service: 'bar' }) # get current value for a given label set -counter.get({ service: 'bar' }) +counter.get(labels: { service: 'bar' }) # => 5 ``` @@ -118,21 +118,21 @@ Gauge is a metric that exposes merely an instantaneous value or some snapshot thereof. ```ruby -gauge = Prometheus::Client::Gauge.new(:room_temperature_celsius, '...') +gauge = Prometheus::Client::Gauge.new(:room_temperature_celsius, docstring: '...') # set a value -gauge.set({ room: 'kitchen' }, 21.534) +gauge.set(21.534, labels: { room: 'kitchen' }) # retrieve the current value for a given label set -gauge.get({ room: 'kitchen' }) +gauge.get(labels: { room: 'kitchen' }) # => 21.534 # increment the value (default is 1) -gauge.increment({ room: 'kitchen' }) +gauge.increment(labels: { room: 'kitchen' }) # => 22.534 # decrement the value by a given value -gauge.decrement({ room: 'kitchen' }, 5) +gauge.decrement(by: 5, labels: { room: 'kitchen' }) # => 17.534 ``` @@ -143,13 +143,13 @@ response sizes) and counts them in configurable buckets. It also provides a sum of all observed values. ```ruby -histogram = Prometheus::Client::Histogram.new(:service_latency_seconds, '...') +histogram = Prometheus::Client::Histogram.new(:service_latency_seconds, docstring: '...') # record a value -histogram.observe({ service: 'users' }, Benchmark.realtime { service.call(arg) }) +histogram.observe(Benchmark.realtime { service.call(arg) }, labels: { service: 'users' }) # retrieve the current bucket values -histogram.get({ service: 'users' }) +histogram.get(labels: { service: 'users' }) # => { 0.005 => 3, 0.01 => 15, 0.025 => 18, ..., 2.5 => 42, 5 => 42, 10 = >42 } ``` @@ -159,13 +159,13 @@ Summary, similar to histograms, is an accumulator for samples. It captures Numeric data and provides an efficient percentile calculation mechanism. ```ruby -summary = Prometheus::Client::Summary.new(:service_latency_seconds, '...') +summary = Prometheus::Client::Summary.new(:service_latency_seconds, docstring: '...') # record a value -summary.observe({ service: 'database' }, Benchmark.realtime { service.call() }) +summary.observe(Benchmark.realtime { service.call() }, labels: { service: 'database' }) # retrieve the current quantile values -summary.get({ service: 'database' }) +summary.get(labels: { service: 'database' }) # => { 0.5 => 0.1233122, 0.9 => 3.4323, 0.99 => 5.3428231 } ``` diff --git a/lib/prometheus/client/counter.rb b/lib/prometheus/client/counter.rb index d1d85b48..4ed7fc45 100644 --- a/lib/prometheus/client/counter.rb +++ b/lib/prometheus/client/counter.rb @@ -10,7 +10,7 @@ def type :counter end - def increment(labels = {}, by = 1) + def increment(by: 1, labels: {}) raise ArgumentError, 'increment must be a non-negative number' if by < 0 label_set = label_set_for(labels) diff --git a/lib/prometheus/client/gauge.rb b/lib/prometheus/client/gauge.rb index ee4c0c50..1f24eb78 100644 --- a/lib/prometheus/client/gauge.rb +++ b/lib/prometheus/client/gauge.rb @@ -12,7 +12,7 @@ def type end # Sets the value for the given label set - def set(labels, value) + def set(value, labels: {}) unless value.is_a?(Numeric) raise ArgumentError, 'value must be a number' end @@ -22,7 +22,7 @@ def set(labels, value) # Increments Gauge value by 1 or adds the given value to the Gauge. # (The value can be negative, resulting in a decrease of the Gauge.) - def increment(labels = {}, by = 1) + def increment(by: 1, labels: {}) label_set = label_set_for(labels) synchronize do @values[label_set] ||= 0 @@ -32,7 +32,7 @@ def increment(labels = {}, by = 1) # Decrements Gauge value by 1 or subtracts the given value from the Gauge. # (The value can be negative, resulting in a increase of the Gauge.) - def decrement(labels = {}, by = 1) + def decrement(by: 1, labels: {}) label_set = label_set_for(labels) synchronize do @values[label_set] ||= 0 diff --git a/lib/prometheus/client/histogram.rb b/lib/prometheus/client/histogram.rb index 23c5899f..172c4827 100644 --- a/lib/prometheus/client/histogram.rb +++ b/lib/prometheus/client/histogram.rb @@ -12,7 +12,7 @@ class Histogram < Metric class Value < Hash attr_accessor :sum, :total - def initialize(buckets) + def initialize(buckets:) @sum = 0.0 @total = 0.0 @@ -38,19 +38,18 @@ def observe(value) 2.5, 5, 10].freeze # Offer a way to manually specify buckets - def initialize(name, docstring, base_labels = {}, - buckets = DEFAULT_BUCKETS) + def initialize(name, docstring:, base_labels: {}, buckets: DEFAULT_BUCKETS) raise ArgumentError, 'Unsorted buckets, typo?' unless sorted? buckets @buckets = buckets - super(name, docstring, base_labels) + super(name, docstring: docstring, base_labels: base_labels) end def type :histogram end - def observe(labels, value) + def observe(value, labels: {}) if labels[:le] raise ArgumentError, 'Label with name "le" is not permitted' end @@ -62,7 +61,7 @@ def observe(labels, value) private def default - Value.new(@buckets) + Value.new(buckets: @buckets) end def sorted?(bucket) diff --git a/lib/prometheus/client/metric.rb b/lib/prometheus/client/metric.rb index 7be679df..f5fb86b8 100644 --- a/lib/prometheus/client/metric.rb +++ b/lib/prometheus/client/metric.rb @@ -9,7 +9,7 @@ module Client class Metric attr_reader :name, :docstring, :base_labels - def initialize(name, docstring, base_labels = {}) + def initialize(name, docstring:, base_labels: {}) @mutex = Mutex.new @validator = LabelSetValidator.new @values = Hash.new { |hash, key| hash[key] = default } @@ -24,7 +24,7 @@ def initialize(name, docstring, base_labels = {}) end # Returns the value for the given label set - def get(labels = {}) + def get(labels: {}) @validator.valid?(labels) @values[labels] diff --git a/lib/prometheus/client/registry.rb b/lib/prometheus/client/registry.rb index 97e8847b..b80ac5ab 100644 --- a/lib/prometheus/client/registry.rb +++ b/lib/prometheus/client/registry.rb @@ -37,21 +37,24 @@ def unregister(name) end end - def counter(name, docstring, base_labels = {}) - register(Counter.new(name, docstring, base_labels)) + def counter(name, docstring:, base_labels: {}) + register(Counter.new(name, docstring: docstring, base_labels: base_labels)) end - def summary(name, docstring, base_labels = {}) - register(Summary.new(name, docstring, base_labels)) + def summary(name, docstring:, base_labels: {}) + register(Summary.new(name, docstring: docstring, base_labels: base_labels)) end - def gauge(name, docstring, base_labels = {}) - register(Gauge.new(name, docstring, base_labels)) + def gauge(name, docstring:, base_labels: {}) + register(Gauge.new(name, docstring: docstring, base_labels: base_labels)) end - def histogram(name, docstring, base_labels = {}, - buckets = Histogram::DEFAULT_BUCKETS) - register(Histogram.new(name, docstring, base_labels, buckets)) + def histogram(name, docstring:, base_labels: {}, + buckets: Histogram::DEFAULT_BUCKETS) + register(Histogram.new(name, + docstring: docstring, + base_labels: base_labels, + buckets: buckets)) end def exist?(name) diff --git a/lib/prometheus/client/summary.rb b/lib/prometheus/client/summary.rb index 0af0397c..0793e12a 100644 --- a/lib/prometheus/client/summary.rb +++ b/lib/prometheus/client/summary.rb @@ -12,7 +12,7 @@ class Summary < Metric class Value < Hash attr_accessor :sum, :total - def initialize(estimator) + def initialize(estimator:) @sum = estimator.sum @total = estimator.observations @@ -27,17 +27,17 @@ def type end # Records a given value. - def observe(labels, value) + def observe(value, labels: {}) label_set = label_set_for(labels) synchronize { @values[label_set].observe(value) } end # Returns the value for the given label set - def get(labels = {}) + def get(labels: {}) @validator.valid?(labels) synchronize do - Value.new(@values[labels]) + Value.new(estimator: @values[labels]) end end @@ -45,7 +45,7 @@ def get(labels = {}) def values synchronize do @values.each_with_object({}) do |(labels, value), memo| - memo[labels] = Value.new(value) + memo[labels] = Value.new(estimator: value) end end end diff --git a/lib/prometheus/middleware/collector.rb b/lib/prometheus/middleware/collector.rb index bf28fe0a..0b0c8e0e 100644 --- a/lib/prometheus/middleware/collector.rb +++ b/lib/prometheus/middleware/collector.rb @@ -64,18 +64,19 @@ def call(env) # :nodoc: def init_request_metrics @requests = @registry.counter( :"#{@metrics_prefix}_requests_total", - 'The total number of HTTP requests handled by the Rack application.', + docstring: + 'The total number of HTTP requests handled by the Rack application.', ) @durations = @registry.histogram( :"#{@metrics_prefix}_request_duration_seconds", - 'The HTTP response duration of the Rack application.', + docstring: 'The HTTP response duration of the Rack application.', ) end def init_exception_metrics @exceptions = @registry.counter( :"#{@metrics_prefix}_exceptions_total", - 'The total number of exceptions raised by the Rack application.', + docstring: 'The total number of exceptions raised by the Rack application.', ) end @@ -85,13 +86,13 @@ def trace(env) record(env, response.first.to_s, duration) return response rescue => exception - @exceptions.increment(exception: exception.class.name) + @exceptions.increment(labels: { exception: exception.class.name }) raise end def record(env, code, duration) - @requests.increment(@counter_lb.call(env, code)) - @durations.observe(@duration_lb.call(env, code), duration) + @requests.increment(labels: @counter_lb.call(env, code)) + @durations.observe(duration, labels: @duration_lb.call(env, code)) rescue # TODO: log unexpected exception during request recording nil diff --git a/spec/examples/metric_example.rb b/spec/examples/metric_example.rb index c577332e..3b20f09e 100644 --- a/spec/examples/metric_example.rb +++ b/spec/examples/metric_example.rb @@ -1,7 +1,7 @@ # encoding: UTF-8 shared_examples_for Prometheus::Client::Metric do - subject { described_class.new(:foo, 'foo description') } + subject { described_class.new(:foo, docstring: 'foo description') } describe '.new' do it 'returns a new metric' do @@ -12,19 +12,21 @@ exception = Prometheus::Client::LabelSetValidator::ReservedLabelError expect do - described_class.new(:foo, 'foo docstring', __name__: 'reserved') + described_class.new(:foo, + docstring: 'foo docstring', + base_labels: { __name__: 'reserved' }) end.to raise_exception exception end it 'raises an exception if the given name is blank' do expect do - described_class.new(nil, 'foo') + described_class.new(nil, docstring: 'foo') end.to raise_exception ArgumentError end it 'raises an exception if docstring is missing' do expect do - described_class.new(:foo, '') + described_class.new(:foo, docstring: '') end.to raise_exception ArgumentError end @@ -37,7 +39,7 @@ "abc\ndef".to_sym, ].each do |name| expect do - described_class.new(name, 'foo') + described_class.new(name, docstring: 'foo') end.to raise_exception(ArgumentError) end end @@ -55,7 +57,7 @@ end it 'returns the current metric value for a given label set' do - expect(subject.get(test: 'label')).to be_a(type) + expect(subject.get(labels: { test: 'label' })).to be_a(type) end end end diff --git a/spec/prometheus/client/counter_spec.rb b/spec/prometheus/client/counter_spec.rb index 69bf02a7..8768ddb5 100644 --- a/spec/prometheus/client/counter_spec.rb +++ b/spec/prometheus/client/counter_spec.rb @@ -4,7 +4,9 @@ require 'examples/metric_example' describe Prometheus::Client::Counter do - let(:counter) { Prometheus::Client::Counter.new(:foo, 'foo description') } + let(:counter) do + Prometheus::Client::Counter.new(:foo, docstring: 'foo description') + end it_behaves_like Prometheus::Client::Metric do let(:type) { Float } @@ -20,20 +22,20 @@ it 'increments the counter for a given label set' do expect do expect do - counter.increment(test: 'label') - end.to change { counter.get(test: 'label') }.by(1.0) + counter.increment(labels: { test: 'label' }) + end.to change { counter.get(labels: { test: 'label' }) }.by(1.0) end.to_not change { counter.get } end it 'increments the counter by a given value' do expect do - counter.increment({}, 5) + counter.increment(by: 5) end.to change { counter.get }.by(5.0) end it 'raises an ArgumentError on negative increments' do expect do - counter.increment({}, -1) + counter.increment(by: -1) end.to raise_error ArgumentError end diff --git a/spec/prometheus/client/gauge_spec.rb b/spec/prometheus/client/gauge_spec.rb index d3e092bb..20ebb7d5 100644 --- a/spec/prometheus/client/gauge_spec.rb +++ b/spec/prometheus/client/gauge_spec.rb @@ -4,7 +4,7 @@ require 'examples/metric_example' describe Prometheus::Client::Gauge do - let(:gauge) { Prometheus::Client::Gauge.new(:foo, 'foo description') } + let(:gauge) { Prometheus::Client::Gauge.new(:foo, docstring: 'foo description') } it_behaves_like Prometheus::Client::Metric do let(:type) { NilClass } @@ -13,22 +13,22 @@ describe '#set' do it 'sets a metric value' do expect do - gauge.set({}, 42) + gauge.set(42) end.to change { gauge.get }.from(nil).to(42) end it 'sets a metric value for a given label set' do expect do expect do - gauge.set({ test: 'value' }, 42) - end.to change { gauge.get(test: 'value') }.from(nil).to(42) + gauge.set(42, labels: { test: 'value' }) + end.to change { gauge.get(labels: { test: 'value' }) }.from(nil).to(42) end.to_not change { gauge.get } end context 'given an invalid value' do it 'raises an ArgumentError' do expect do - gauge.set({}, nil) + gauge.set(nil) end.to raise_exception(ArgumentError) end end @@ -36,7 +36,7 @@ describe '#increment' do before do - gauge.set(RSpec.current_example.metadata[:labels] || {}, 0) + gauge.set(0, labels: RSpec.current_example.metadata[:labels] || {}) end it 'increments the gauge' do @@ -48,14 +48,14 @@ it 'increments the gauge for a given label set', labels: { test: 'one' } do expect do expect do - gauge.increment(test: 'one') - end.to change { gauge.get(test: 'one') }.by(1.0) - end.to_not change { gauge.get(test: 'another') } + gauge.increment(labels: { test: 'one' }) + end.to change { gauge.get(labels: { test: 'one' }) }.by(1.0) + end.to_not change { gauge.get(labels: { test: 'another' }) } end it 'increments the gauge by a given value' do expect do - gauge.increment({}, 5) + gauge.increment(by: 5) end.to change { gauge.get }.by(5.0) end @@ -76,7 +76,7 @@ describe '#decrement' do before do - gauge.set(RSpec.current_example.metadata[:labels] || {}, 0) + gauge.set(0, labels: RSpec.current_example.metadata[:labels] || {}) end it 'increments the gauge' do @@ -88,14 +88,14 @@ it 'decrements the gauge for a given label set', labels: { test: 'one' } do expect do expect do - gauge.decrement(test: 'one') - end.to change { gauge.get(test: 'one') }.by(-1.0) - end.to_not change { gauge.get(test: 'another') } + gauge.decrement(labels: { test: 'one' }) + end.to change { gauge.get(labels: { test: 'one' }) }.by(-1.0) + end.to_not change { gauge.get(labels: { test: 'another' }) } end it 'decrements the gauge by a given value' do expect do - gauge.decrement({}, 5) + gauge.decrement(by: 5) end.to change { gauge.get }.by(-5.0) end diff --git a/spec/prometheus/client/histogram_spec.rb b/spec/prometheus/client/histogram_spec.rb index cee7dbae..ed592abb 100644 --- a/spec/prometheus/client/histogram_spec.rb +++ b/spec/prometheus/client/histogram_spec.rb @@ -5,7 +5,7 @@ describe Prometheus::Client::Histogram do let(:histogram) do - described_class.new(:bar, 'bar description', {}, [2.5, 5, 10]) + described_class.new(:bar, docstring: 'bar description', buckets: [2.5, 5, 10]) end it_behaves_like Prometheus::Client::Metric do @@ -15,7 +15,7 @@ describe '#initialization' do it 'raise error for unsorted buckets' do expect do - described_class.new(:bar, 'bar description', {}, [5, 2.5, 10]) + described_class.new(:bar, docstring: 'bar description', buckets: [5, 2.5, 10]) end.to raise_error ArgumentError end end @@ -23,45 +23,46 @@ describe '#observe' do it 'records the given value' do expect do - histogram.observe({}, 5) + histogram.observe(5) end.to change { histogram.get } end it 'raise error for le labels' do expect do - histogram.observe({ le: 1 }, 5) + histogram.observe(5, labels: { le: 1 }) end.to raise_error ArgumentError end end describe '#get' do before do - histogram.observe({ foo: 'bar' }, 3) - histogram.observe({ foo: 'bar' }, 5.2) - histogram.observe({ foo: 'bar' }, 13) - histogram.observe({ foo: 'bar' }, 4) + histogram.observe(3, labels: { foo: 'bar' }) + histogram.observe(5.2, labels: { foo: 'bar' }) + histogram.observe(13, labels: { foo: 'bar' }) + histogram.observe(4, labels: { foo: 'bar' }) end it 'returns a set of buckets values' do - expect(histogram.get(foo: 'bar')).to eql(2.5 => 0.0, 5 => 2.0, 10 => 3.0) + expect(histogram.get(labels: { foo: 'bar' })) + .to eql(2.5 => 0.0, 5 => 2.0, 10 => 3.0) end it 'returns a value which responds to #sum and #total' do - value = histogram.get(foo: 'bar') + value = histogram.get(labels: { foo: 'bar' }) expect(value.sum).to eql(25.2) expect(value.total).to eql(4.0) end it 'uses zero as default value' do - expect(histogram.get({})).to eql(2.5 => 0.0, 5 => 0.0, 10 => 0.0) + expect(histogram.get).to eql(2.5 => 0.0, 5 => 0.0, 10 => 0.0) end end describe '#values' do it 'returns a hash of all recorded summaries' do - histogram.observe({ status: 'bar' }, 3) - histogram.observe({ status: 'foo' }, 6) + histogram.observe(3, labels: { status: 'bar' }) + histogram.observe(6, labels: { status: 'foo' }) expect(histogram.values).to eql( { status: 'bar' } => { 2.5 => 0.0, 5 => 1.0, 10 => 1.0 }, diff --git a/spec/prometheus/client/registry_spec.rb b/spec/prometheus/client/registry_spec.rb index 3ca9db47..32727d00 100644 --- a/spec/prometheus/client/registry_spec.rb +++ b/spec/prometheus/client/registry_spec.rb @@ -68,7 +68,7 @@ def registry.exist?(*args) describe '#counter' do it 'registers a new counter metric container and returns the counter' do - metric = registry.counter(:test, 'test docstring') + metric = registry.counter(:test, docstring: 'test docstring') expect(metric).to be_a(Prometheus::Client::Counter) end @@ -76,7 +76,7 @@ def registry.exist?(*args) describe '#gauge' do it 'registers a new gauge metric container and returns the gauge' do - metric = registry.gauge(:test, 'test docstring') + metric = registry.gauge(:test, docstring: 'test docstring') expect(metric).to be_a(Prometheus::Client::Gauge) end @@ -84,7 +84,7 @@ def registry.exist?(*args) describe '#summary' do it 'registers a new summary metric container and returns the summary' do - metric = registry.summary(:test, 'test docstring') + metric = registry.summary(:test, docstring: 'test docstring') expect(metric).to be_a(Prometheus::Client::Summary) end @@ -92,7 +92,7 @@ def registry.exist?(*args) describe '#histogram' do it 'registers a new histogram metric container and returns the histogram' do - metric = registry.histogram(:test, 'test docstring') + metric = registry.histogram(:test, docstring: 'test docstring') expect(metric).to be_a(Prometheus::Client::Histogram) end diff --git a/spec/prometheus/client/summary_spec.rb b/spec/prometheus/client/summary_spec.rb index 627039a5..bae3327d 100644 --- a/spec/prometheus/client/summary_spec.rb +++ b/spec/prometheus/client/summary_spec.rb @@ -4,7 +4,10 @@ require 'examples/metric_example' describe Prometheus::Client::Summary do - let(:summary) { Prometheus::Client::Summary.new(:bar, 'bar description') } + let(:summary) do + Prometheus::Client::Summary.new(:bar, + docstring: 'bar description') + end it_behaves_like Prometheus::Client::Metric do let(:type) { Hash } @@ -13,39 +16,40 @@ describe '#observe' do it 'records the given value' do expect do - summary.observe({}, 5) + summary.observe(5) end.to change { summary.get } end end describe '#get' do before do - summary.observe({ foo: 'bar' }, 3) - summary.observe({ foo: 'bar' }, 5.2) - summary.observe({ foo: 'bar' }, 13) - summary.observe({ foo: 'bar' }, 4) + summary.observe(3, labels: { foo: 'bar' }) + summary.observe(5.2, labels: { foo: 'bar' }) + summary.observe(13, labels: { foo: 'bar' }) + summary.observe(4, labels: { foo: 'bar' }) end it 'returns a set of quantile values' do - expect(summary.get(foo: 'bar')).to eql(0.5 => 4, 0.9 => 5.2, 0.99 => 5.2) + expect(summary.get(labels: { foo: 'bar' })) + .to eql(0.5 => 4, 0.9 => 5.2, 0.99 => 5.2) end it 'returns a value which responds to #sum and #total' do - value = summary.get(foo: 'bar') + value = summary.get(labels: { foo: 'bar' }) expect(value.sum).to eql(25.2) expect(value.total).to eql(4) end it 'uses nil as default value' do - expect(summary.get({})).to eql(0.5 => nil, 0.9 => nil, 0.99 => nil) + expect(summary.get).to eql(0.5 => nil, 0.9 => nil, 0.99 => nil) end end describe '#values' do it 'returns a hash of all recorded summaries' do - summary.observe({ status: 'bar' }, 3) - summary.observe({ status: 'foo' }, 5) + summary.observe(3, labels: { status: 'bar' }) + summary.observe(5, labels: { status: 'foo' }) expect(summary.values).to eql( { status: 'bar' } => { 0.5 => 3, 0.9 => 3, 0.99 => 3 }, diff --git a/spec/prometheus/middleware/collector_spec.rb b/spec/prometheus/middleware/collector_spec.rb index 5dd38708..27b18ea3 100644 --- a/spec/prometheus/middleware/collector_spec.rb +++ b/spec/prometheus/middleware/collector_spec.rb @@ -41,11 +41,11 @@ metric = :http_server_requests_total labels = { method: 'get', path: '/foo', code: '200' } - expect(registry.get(metric).get(labels)).to eql(1.0) + expect(registry.get(metric).get(labels: labels)).to eql(1.0) metric = :http_server_request_duration_seconds labels = { method: 'get', path: '/foo' } - expect(registry.get(metric).get(labels)).to include(0.1 => 0, 0.25 => 1) + expect(registry.get(metric).get(labels: labels)).to include(0.1 => 0, 0.25 => 1) end it 'normalizes paths containing numeric IDs by default' do @@ -55,11 +55,11 @@ metric = :http_server_requests_total labels = { method: 'get', path: '/foo/:id/bars', code: '200' } - expect(registry.get(metric).get(labels)).to eql(1.0) + expect(registry.get(metric).get(labels: labels)).to eql(1.0) metric = :http_server_request_duration_seconds labels = { method: 'get', path: '/foo/:id/bars' } - expect(registry.get(metric).get(labels)).to include(0.1 => 0, 0.5 => 1) + expect(registry.get(metric).get(labels: labels)).to include(0.1 => 0, 0.5 => 1) end it 'normalizes paths containing UUIDs by default' do @@ -69,11 +69,11 @@ metric = :http_server_requests_total labels = { method: 'get', path: '/foo/:uuid/bars', code: '200' } - expect(registry.get(metric).get(labels)).to eql(1.0) + expect(registry.get(metric).get(labels: labels)).to eql(1.0) metric = :http_server_request_duration_seconds labels = { method: 'get', path: '/foo/:uuid/bars' } - expect(registry.get(metric).get(labels)).to include(0.1 => 0, 0.5 => 1) + expect(registry.get(metric).get(labels: labels)).to include(0.1 => 0, 0.5 => 1) end context 'when the app raises an exception' do @@ -94,7 +94,7 @@ metric = :http_server_exceptions_total labels = { exception: 'NoMethodError' } - expect(registry.get(metric).get(labels)).to eql(1.0) + expect(registry.get(metric).get(labels: labels)).to eql(1.0) end end @@ -117,7 +117,7 @@ metric = :http_server_requests_total labels = { method: 'get', code: '200' } - expect(registry.get(metric).get(labels)).to eql(1.0) + expect(registry.get(metric).get(labels: labels)).to eql(1.0) end end diff --git a/spec/prometheus/middleware/exporter_spec.rb b/spec/prometheus/middleware/exporter_spec.rb index b1a1a791..8916513b 100644 --- a/spec/prometheus/middleware/exporter_spec.rb +++ b/spec/prometheus/middleware/exporter_spec.rb @@ -29,7 +29,7 @@ shared_examples 'ok' do |headers, fmt| it "responds with 200 OK and Content-Type #{fmt::CONTENT_TYPE}" do - registry.counter(:foo, 'foo counter').increment({}, 9) + registry.counter(:foo, docstring: 'foo counter').increment(by: 9) get '/metrics', nil, headers From 1a7235f4528986e1438c4bd429f20652df21b49d Mon Sep 17 00:00:00 2001 From: Daniel Magliola Date: Tue, 11 Sep 2018 12:02:37 +0100 Subject: [PATCH 04/18] Require declaration of labels when creating metrics Instead of allowing the first observation / setting of a metric to have any labels, and then validating that any further observations match those, we now require declaring what labels will be specified at the time of instantiating the metric. The Validator now only needs to verify that the provided labels match the expectation, without needing to keep track of the first observation. For now, it's still caching the validations, for performance, but that should disappear in a soon-to-come commit. `base_labels` now works slightly different. Instead of being a "base" value that is set when creating the metric, and then combined with the "other" labels at the time of exporting, it's now called `preset_values`, and it that get merged as part of the labels stored with every observation. This means the storage that holds the values will always have all the labels, including the common ones. This allows having a method like the `labels()` one recommended by the best practices, to have a "view" of a metric with some labels pre-applied. This will be added on a future commit, once we have a central data store. Finally, this needed a little adaptation of the Collector middleware that comes bundled in. This Collector automatically tracks metrics with some pre-defined labels, but allows the user to define which labels they want, on a per-request basis, by specifying a `proc` that will generate the labels given a Rack `env`. Given this design, we can't know what labels the custom `proc` will return at the time of initializing the metric, and we don't have an `env` to pass to them, so the interface of these procs has changed such that they now need to be able to handle an empty `env`, and still return a `hash` with all the right keys. Signed-off-by: Daniel Magliola --- README.md | 52 +++++++++++++-- examples/rack/README.md | 2 + lib/prometheus/client/formats/text.rb | 8 +-- lib/prometheus/client/histogram.rb | 19 ++++-- lib/prometheus/client/label_set_validator.rb | 18 ++++-- lib/prometheus/client/metric.rb | 23 ++++--- lib/prometheus/client/registry.rb | 26 +++++--- lib/prometheus/client/summary.rb | 4 ++ lib/prometheus/middleware/collector.rb | 12 ++++ spec/examples/metric_example.rb | 10 ++- spec/prometheus/client/counter_spec.rb | 24 +++++-- spec/prometheus/client/formats/text_spec.rb | 15 ++--- spec/prometheus/client/gauge_spec.rb | 64 +++++++++++++++---- spec/prometheus/client/histogram_spec.rb | 39 ++++++++++- .../client/label_set_validator_spec.rb | 25 ++++++-- spec/prometheus/client/summary_spec.rb | 40 +++++++++++- spec/prometheus/middleware/collector_spec.rb | 2 + 17 files changed, 301 insertions(+), 82 deletions(-) diff --git a/README.md b/README.md index 80580f84..6f37040e 100644 --- a/README.md +++ b/README.md @@ -99,7 +99,7 @@ The following metric types are currently supported. Counter is a metric that exposes merely a sum or tally of things. ```ruby -counter = Prometheus::Client::Counter.new(:service_requests_total, docstring: '...') +counter = Prometheus::Client::Counter.new(:service_requests_total, docstring: '...', labels: [:service]) # increment the counter for a given label set counter.increment(labels: { service: 'foo' }) @@ -118,7 +118,7 @@ Gauge is a metric that exposes merely an instantaneous value or some snapshot thereof. ```ruby -gauge = Prometheus::Client::Gauge.new(:room_temperature_celsius, docstring: '...') +gauge = Prometheus::Client::Gauge.new(:room_temperature_celsius, docstring: '...', labels: [:room]) # set a value gauge.set(21.534, labels: { room: 'kitchen' }) @@ -143,7 +143,7 @@ response sizes) and counts them in configurable buckets. It also provides a sum of all observed values. ```ruby -histogram = Prometheus::Client::Histogram.new(:service_latency_seconds, docstring: '...') +histogram = Prometheus::Client::Histogram.new(:service_latency_seconds, docstring: '...', labels: [:service]) # record a value histogram.observe(Benchmark.realtime { service.call(arg) }, labels: { service: 'users' }) @@ -159,7 +159,7 @@ Summary, similar to histograms, is an accumulator for samples. It captures Numeric data and provides an efficient percentile calculation mechanism. ```ruby -summary = Prometheus::Client::Summary.new(:service_latency_seconds, docstring: '...') +summary = Prometheus::Client::Summary.new(:service_latency_seconds, docstring: '...', labels: [:service]) # record a value summary.observe(Benchmark.realtime { service.call() }, labels: { service: 'database' }) @@ -169,6 +169,50 @@ summary.get(labels: { service: 'database' }) # => { 0.5 => 0.1233122, 0.9 => 3.4323, 0.99 => 5.3428231 } ``` +## Labels + +All metrics can have labels, allowing grouping of related time series. + +Labels are an extremely powerful feature, but one that must be used with care. +Refer to the best practices on [naming](https://prometheus.io/docs/practices/naming/) and +[labels](https://prometheus.io/docs/practices/instrumentation/#use-labels). + +Most importantly, avoid labels that can have a large number of possible values (high +cardinality). For example, an HTTP Status Code is a good label. A User ID is **not**. + +Labels are specified optionally when updating metrics, as a hash of `label_name => value`. +Refer to [the Prometheus documentation](https://prometheus.io/docs/concepts/data_model/#metric-names-and-labels) +as to what's a valid `label_name`. + +In order for a metric to accept labels, their names must be specified when first initializing +the metric. Then, when the metric is updated, all the specified labels must be present. + +Example: + +```ruby +https_requests_total = Counter.new(:http_requests_total, docstring: '...', labels: [:service, :status_code]) + +# increment the counter for a given label set +https_requests_total.increment(labels: { service: "my_service", status_code: response.status_code }) +``` + +### Pre-set Label Values + +You can also "pre-set" some of these label values, if they'll always be the same, so you don't +need to specify them every time: + +```ruby +https_requests_total = Counter.new(:http_requests_total, + docstring: '...', + labels: [:service, :status_code], + preset_labels: { service: "my_service" }) + +# increment the counter for a given label set +https_requests_total.increment(labels: { status_code: response.status_code }) +``` + + + ## Tests Install necessary development gems with `bundle install` and run tests with diff --git a/examples/rack/README.md b/examples/rack/README.md index aecdfc6c..d96546d0 100644 --- a/examples/rack/README.md +++ b/examples/rack/README.md @@ -49,6 +49,8 @@ something like this: ```ruby use Prometheus::Middleware::Collector, counter_label_builder: ->(env, code) { + next { code: nil, method: nil, host: nil, path: nil } if env.empty? + { code: code, method: env['REQUEST_METHOD'].downcase, diff --git a/lib/prometheus/client/formats/text.rb b/lib/prometheus/client/formats/text.rb index 040435fa..a4ea75f8 100644 --- a/lib/prometheus/client/formats/text.rb +++ b/lib/prometheus/client/formats/text.rb @@ -40,14 +40,12 @@ class << self private def representation(metric, label_set, value, &block) - set = metric.base_labels.merge(label_set) - if metric.type == :summary - summary(metric.name, set, value, &block) + summary(metric.name, label_set, value, &block) elsif metric.type == :histogram - histogram(metric.name, set, value, &block) + histogram(metric.name, label_set, value, &block) else - yield metric(metric.name, labels(set), value) + yield metric(metric.name, labels(label_set), value) end end diff --git a/lib/prometheus/client/histogram.rb b/lib/prometheus/client/histogram.rb index 172c4827..03f7cec4 100644 --- a/lib/prometheus/client/histogram.rb +++ b/lib/prometheus/client/histogram.rb @@ -38,11 +38,18 @@ def observe(value) 2.5, 5, 10].freeze # Offer a way to manually specify buckets - def initialize(name, docstring:, base_labels: {}, buckets: DEFAULT_BUCKETS) + def initialize(name, + docstring:, + labels: [], + preset_labels: {}, + buckets: DEFAULT_BUCKETS) raise ArgumentError, 'Unsorted buckets, typo?' unless sorted? buckets @buckets = buckets - super(name, docstring: docstring, base_labels: base_labels) + super(name, + docstring: docstring, + labels: labels, + preset_labels: preset_labels) end def type @@ -50,16 +57,16 @@ def type end def observe(value, labels: {}) - if labels[:le] - raise ArgumentError, 'Label with name "le" is not permitted' - end - label_set = label_set_for(labels) synchronize { @values[label_set].observe(value) } end private + def reserved_labels + [:le] + end + def default Value.new(buckets: @buckets) end diff --git a/lib/prometheus/client/label_set_validator.rb b/lib/prometheus/client/label_set_validator.rb index 9536f4fa..943f9a77 100644 --- a/lib/prometheus/client/label_set_validator.rb +++ b/lib/prometheus/client/label_set_validator.rb @@ -6,14 +6,18 @@ module Client # Prometheus specification. class LabelSetValidator # TODO: we might allow setting :instance in the future - RESERVED_LABELS = [:job, :instance].freeze + BASE_RESERVED_LABELS = [:job, :instance].freeze class LabelSetError < StandardError; end class InvalidLabelSetError < LabelSetError; end class InvalidLabelError < LabelSetError; end class ReservedLabelError < LabelSetError; end - def initialize + attr_reader :expected_labels, :reserved_labels + + def initialize(expected_labels:, reserved_labels: []) + @expected_labels = expected_labels.sort + @reserved_labels = BASE_RESERVED_LABELS + reserved_labels @validated = {} end @@ -34,10 +38,10 @@ def validate(labels) valid?(labels) - unless @validated.empty? || match?(labels, @validated.first.last) + unless keys_match?(labels) raise InvalidLabelSetError, "labels must have the same signature " \ "(keys given: #{labels.keys.sort} vs." \ - " keys expected: #{@validated.first.last.keys.sort}" + " keys expected: #{expected_labels}" end @validated[labels.hash] = labels @@ -45,8 +49,8 @@ def validate(labels) private - def match?(a, b) - a.keys.sort == b.keys.sort + def keys_match?(labels) + labels.keys.sort == expected_labels end def validate_symbol(key) @@ -62,7 +66,7 @@ def validate_name(key) end def validate_reserved_key(key) - return true unless RESERVED_LABELS.include?(key) + return true unless reserved_labels.include?(key) raise ReservedLabelError, "#{key} is reserved" end diff --git a/lib/prometheus/client/metric.rb b/lib/prometheus/client/metric.rb index f5fb86b8..eafce142 100644 --- a/lib/prometheus/client/metric.rb +++ b/lib/prometheus/client/metric.rb @@ -7,27 +7,28 @@ module Prometheus module Client # Metric class Metric - attr_reader :name, :docstring, :base_labels + attr_reader :name, :docstring, :preset_labels - def initialize(name, docstring:, base_labels: {}) + def initialize(name, docstring:, labels: [], preset_labels: {}) @mutex = Mutex.new - @validator = LabelSetValidator.new + @validator = LabelSetValidator.new(expected_labels: labels, + reserved_labels: reserved_labels) @values = Hash.new { |hash, key| hash[key] = default } validate_name(name) validate_docstring(docstring) - @validator.valid?(base_labels) + @validator.valid?(labels) + @validator.valid?(preset_labels) @name = name @docstring = docstring - @base_labels = base_labels + @preset_labels = preset_labels end # Returns the value for the given label set def get(labels: {}) - @validator.valid?(labels) - - @values[labels] + label_set = label_set_for(labels) + @values[label_set] end # Returns all label sets with their values @@ -41,6 +42,10 @@ def values private + def reserved_labels + [] + end + def default nil end @@ -62,7 +67,7 @@ def validate_docstring(docstring) end def label_set_for(labels) - @validator.validate(labels) + @validator.validate(preset_labels.merge(labels)) end def synchronize diff --git a/lib/prometheus/client/registry.rb b/lib/prometheus/client/registry.rb index b80ac5ab..58893b3c 100644 --- a/lib/prometheus/client/registry.rb +++ b/lib/prometheus/client/registry.rb @@ -37,23 +37,33 @@ def unregister(name) end end - def counter(name, docstring:, base_labels: {}) - register(Counter.new(name, docstring: docstring, base_labels: base_labels)) + def counter(name, docstring:, labels: [], preset_labels: {}) + register(Counter.new(name, + docstring: docstring, + labels: labels, + preset_labels: preset_labels)) end - def summary(name, docstring:, base_labels: {}) - register(Summary.new(name, docstring: docstring, base_labels: base_labels)) + def summary(name, docstring:, labels: [], preset_labels: {}) + register(Summary.new(name, + docstring: docstring, + labels: labels, + preset_labels: preset_labels)) end - def gauge(name, docstring:, base_labels: {}) - register(Gauge.new(name, docstring: docstring, base_labels: base_labels)) + def gauge(name, docstring:, labels: [], preset_labels: {}) + register(Gauge.new(name, + docstring: docstring, + labels: labels, + preset_labels: preset_labels)) end - def histogram(name, docstring:, base_labels: {}, + def histogram(name, docstring:, labels: [], preset_labels: {}, buckets: Histogram::DEFAULT_BUCKETS) register(Histogram.new(name, docstring: docstring, - base_labels: base_labels, + labels: labels, + preset_labels: preset_labels, buckets: buckets)) end diff --git a/lib/prometheus/client/summary.rb b/lib/prometheus/client/summary.rb index 0793e12a..455dee71 100644 --- a/lib/prometheus/client/summary.rb +++ b/lib/prometheus/client/summary.rb @@ -52,6 +52,10 @@ def values private + def reserved_labels + [:quantile] + end + def default Quantile::Estimator.new end diff --git a/lib/prometheus/middleware/collector.rb b/lib/prometheus/middleware/collector.rb index 0b0c8e0e..08445ec3 100644 --- a/lib/prometheus/middleware/collector.rb +++ b/lib/prometheus/middleware/collector.rb @@ -20,6 +20,11 @@ module Middleware # # The request duration metric is broken down by method and path by default. # Set the `:duration_label_builder` option to use a custom label builder. + # + # Label Builder functions will receive a Rack env and a status code, and must + # return a hash with the labels for that request. They must also accept an empty + # env, and return a hash with the correct keys. This is necessary to initialize + # the metrics with the correct set of labels. class Collector attr_reader :app, :registry @@ -47,6 +52,8 @@ def call(env) # :nodoc: end COUNTER_LB = proc do |env, code| + next { code: nil, method: nil, path: nil } if env.empty? + { code: code, method: env['REQUEST_METHOD'].downcase, @@ -55,6 +62,8 @@ def call(env) # :nodoc: end DURATION_LB = proc do |env, _| + next { method: nil, path: nil } if env.empty? + { method: env['REQUEST_METHOD'].downcase, path: aggregation.call(env['PATH_INFO']), @@ -66,10 +75,12 @@ def init_request_metrics :"#{@metrics_prefix}_requests_total", docstring: 'The total number of HTTP requests handled by the Rack application.', + labels: @counter_lb.call({}, "").keys ) @durations = @registry.histogram( :"#{@metrics_prefix}_request_duration_seconds", docstring: 'The HTTP response duration of the Rack application.', + labels: @duration_lb.call({}, "").keys ) end @@ -77,6 +88,7 @@ def init_exception_metrics @exceptions = @registry.counter( :"#{@metrics_prefix}_exceptions_total", docstring: 'The total number of exceptions raised by the Rack application.', + labels: [:exception] ) end diff --git a/spec/examples/metric_example.rb b/spec/examples/metric_example.rb index 3b20f09e..ced2c317 100644 --- a/spec/examples/metric_example.rb +++ b/spec/examples/metric_example.rb @@ -14,7 +14,7 @@ expect do described_class.new(:foo, docstring: 'foo docstring', - base_labels: { __name__: 'reserved' }) + preset_labels: { __name__: 'reserved' }) end.to raise_exception exception end @@ -56,8 +56,12 @@ expect(subject.get).to be_a(type) end - it 'returns the current metric value for a given label set' do - expect(subject.get(labels: { test: 'label' })).to be_a(type) + context "with a subject that expects labels" do + subject { described_class.new(:foo, docstring: 'Labels', labels: [:test]) } + + it 'returns the current metric value for a given label set' do + expect(subject.get(labels: { test: 'label' })).to be_a(type) + end end end end diff --git a/spec/prometheus/client/counter_spec.rb b/spec/prometheus/client/counter_spec.rb index 8768ddb5..8e5a47d1 100644 --- a/spec/prometheus/client/counter_spec.rb +++ b/spec/prometheus/client/counter_spec.rb @@ -4,8 +4,12 @@ require 'examples/metric_example' describe Prometheus::Client::Counter do + let(:expected_labels) { [] } + let(:counter) do - Prometheus::Client::Counter.new(:foo, docstring: 'foo description') + Prometheus::Client::Counter.new(:foo, + docstring: 'foo description', + labels: expected_labels) end it_behaves_like Prometheus::Client::Metric do @@ -19,12 +23,22 @@ end.to change { counter.get }.by(1.0) end - it 'increments the counter for a given label set' do + it 'raises an InvalidLabelSetError if sending unexpected labels' do expect do + counter.increment(labels: { test: 'label' }) + end.to raise_error Prometheus::Client::LabelSetValidator::InvalidLabelSetError + end + + context "with a an expected label set" do + let(:expected_labels) { [:test] } + + it 'increments the counter for a given label set' do expect do - counter.increment(labels: { test: 'label' }) - end.to change { counter.get(labels: { test: 'label' }) }.by(1.0) - end.to_not change { counter.get } + expect do + counter.increment(labels: { test: 'label' }) + end.to change { counter.get(labels: { test: 'label' }) }.by(1.0) + end.to_not change { counter.get(labels: { test: 'other' }) } + end end it 'increments the counter by a given value' do diff --git a/spec/prometheus/client/formats/text_spec.rb b/spec/prometheus/client/formats/text_spec.rb index 60aa0c60..47493773 100644 --- a/spec/prometheus/client/formats/text_spec.rb +++ b/spec/prometheus/client/formats/text_spec.rb @@ -20,27 +20,24 @@ double( name: :foo, docstring: 'foo description', - base_labels: { umlauts: 'Björn', utf: '佖佥' }, type: :counter, values: { - { code: 'red' } => 42.0, - { code: 'green' } => 3.14E42, - { code: 'blue' } => -1.23e-45, + { umlauts: 'Björn', utf: '佖佥', code: 'red' } => 42.0, + { umlauts: 'Björn', utf: '佖佥', code: 'green' } => 3.14E42, + { umlauts: 'Björn', utf: '佖佥', code: 'blue' } => -1.23e-45, }, ), double( name: :bar, docstring: "bar description\nwith newline", - base_labels: { status: 'success' }, type: :gauge, values: { - { code: 'pink' } => 15.0, + { status: 'success', code: 'pink' } => 15.0, }, ), double( name: :baz, docstring: 'baz "description" \\escaping', - base_labels: {}, type: :counter, values: { { text: "with \"quotes\", \\escape \n and newline" } => 15.0, @@ -49,16 +46,14 @@ double( name: :qux, docstring: 'qux description', - base_labels: { for: 'sake' }, type: :summary, values: { - { code: '1' } => summary_value, + { for: 'sake', code: '1' } => summary_value, }, ), double( name: :xuq, docstring: 'xuq description', - base_labels: {}, type: :histogram, values: { { code: 'ah' } => histogram_value, diff --git a/spec/prometheus/client/gauge_spec.rb b/spec/prometheus/client/gauge_spec.rb index 20ebb7d5..14211c50 100644 --- a/spec/prometheus/client/gauge_spec.rb +++ b/spec/prometheus/client/gauge_spec.rb @@ -4,7 +4,13 @@ require 'examples/metric_example' describe Prometheus::Client::Gauge do - let(:gauge) { Prometheus::Client::Gauge.new(:foo, docstring: 'foo description') } + let(:expected_labels) { [] } + + let(:gauge) do + Prometheus::Client::Gauge.new(:foo, + docstring: 'foo description', + labels: expected_labels) + end it_behaves_like Prometheus::Client::Metric do let(:type) { NilClass } @@ -17,12 +23,22 @@ end.to change { gauge.get }.from(nil).to(42) end - it 'sets a metric value for a given label set' do + it 'raises an InvalidLabelSetError if sending unexpected labels' do expect do + gauge.set(42, labels: { test: 'value' }) + end.to raise_error Prometheus::Client::LabelSetValidator::InvalidLabelSetError + end + + context "with a an expected label set" do + let(:expected_labels) { [:test] } + + it 'sets a metric value for a given label set' do expect do - gauge.set(42, labels: { test: 'value' }) - end.to change { gauge.get(labels: { test: 'value' }) }.from(nil).to(42) - end.to_not change { gauge.get } + expect do + gauge.set(42, labels: { test: 'value' }) + end.to change { gauge.get(labels: { test: 'value' }) }.from(nil).to(42) + end.to_not change { gauge.get(labels: { test: 'other' }) } + end end context 'given an invalid value' do @@ -45,12 +61,22 @@ end.to change { gauge.get }.by(1.0) end - it 'increments the gauge for a given label set', labels: { test: 'one' } do + it 'raises an InvalidLabelSetError if sending unexpected labels' do expect do + gauge.increment(labels: { test: 'value' }) + end.to raise_error Prometheus::Client::LabelSetValidator::InvalidLabelSetError + end + + context "with a an expected label set" do + let(:expected_labels) { [:test] } + + it 'increments the gauge for a given label set', labels: { test: 'one' } do expect do - gauge.increment(labels: { test: 'one' }) - end.to change { gauge.get(labels: { test: 'one' }) }.by(1.0) - end.to_not change { gauge.get(labels: { test: 'another' }) } + expect do + gauge.increment(labels: { test: 'one' }) + end.to change { gauge.get(labels: { test: 'one' }) }.by(1.0) + end.to_not change { gauge.get(labels: { test: 'another' }) } + end end it 'increments the gauge by a given value' do @@ -79,18 +105,28 @@ gauge.set(0, labels: RSpec.current_example.metadata[:labels] || {}) end - it 'increments the gauge' do + it 'decrements the gauge' do expect do gauge.decrement end.to change { gauge.get }.by(-1.0) end - it 'decrements the gauge for a given label set', labels: { test: 'one' } do + it 'raises an InvalidLabelSetError if sending unexpected labels' do expect do + gauge.decrement(labels: { test: 'value' }) + end.to raise_error Prometheus::Client::LabelSetValidator::InvalidLabelSetError + end + + context "with a an expected label set" do + let(:expected_labels) { [:test] } + + it 'decrements the gauge for a given label set', labels: { test: 'one' } do expect do - gauge.decrement(labels: { test: 'one' }) - end.to change { gauge.get(labels: { test: 'one' }) }.by(-1.0) - end.to_not change { gauge.get(labels: { test: 'another' }) } + expect do + gauge.decrement(labels: { test: 'one' }) + end.to change { gauge.get(labels: { test: 'one' }) }.by(-1.0) + end.to_not change { gauge.get(labels: { test: 'another' }) } + end end it 'decrements the gauge by a given value' do diff --git a/spec/prometheus/client/histogram_spec.rb b/spec/prometheus/client/histogram_spec.rb index ed592abb..32c329ac 100644 --- a/spec/prometheus/client/histogram_spec.rb +++ b/spec/prometheus/client/histogram_spec.rb @@ -4,8 +4,13 @@ require 'examples/metric_example' describe Prometheus::Client::Histogram do + let(:expected_labels) { [] } + let(:histogram) do - described_class.new(:bar, docstring: 'bar description', buckets: [2.5, 5, 10]) + described_class.new(:bar, + docstring: 'bar description', + labels: expected_labels, + buckets: [2.5, 5, 10]) end it_behaves_like Prometheus::Client::Metric do @@ -18,6 +23,12 @@ described_class.new(:bar, docstring: 'bar description', buckets: [5, 2.5, 10]) end.to raise_error ArgumentError end + + it 'raise error for `le` label' do + expect do + described_class.new(:bar, docstring: 'bar description', labels: [:le]) + end.to raise_error Prometheus::Client::LabelSetValidator::ReservedLabelError + end end describe '#observe' do @@ -30,11 +41,31 @@ it 'raise error for le labels' do expect do histogram.observe(5, labels: { le: 1 }) - end.to raise_error ArgumentError + end.to raise_error Prometheus::Client::LabelSetValidator::ReservedLabelError + end + + it 'raises an InvalidLabelSetError if sending unexpected labels' do + expect do + histogram.observe(5, labels: { foo: 'bar' }) + end.to raise_error Prometheus::Client::LabelSetValidator::InvalidLabelSetError + end + + context "with a an expected label set" do + let(:expected_labels) { [:test] } + + it 'observes a value for a given label set' do + expect do + expect do + histogram.observe(5, labels: { test: 'value' }) + end.to change { histogram.get(labels: { test: 'value' }) } + end.to_not change { histogram.get(labels: { test: 'other' }) } + end end end describe '#get' do + let(:expected_labels) { [:foo] } + before do histogram.observe(3, labels: { foo: 'bar' }) histogram.observe(5.2, labels: { foo: 'bar' }) @@ -55,11 +86,13 @@ end it 'uses zero as default value' do - expect(histogram.get).to eql(2.5 => 0.0, 5 => 0.0, 10 => 0.0) + expect(histogram.get(labels: { foo: '' })).to eql(2.5 => 0.0, 5 => 0.0, 10 => 0.0) end end describe '#values' do + let(:expected_labels) { [:status] } + it 'returns a hash of all recorded summaries' do histogram.observe(3, labels: { status: 'bar' }) histogram.observe(6, labels: { status: 'foo' }) diff --git a/spec/prometheus/client/label_set_validator_spec.rb b/spec/prometheus/client/label_set_validator_spec.rb index f6071aaf..b6f959d5 100644 --- a/spec/prometheus/client/label_set_validator_spec.rb +++ b/spec/prometheus/client/label_set_validator_spec.rb @@ -3,7 +3,8 @@ require 'prometheus/client/label_set_validator' describe Prometheus::Client::LabelSetValidator do - let(:validator) { Prometheus::Client::LabelSetValidator.new } + let(:expected_labels) { [] } + let(:validator) { Prometheus::Client::LabelSetValidator.new(expected_labels: expected_labels) } let(:invalid) { Prometheus::Client::LabelSetValidator::InvalidLabelSetError } describe '.new' do @@ -45,25 +46,35 @@ end describe '#validate' do + let(:expected_labels) { [:method, :code] } + it 'returns a given valid label set' do - hash = { version: 'alpha' } + hash = { method: 'get', code: '200' } expect(validator.validate(hash)).to eql(hash) end - it 'raises an exception if a given label set is not valid' do + it 'raises an exception if a given label set is not `valid?`' do input = 'broken' expect(validator).to receive(:valid?).with(input).and_raise(invalid) expect { validator.validate(input) }.to raise_exception(invalid) end - it 'raises InvalidLabelSetError for varying label sets' do - validator.validate(method: 'get', code: '200') + it 'raises an exception if there are unexpected labels' do + expect do + validator.validate(method: 'get', code: '200', exception: 'NoMethodError') + end.to raise_exception(invalid, /keys given: \[:code, :exception, :method\] vs. keys expected: \[:code, :method\]/) + end + + it 'raises an exception if there are missing labels' do + expect do + validator.validate(method: 'get') + end.to raise_exception(invalid, /keys given: \[:method\] vs. keys expected: \[:code, :method\]/) expect do - validator.validate(method: 'get', exception: 'NoMethodError') - end.to raise_exception(invalid, /keys given: \[:exception, :method\] vs. keys expected: \[:code, :method\]/) + validator.validate(code: '200') + end.to raise_exception(invalid, /keys given: \[:code\] vs. keys expected: \[:code, :method\]/) end end end diff --git a/spec/prometheus/client/summary_spec.rb b/spec/prometheus/client/summary_spec.rb index bae3327d..7b702a6c 100644 --- a/spec/prometheus/client/summary_spec.rb +++ b/spec/prometheus/client/summary_spec.rb @@ -4,24 +4,60 @@ require 'examples/metric_example' describe Prometheus::Client::Summary do + let(:expected_labels) { [] } + let(:summary) do Prometheus::Client::Summary.new(:bar, - docstring: 'bar description') + docstring: 'bar description', + labels: expected_labels) end it_behaves_like Prometheus::Client::Metric do let(:type) { Hash } end + describe '#initialization' do + it 'raise error for `quantile` label' do + expect do + described_class.new(:bar, docstring: 'bar description', labels: [:quantile]) + end.to raise_error Prometheus::Client::LabelSetValidator::ReservedLabelError + end + end + describe '#observe' do it 'records the given value' do expect do summary.observe(5) end.to change { summary.get } end + it 'raise error for quantile labels' do + expect do + summary.observe(5, labels: { quantile: 1 }) + end.to raise_error Prometheus::Client::LabelSetValidator::ReservedLabelError + end + + it 'raises an InvalidLabelSetError if sending unexpected labels' do + expect do + summary.observe(5, labels: { foo: 'bar' }) + end.to raise_error Prometheus::Client::LabelSetValidator::InvalidLabelSetError + end + + context "with a an expected label set" do + let(:expected_labels) { [:test] } + + it 'observes a value for a given label set' do + expect do + expect do + summary.observe(5, labels: { test: 'value' }) + end.to change { summary.get(labels: { test: 'value' }) } + end.to_not change { summary.get(labels: { test: 'other' }) } + end + end end describe '#get' do + let(:expected_labels) { [:foo] } + before do summary.observe(3, labels: { foo: 'bar' }) summary.observe(5.2, labels: { foo: 'bar' }) @@ -47,6 +83,8 @@ end describe '#values' do + let(:expected_labels) { [:status] } + it 'returns a hash of all recorded summaries' do summary.observe(3, labels: { status: 'bar' }) summary.observe(5, labels: { status: 'foo' }) diff --git a/spec/prometheus/middleware/collector_spec.rb b/spec/prometheus/middleware/collector_spec.rb index 27b18ea3..bbebd8fb 100644 --- a/spec/prometheus/middleware/collector_spec.rb +++ b/spec/prometheus/middleware/collector_spec.rb @@ -104,6 +104,8 @@ original_app, registry: registry, counter_label_builder: lambda do |env, code| + next { code: nil, method: nil } if env.empty? + { code: code, method: env['REQUEST_METHOD'].downcase, From ce6d44dea95f8af7f4d772c68cd2775f6668ec2d Mon Sep 17 00:00:00 2001 From: Daniel Magliola Date: Thu, 13 Sep 2018 13:43:19 +0100 Subject: [PATCH 05/18] Remove quantile calculation from `Summary` Expose only `sum` and `count` instead, with no quantiles / percentiles. The main reason to do this is that all this change is based on the idea of having a "Store" interface that can be shared between Ruby processes. The `quantile` gem doesn't play well with this, since we'd need to store instances of `Quantile::Estimator`, which is a complex data structure, and tricky to share between Ruby processes. Moreover, individual Summaries on different processes cannot be aggregated, so all processes would actually have to share one instance of this class, which makes it extremely tricky, particularly to do performantly. Even though this is a loss of functionality, this puts the Ruby client on par with other client libraries, like the Python one, which also only offers sum and count without quantiles. Also, this is actually more compliant with the Client Library best practices: - Summary is ENCOURAGED to offer quantiles as exports - MUST allow not having quantiles, since just count and sum is quite useful - This MUST be the default The original client didn't comply with the last 2 rules, where this one does, just like the Python client. And quantiles, while seemingly the point of summaries, are encouraged but not required. I'm not discarding the idea of adding quantiles back. I have ideas on how this could be done, but it'd probably be pretty expensive, as a single Estimator instance would have to be marshalled / unmarshalled into bytes for every call to `observe`, and it'd have a hard requirement of having some sort of low-cardinality Process ID as a label (possible with Unicorn) to avoid aggregation. Signed-off-by: Daniel Magliola --- README.md | 9 +++-- lib/prometheus/client/formats/text.rb | 4 --- lib/prometheus/client/summary.rb | 38 ++++++--------------- prometheus-client.gemspec | 2 -- spec/prometheus/client/formats/text_spec.rb | 9 ++--- spec/prometheus/client/summary_spec.rb | 30 ++++++---------- 6 files changed, 29 insertions(+), 63 deletions(-) diff --git a/README.md b/README.md index 6f37040e..044ce55c 100644 --- a/README.md +++ b/README.md @@ -158,15 +158,18 @@ histogram.get(labels: { service: 'users' }) Summary, similar to histograms, is an accumulator for samples. It captures Numeric data and provides an efficient percentile calculation mechanism. +For now, only `sum` and `total` (count of observations) are supported, no actual quantiles. + ```ruby summary = Prometheus::Client::Summary.new(:service_latency_seconds, docstring: '...', labels: [:service]) # record a value summary.observe(Benchmark.realtime { service.call() }, labels: { service: 'database' }) -# retrieve the current quantile values -summary.get(labels: { service: 'database' }) -# => { 0.5 => 0.1233122, 0.9 => 3.4323, 0.99 => 5.3428231 } +# retrieve the current sum and total values +summary_value = summary.get(labels: { service: 'database' }) +summary_value.sum # => 123.45 +summary_value.count # => 100 ``` ## Labels diff --git a/lib/prometheus/client/formats/text.rb b/lib/prometheus/client/formats/text.rb index a4ea75f8..9db12542 100644 --- a/lib/prometheus/client/formats/text.rb +++ b/lib/prometheus/client/formats/text.rb @@ -50,10 +50,6 @@ def representation(metric, label_set, value, &block) end def summary(name, set, value) - value.each do |q, v| - yield metric(name, labels(set.merge(quantile: q)), v) - end - l = labels(set) yield metric("#{name}_sum", l, value.sum) yield metric("#{name}_count", l, value.total) diff --git a/lib/prometheus/client/summary.rb b/lib/prometheus/client/summary.rb index 455dee71..066ab0e8 100644 --- a/lib/prometheus/client/summary.rb +++ b/lib/prometheus/client/summary.rb @@ -1,24 +1,24 @@ # encoding: UTF-8 -require 'quantile' require 'prometheus/client/metric' module Prometheus module Client # Summary is an accumulator for samples. It captures Numeric data and - # provides an efficient quantile calculation mechanism. + # provides the total count and sum of observations. class Summary < Metric # Value represents the state of a Summary at a given point. - class Value < Hash + class Value attr_accessor :sum, :total - def initialize(estimator:) - @sum = estimator.sum - @total = estimator.observations + def initialize + @sum = 0.0 + @total = 0.0 + end - estimator.invariants.each do |invariant| - self[invariant.quantile] = estimator.query(invariant.quantile) - end + def observe(value) + @sum += value + @total += 1 end end @@ -32,24 +32,6 @@ def observe(value, labels: {}) synchronize { @values[label_set].observe(value) } end - # Returns the value for the given label set - def get(labels: {}) - @validator.valid?(labels) - - synchronize do - Value.new(estimator: @values[labels]) - end - end - - # Returns all label sets with their values - def values - synchronize do - @values.each_with_object({}) do |(labels, value), memo| - memo[labels] = Value.new(estimator: value) - end - end - end - private def reserved_labels @@ -57,7 +39,7 @@ def reserved_labels end def default - Quantile::Estimator.new + Value.new end end end diff --git a/prometheus-client.gemspec b/prometheus-client.gemspec index c9277dc2..0d5c9659 100644 --- a/prometheus-client.gemspec +++ b/prometheus-client.gemspec @@ -14,6 +14,4 @@ Gem::Specification.new do |s| s.files = %w(README.md) + Dir.glob('{lib/**/*}') s.require_paths = ['lib'] - - s.add_dependency 'quantile', '~> 0.2.1' end diff --git a/spec/prometheus/client/formats/text_spec.rb b/spec/prometheus/client/formats/text_spec.rb index 47493773..0150fdf9 100644 --- a/spec/prometheus/client/formats/text_spec.rb +++ b/spec/prometheus/client/formats/text_spec.rb @@ -4,9 +4,7 @@ describe Prometheus::Client::Formats::Text do let(:summary_value) do - { 0.5 => 4.2, 0.9 => 8.32, 0.99 => 15.3 }.tap do |value| - allow(value).to receive_messages(sum: 1243.21, total: 93) - end + Struct.new(:sum, :total).new(1243.21, 93.0) end let(:histogram_value) do @@ -79,11 +77,8 @@ baz{text="with \"quotes\", \\escape \n and newline"} 15.0 # TYPE qux summary # HELP qux qux description -qux{for="sake",code="1",quantile="0.5"} 4.2 -qux{for="sake",code="1",quantile="0.9"} 8.32 -qux{for="sake",code="1",quantile="0.99"} 15.3 qux_sum{for="sake",code="1"} 1243.21 -qux_count{for="sake",code="1"} 93 +qux_count{for="sake",code="1"} 93.0 # TYPE xuq histogram # HELP xuq xuq description xuq_bucket{code="ah",le="10"} 1.0 diff --git a/spec/prometheus/client/summary_spec.rb b/spec/prometheus/client/summary_spec.rb index 7b702a6c..740a4d23 100644 --- a/spec/prometheus/client/summary_spec.rb +++ b/spec/prometheus/client/summary_spec.rb @@ -13,7 +13,7 @@ end it_behaves_like Prometheus::Client::Metric do - let(:type) { Hash } + let(:type) { Prometheus::Client::Summary::Value } end describe '#initialization' do @@ -27,9 +27,12 @@ describe '#observe' do it 'records the given value' do expect do - summary.observe(5) - end.to change { summary.get } + expect do + summary.observe(5) + end.to change { summary.get.sum }.from(0.0).to(5.0) + end.to change { summary.get.total }.from(0.0).to(1.0) end + it 'raise error for quantile labels' do expect do summary.observe(5, labels: { quantile: 1 }) @@ -49,8 +52,8 @@ expect do expect do summary.observe(5, labels: { test: 'value' }) - end.to change { summary.get(labels: { test: 'value' }) } - end.to_not change { summary.get(labels: { test: 'other' }) } + end.to change { summary.get(labels: { test: 'value' }).total } + end.to_not change { summary.get(labels: { test: 'other' }).total } end end end @@ -65,20 +68,11 @@ summary.observe(4, labels: { foo: 'bar' }) end - it 'returns a set of quantile values' do - expect(summary.get(labels: { foo: 'bar' })) - .to eql(0.5 => 4, 0.9 => 5.2, 0.99 => 5.2) - end - it 'returns a value which responds to #sum and #total' do value = summary.get(labels: { foo: 'bar' }) expect(value.sum).to eql(25.2) - expect(value.total).to eql(4) - end - - it 'uses nil as default value' do - expect(summary.get).to eql(0.5 => nil, 0.9 => nil, 0.99 => nil) + expect(value.total).to eql(4.0) end end @@ -89,10 +83,8 @@ summary.observe(3, labels: { status: 'bar' }) summary.observe(5, labels: { status: 'foo' }) - expect(summary.values).to eql( - { status: 'bar' } => { 0.5 => 3, 0.9 => 3, 0.99 => 3 }, - { status: 'foo' } => { 0.5 => 5, 0.9 => 5, 0.99 => 5 }, - ) + expect(summary.values[{ status: 'bar' }].sum).to eql(3.0) + expect(summary.values[{ status: 'foo' }].sum).to eql(5.0) end end end From b787e37f3e93cd0464fd77280526bd8c8856327a Mon Sep 17 00:00:00 2001 From: Daniel Magliola Date: Fri, 14 Sep 2018 12:08:42 +0100 Subject: [PATCH 06/18] Add `concurrent-ruby` gem The reason we need this gem is to be able to provide a good concurrency story in our "Data Stores". We want the data stores to be thread-safe to make it easy for metrics to use them, and to shield away the complexity so that the store can do what's most efficient, and metrics don't need to make assumptions on how it works. However, we also need metrics to be able to update multiple values at once so in some cases (most notably, histograms), the store does need to provide a synchronization method. Since the normal `set` / `increment` methods call a Mutex already, having a Mutex block around them means we end up calling `Mutex.synchronize` twice, which *should* work, but Ruby Mutexes are not reentrant. `concurrent-ruby` provides a Reentrant lock. It's a Read-Write Lock, not a Mutex, but by always grabbing Write locks, it's functionally the same Signed-off-by: Daniel Magliola --- prometheus-client.gemspec | 2 ++ 1 file changed, 2 insertions(+) diff --git a/prometheus-client.gemspec b/prometheus-client.gemspec index 0d5c9659..9642a7d0 100644 --- a/prometheus-client.gemspec +++ b/prometheus-client.gemspec @@ -14,4 +14,6 @@ Gem::Specification.new do |s| s.files = %w(README.md) + Dir.glob('{lib/**/*}') s.require_paths = ['lib'] + + s.add_dependency 'concurrent-ruby' end From a951c225151914fd469b4a01ffabe856b8f842a6 Mon Sep 17 00:00:00 2001 From: Daniel Magliola Date: Fri, 14 Sep 2018 16:21:26 +0100 Subject: [PATCH 07/18] Abstract away storage of Metric data into DataStore objects Currently, each Metric object stores its data (labels and values) in a Hash object internal to itself. This change introduces the concept of a Data Store that is external to the metrics themselves, which holds all the data. The main reason to do this is that by having a standardized and simple interface that metrics use to access the store, we abstract away the details of storing the data from the specific needs of each metric. This allows us to then simply swap around the stores based on the needs of different circumstances, with no changes to the rest of the client. The main use case for this is solving the "pre-fork" server scenario, where multiple processes need to share the metric storage, to be able to report coherent numbers to Prometheus. This change provides one store, a Synchronized hash, that pretty much has the same characteristics that the existing Storage had, plus some Mutex overhead. Within a single process scenario, this should be the fastest way to operate in a multi-threaded environment, and the safest. As such, this is the default store. Future commits will introduce new ones, for specific scenarios, and also each consumer of the Prometheus Client can make their own and simply swap them for the built-in ones, if they have specific needs. The interface and requirements of Stores are specified in detail in a README.md file in the `client/data_stores` directory. This is the documentation that must be used by anyone wishing to create their own store. Signed-off-by: Daniel Magliola --- README.md | 124 ++++++++ lib/prometheus/client.rb | 5 + lib/prometheus/client/config.rb | 15 + lib/prometheus/client/counter.rb | 8 +- lib/prometheus/client/data_stores/README.md | 285 ++++++++++++++++++ .../client/data_stores/synchronized.rb | 64 ++++ lib/prometheus/client/formats/text.rb | 10 +- lib/prometheus/client/gauge.rb | 12 +- lib/prometheus/client/histogram.rb | 75 +++-- lib/prometheus/client/metric.rb | 34 +-- lib/prometheus/client/registry.rb | 21 +- lib/prometheus/client/summary.rb | 49 +-- spec/examples/data_store_example.rb | 58 ++++ spec/prometheus/client/counter_spec.rb | 6 + .../client/data_stores/synchronized_spec.rb | 19 ++ spec/prometheus/client/formats/text_spec.rb | 6 +- spec/prometheus/client/gauge_spec.rb | 12 +- spec/prometheus/client/histogram_spec.rb | 24 +- spec/prometheus/client/summary_spec.rb | 32 +- spec/prometheus/middleware/collector_spec.rb | 5 + 20 files changed, 737 insertions(+), 127 deletions(-) create mode 100644 lib/prometheus/client/config.rb create mode 100644 lib/prometheus/client/data_stores/README.md create mode 100644 lib/prometheus/client/data_stores/synchronized.rb create mode 100644 spec/examples/data_store_example.rb create mode 100644 spec/prometheus/client/data_stores/synchronized_spec.rb diff --git a/README.md b/README.md index 044ce55c..b3f2e167 100644 --- a/README.md +++ b/README.md @@ -216,6 +216,130 @@ https_requests_total.increment(labels: { status_code: response.status_code }) + +## Data Stores + +The data for all the metrics (the internal counters associated with each labelset) +is stored in a global Data Store object, rather than in the metric objects themselves. +(This "storage" is ephemeral, generally in-memory, it's not "long-term storage") + +The main reason to do this is that different applications may have different requirements +for their metrics storage. Application running in pre-fork servers (like Unicorn, for +example), require a shared store between all the processes, to be able to report coherent +numbers. At the same time, other applications may not have this requirement but be very +sensitive to performance, and would prefer instead a simpler, faster store. + +By having a standardized and simple interface that metrics use to access this store, +we abstract away the details of storing the data from the specific needs of each metric. +This allows us to then simply swap around the stores based on the needs of different +applications, with no changes to the rest of the client. + +The client provides 3 built-in stores, but if neither of these is ideal for your +requirements, you can easily make your own store and use that instead. More on this below. + +### Configuring which store to use. + +By default, the Client uses the `Synchronized` store, which is a simple, thread-safe Store +for single-process scenarios. + +If you need to use a different store, set it in the Client Config: + +```ruby +Prometheus::Client.config.data_store = Prometheus::Client::DataStores::DataStore.new(store_specific_params) +``` + +NOTE: You **must** make sure to set the `data_store` before initializing any metrics. +If using Rails, you probably want to set up your Data Store on `config/application.rb`, +or `config/environments/*`, both of which run before `config/initializers/*` + +Also note that `config.data_store` is set to an *instance* of a `DataStore`, not to the +class. This is so that the stores can receive parameters. Most of the built-in stores +don't require any, but `DirectFileStore` does, for example. + +When instantiating metrics, there is an optional `store_settings` attribute. This is used +to set up store-specific settings for each metric. For most stores, this is not used, but +for multi-process stores, this is used to specify how to aggregate the values of each +metric across multiple processes. For the most part, this is used for Gauges, to specify +whether you want to report the `SUM`, `MAX` or `MIN` value observed across all processes. +For almost all other cases, you'd leave the default (`SUM`). More on this on the +*Aggregation* section below. + +Other custom stores may also require or accept extra parameters besides `:aggregation`. +See the documentation of each store for more details. + +### Built-in stores + +There are 3 built-in stores, with different trade-offs: + +- **Synchronized**: Default store. Thread safe, but not suitable for multi-process + scenarios (e.g. pre-fork servers, like Unicorn). Stores data in Hashes, with all accesses + protected by Mutexes. +- **SingleThreaded**: Fastest store, but only suitable for single-threaded scenarios. + This store does not make any effort to synchronize access to its internal hashes, so + it's absolutely not thread safe. +- **DirectFileStore**: Stores data in binary files, one file per process and per metric. + This is generally the recommended store to use with pre-fork servers and other + "multi-process" scenarios. + + Each metric gets a file for each process, and manages its contents by storing keys and + binary floats next to them, and updating the offsets of those Floats directly. When + exporting metrics, it will find all the files that apply to each metric, read them, + and aggregate them. + + In order to do this, each Metric needs an `:aggregation` setting, specifying how + to aggregate the multiple possible values we can get for each labelset. By default, + they are `SUM`med, which is what most use-cases call for (counters and histograms, + for example). However, for Gauges, it's possible to set `MAX` or `MIN` as aggregation, + to get the highest/lowest value of all the processes / threads. + + Even though this store saves data on disk, it's still much faster than would probably be + expected, because the files are never actually `fsync`ed, so the store never blocks + while waiting for disk. FS caching is incredibly efficient in this regard. + + If in doubt, check the benchmark scripts described in the documentation for creating + your own stores and run them in your particular runtime environment to make sure this + provides adequate performance. + +### Building your own store, and stores other than the built-in ones. + +If none of these stores is suitable for your requirements, you can easily make your own. + +The interface and requirements of Stores are specified in detail in the `README.md` +in the `client/data_stores` directory. This thoroughly documents how to make your own +store. + +There are also links there to non-built-in stores created by others that may be useful, +either as they are, or as a starting point for making your own. + +### Aggregation settings for multi-process stores + +If you are in a multi-process environment (such as pre-fork servers like Unicorn), each +process will probably keep their own counters, which need to be aggregated when receiving +a Prometheus scrape, to report coherent total numbers. + +For Counters and Histograms (and quantile-less Summaries), this is simply a matter of +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. + +In those cases, you should use the `store_settings` parameter when registering the +metric, to specify an `:aggregation` setting. + +```ruby +free_disk_space = registry.gauge(:free_disk_space_bytes, + docstring: "Free disk space, in bytes", + store_settings: { aggregation: :max }) +``` + +NOTE: This will only work if the store you're using supports the `:aggregation` setting. +Of the built-in stores, only `DirectFileStore` does. + +Also note that the `:aggregation` setting works for all metric types, not just for gauges. +It would be unusual to use it for anything other than gauges, but if your use-case +requires it, the store will respect your aggregation wishes. + ## Tests Install necessary development gems with `bundle install` and run tests with diff --git a/lib/prometheus/client.rb b/lib/prometheus/client.rb index a7e9acaa..fe09d9ba 100644 --- a/lib/prometheus/client.rb +++ b/lib/prometheus/client.rb @@ -1,6 +1,7 @@ # encoding: UTF-8 require 'prometheus/client/registry' +require 'prometheus/client/config' module Prometheus # Client is a ruby implementation for a Prometheus compatible client. @@ -9,5 +10,9 @@ module Client def self.registry @registry ||= Registry.new end + + def self.config + @config ||= Config.new + end end end diff --git a/lib/prometheus/client/config.rb b/lib/prometheus/client/config.rb new file mode 100644 index 00000000..7f76c2a0 --- /dev/null +++ b/lib/prometheus/client/config.rb @@ -0,0 +1,15 @@ +# encoding: UTF-8 + +require 'prometheus/client/data_stores/synchronized' + +module Prometheus + module Client + class Config + attr_accessor :data_store + + def initialize + @data_store = Prometheus::Client::DataStores::Synchronized.new + end + end + end +end diff --git a/lib/prometheus/client/counter.rb b/lib/prometheus/client/counter.rb index 4ed7fc45..28ec2f1e 100644 --- a/lib/prometheus/client/counter.rb +++ b/lib/prometheus/client/counter.rb @@ -14,13 +14,7 @@ def increment(by: 1, labels: {}) raise ArgumentError, 'increment must be a non-negative number' if by < 0 label_set = label_set_for(labels) - synchronize { @values[label_set] += by } - end - - private - - def default - 0.0 + @store.increment(labels: label_set, by: by) end end end diff --git a/lib/prometheus/client/data_stores/README.md b/lib/prometheus/client/data_stores/README.md new file mode 100644 index 00000000..e4243839 --- /dev/null +++ b/lib/prometheus/client/data_stores/README.md @@ -0,0 +1,285 @@ +# Custom Data Stores + +Stores are basically an abstraction over a Hash, whose keys are in turn a Hash of labels +plus a metric name. The intention behind having different data stores is solving +different requirements for different production scenarios, or performance trade-offs. + +The most common of these scenarios are pre-fork servers like Unicorn, which have multiple +separate processes gathering metrics. If each of these had their own store, the metrics +reported on each Prometheus scrape would be different, depending on which process handles +the request. Solving this requires some sort of shared storage between these processes, +and there are many ways to solve this problem, each with their own trade-offs. + +This abstraction allows us to easily plug in the most adequate store for each scenario. + +## Interface + +`Store` exposes a `for_metric` method, which returns a store-specific and metric-specific +`MetricStore` object, which represents a "view" onto the actual internal storage for one +particular metric. Each metric / collector object will have a references to this +`MetricStore` and interact with it directly. + +The `MetricStore` class must expose `synchronize`, `set`, `increment`, `get` and `all_values` +methods, which are explained in the code sample below. Its initializer should be called +only by `Store#for_metric`, not directly. + +All values stored are `Float`s. + +Internally, a `Store` can store the data however it needs to, based on its requirements. +For example, a store that needs to work in a multi-process environment needs to have a +shared section of memory, via either Files, an MMap, an external database, or whatever the +implementor chooses for their particular use case. + +Each `Store` / `MetricStore` will also choose how to divide responsibilities over the +storage of values. For some use cases, each `MetricStore` may have their own individual +storage, whereas for others, the `Store` may own a central storage, and `MetricStore` +objects will access it through the `Store`. This depends on the design choices of each `Store`. + +`Store` and `MetricStore` MUST be thread safe. This applies not only to operations on +stored values (`set`, `increment`), but `MetricStore` must also expose a `synchronize` +method that would allow a Metric to increment multiple values atomically (Histograms need +this, for example). + +Ideally, multiple keys should be modifiable simultaneously, but this is not a +hard requirement. + +This is what the interface looks like, in practice: + +```ruby +module Prometheus + module Client + module DataStores + class CustomStore + + # Return a MetricStore, which provides a view of the internal data store, + # catering specifically to that metric. + # + # - `metric_settings` specifies configuration parameters for this metric + # specifically. These may or may not be necessary, depending on each specific + # store and metric. The most obvious example of this is for gauges in + # multi-process environments, where the developer needs to choose how those + # gauges will get aggregated between all the per-process values. + # + # The settings that the store will accept, and what it will do with them, are + # 100% Store-specific. Each store should document what settings it will accept + # and how to use them, so the developer using that store can pass the appropriate + # instantiating the Store itself, and the Metrics they declare. + # + # - `metric_type` is specified in case a store wants to validate that the settings + # are valid for the metric being set up. It may go unused by most Stores + # + # Even if your store doesn't need these two parameters, the Store must expose them + # to make them swappable. + def for_metric(metric_name, metric_type:, metric_settings: {}) + # Generally, here a Store would validate that the settings passed in are valid, + # and raise if they aren't. + validate_metric_settings(metric_type: metric_type, + metric_settings: metric_settings) + MetricStore.new(store: self, + metric_name: metric_name, + metric_type: metric_type, + metric_settings: metric_settings) + end + + + # MetricStore manages the data for one specific metric. It's generally a view onto + # the central store shared by all metrics, but it could also hold the data itself + # if that's better for the specific scenario + class MetricStore + # This constructor is internal to this store, so the signature doesn't need to + # be this. No one other than the Store should be creating MetricStores + def initialize(store:, metric_name:, metric_type:, metric_settings:) + end + + # Metrics may need to modify multiple values at once (Histograms do this, for + # example). MetricStore needs to provide a way to synchronize those, in addition + # to all of the value modifications being thread-safe without a need for simple + # Metrics to call `synchronize` + def synchronize + raise NotImplementedError + end + + # Store a value for this metric and a set of labels + # Internally, may add extra "labels" to disambiguate values between, + # for example, different processes + def set(labels:, val:) + raise NotImplementedError + end + + def increment(labels:, by: 1) + raise NotImplementedError + end + + # Return a value for a set of labels + # Will return the same value stored by `set`, as opposed to `all_values`, which + # may aggregate multiple values. + # + # For example, in a multi-process scenario, `set` may add an extra internal + # label tagging the value with the process id. `get` will return the value for + # "this" process ID. `all_values` will return an aggregated value for all + # process IDs. + def get(labels:) + raise NotImplementedError + end + + # Returns all the sets of labels seen by the Store, and the aggregated value for + # each. + # + # In some cases, this is just a matter of returning the stored value. + # + # In other cases, the store may need to aggregate multiple values for the same + # set of labels. For example, in a multiple process it may need to `sum` the + # values of counters from each process. Or for `gauges`, it may need to take the + # `max`. This is generally specified in `metric_settings` when calling + # `Store#for_metric`. + def all_values + raise NotImplementedError + end + end + end + end + end +end +``` + +## Conventions + +- Your store MAY require or accept extra settings for each metric on the call to `for_metric`. +- You SHOULD validate these parameters to make sure they are correct, and raise if they aren't. +- If your store needs to aggregate multiple values for the same metric (for example, in + a multi-process scenario), you MUST accept a setting to define how values are aggregated. + - This setting MUST be called `:aggregation` + - It MUST support, at least, `:sum`, `:max` and `:min`. + - It MAY support other aggregation modes that may apply to your requirements. + - It MUST default to `:sum` + +## Testing your Store + +In order to make it easier to test your store, the basic functionality is tested using +`shared_examples`: + +`it_behaves_like Prometheus::Client::DataStores` + +Follow the simple structure in `synchronized_spec.rb` for a starting point. + +Note that if your store stores data somewhere other than in-memory (in files, Redis, +databases, etc), you will need to do cleanup between tests in a `before` block. + +The tests for `DirectFileStore` have a good example at the top of the file. This file also +has some examples on testing multi-process stores, checking that aggregation between +processes works correctly. + +## Sample, imaginary multi-process Data Store + +This is just an example of how one could implement a data store, and a clarification on +the "aggregation" point + +Important: This is **VAPORWARE**, intended simply to show how this could work / how to +implement these interfaces. + +There are some key pieces of code missing, which are fairly uninteresting, this only shows +the parts that illustrate the idea of storing multiple different values, and aggregate +them + +```ruby +module Prometheus + module Client + module DataStores + # Stores all the data in a magic data structure that keeps cross-process data, in a + # way that all processes can read it, but each can write only to their own set of + # keys. + # It doesn't care how that works, this is not an actual solution to anything, + # just an example of how the interface would work with something like that. + # + # Metric Settings have one possible key, `aggregation`, which must be one of + # `AGGREGATION_MODES` + class SampleMagicMultiprocessStore + AGGREGATION_MODES = [MAX = :max, MIN = :min, SUM = :sum] + DEFAULT_METRIC_SETTINGS = { aggregation: SUM } + + def initialize + @internal_store = MagicHashSharedBetweenProcesses.new # PStore, for example + end + + def for_metric(metric_name, metric_type:, metric_settings: {}) + settings = DEFAULT_METRIC_SETTINGS.merge(metric_settings) + validate_metric_settings(metric_settings: settings) + MetricStore.new(store: self, + metric_name: metric_name, + metric_type: metric_type, + metric_settings: settings) + end + + private + + def validate_metric_settings(metric_settings:) + raise unless metric_settings.has_key?(:aggregation) + raise unless metric_settings[:aggregation].in?(AGGREGATION_MODES) + end + + class MetricStore + def initialize(store:, metric_name:, metric_type:, metric_settings:) + @store = store + @internal_store = store.internal_store + @metric_name = metric_name + @aggregation_mode = metric_settings[:aggregation] + end + + def set(labels:, val:) + @internal_store[store_key(labels)] = val.to_f + end + + def get(labels:) + @internal_store[store_key(labels)] + end + + def all_values + non_aggregated_values = all_store_values.each_with_object({}) do |(labels, v), acc| + if labels["__metric_name"] == @metric_name + label_set = labels.reject { |k,_| k.in?("__metric_name", "__pid") } + acc[label_set] ||= [] + acc[label_set] << v + end + end + + # Aggregate all the different values for each label_set + non_aggregated_values.each_with_object({}) do |(label_set, values), acc| + acc[label_set] = aggregate(values) + end + end + + private + + def all_store_values + # This assumes there's a something common that all processes can write to, and + # it's magically synchronized (which is not true of a PStore, for example, but + # would of some sort of external data store like Redis, Memcached, SQLite) + + # This could also have some sort of: + # file_list = Dir.glob(File.join(path, '*.db')).sort + # which reads all the PStore files / MMapped files, etc, and returns a hash + # with all of them together, which then `values` and `label_sets` can use + end + + # This method holds most of the key to how this Store works. Adding `_pid` as + # one of the labels, we hold each process's value separately, which we can + # aggregate later + def store_key(labels) + labels.merge( + { + "__metric_name" => @metric_name, + "__pid" => Process.pid + } + ) + end + + def aggregate(values) + # This is a horrible way to do this, just illustrating the point + values.send(@aggregation_mode) + end + end + end + end + end +end +``` diff --git a/lib/prometheus/client/data_stores/synchronized.rb b/lib/prometheus/client/data_stores/synchronized.rb new file mode 100644 index 00000000..17e2b715 --- /dev/null +++ b/lib/prometheus/client/data_stores/synchronized.rb @@ -0,0 +1,64 @@ +require 'concurrent' + +module Prometheus + module Client + module DataStores + # Stores all the data in simple hashes, one per metric. Each of these metrics + # synchronizes access to their hash, but multiple metrics can run observations + # concurrently. + class Synchronized + class InvalidStoreSettingsError < StandardError; end + + def for_metric(metric_name, metric_type:, metric_settings: {}) + # We don't need `metric_type` or `metric_settings` for this particular store + validate_metric_settings(metric_settings: metric_settings) + MetricStore.new + end + + private + + def validate_metric_settings(metric_settings:) + unless metric_settings.empty? + raise InvalidStoreSettingsError, + "Synchronized doesn't allow any metric_settings" + end + end + + class MetricStore + def initialize + @internal_store = Hash.new { |hash, key| hash[key] = 0.0 } + @rwlock = Concurrent::ReentrantReadWriteLock.new + end + + def synchronize + @rwlock.with_write_lock { yield } + end + + def set(labels:, val:) + synchronize do + @internal_store[labels] = val.to_f + end + end + + def increment(labels:, by: 1) + synchronize do + @internal_store[labels] += by + end + end + + def get(labels:) + synchronize do + @internal_store[labels] + end + end + + def all_values + synchronize { @internal_store.dup } + end + end + + private_constant :MetricStore + end + end + end +end diff --git a/lib/prometheus/client/formats/text.rb b/lib/prometheus/client/formats/text.rb index 9db12542..b735389c 100644 --- a/lib/prometheus/client/formats/text.rb +++ b/lib/prometheus/client/formats/text.rb @@ -51,20 +51,20 @@ def representation(metric, label_set, value, &block) def summary(name, set, value) l = labels(set) - yield metric("#{name}_sum", l, value.sum) - yield metric("#{name}_count", l, value.total) + yield metric("#{name}_sum", l, value["sum"]) + yield metric("#{name}_count", l, value["count"]) end def histogram(name, set, value) bucket = "#{name}_bucket" value.each do |q, v| + next if q == "sum" yield metric(bucket, labels(set.merge(le: q)), v) end - yield metric(bucket, labels(set.merge(le: '+Inf')), value.total) l = labels(set) - yield metric("#{name}_sum", l, value.sum) - yield metric("#{name}_count", l, value.total) + yield metric("#{name}_sum", l, value["sum"]) + yield metric("#{name}_count", l, value["+Inf"]) end def metric(name, labels, value) diff --git a/lib/prometheus/client/gauge.rb b/lib/prometheus/client/gauge.rb index 1f24eb78..e0f76521 100644 --- a/lib/prometheus/client/gauge.rb +++ b/lib/prometheus/client/gauge.rb @@ -17,27 +17,21 @@ def set(value, labels: {}) raise ArgumentError, 'value must be a number' end - @values[label_set_for(labels)] = value.to_f + @store.set(labels: label_set_for(labels), val: value) end # Increments Gauge value by 1 or adds the given value to the Gauge. # (The value can be negative, resulting in a decrease of the Gauge.) def increment(by: 1, labels: {}) label_set = label_set_for(labels) - synchronize do - @values[label_set] ||= 0 - @values[label_set] += by - end + @store.increment(labels: label_set, by: by) end # Decrements Gauge value by 1 or subtracts the given value from the Gauge. # (The value can be negative, resulting in a increase of the Gauge.) def decrement(by: 1, labels: {}) label_set = label_set_for(labels) - synchronize do - @values[label_set] ||= 0 - @values[label_set] -= by - end + @store.increment(labels: label_set, by: -by) end end end diff --git a/lib/prometheus/client/histogram.rb b/lib/prometheus/client/histogram.rb index 03f7cec4..236cf3a6 100644 --- a/lib/prometheus/client/histogram.rb +++ b/lib/prometheus/client/histogram.rb @@ -8,48 +8,29 @@ module Client # or response sizes) and counts them in configurable buckets. It also # provides a sum of all observed values. class Histogram < Metric - # Value represents the state of a Histogram at a given point. - class Value < Hash - attr_accessor :sum, :total - - def initialize(buckets:) - @sum = 0.0 - @total = 0.0 - - buckets.each do |bucket| - self[bucket] = 0.0 - end - end - - def observe(value) - @sum += value - @total += 1 - - each_key do |bucket| - self[bucket] += 1 if value <= bucket - end - end - end - # DEFAULT_BUCKETS are the default Histogram buckets. The default buckets # are tailored to broadly measure the response time (in seconds) of a # network service. (From DefBuckets client_golang) DEFAULT_BUCKETS = [0.005, 0.01, 0.025, 0.05, 0.1, 0.25, 0.5, 1, 2.5, 5, 10].freeze + attr_reader :buckets + # Offer a way to manually specify buckets def initialize(name, docstring:, labels: [], preset_labels: {}, - buckets: DEFAULT_BUCKETS) - raise ArgumentError, 'Unsorted buckets, typo?' unless sorted? buckets + buckets: DEFAULT_BUCKETS, + store_settings: {}) + raise ArgumentError, 'Unsorted buckets, typo?' unless sorted?(buckets) @buckets = buckets super(name, docstring: docstring, labels: labels, - preset_labels: preset_labels) + preset_labels: preset_labels, + store_settings: store_settings) end def type @@ -57,8 +38,42 @@ def type end def observe(value, labels: {}) - label_set = label_set_for(labels) - synchronize { @values[label_set].observe(value) } + base_label_set = label_set_for(labels) + + @store.synchronize do + buckets.each do |upper_limit| + next if value > upper_limit + @store.increment(labels: base_label_set.merge(le: upper_limit), by: 1) + end + @store.increment(labels: base_label_set.merge(le: "+Inf"), by: 1) + @store.increment(labels: base_label_set.merge(le: "sum"), by: value) + end + end + + # Returns a hash with all the buckets plus +Inf (count) plus Sum for the given label set + def get(labels: {}) + base_label_set = label_set_for(labels) + + all_buckets = buckets + ["+Inf", "sum"] + + @store.synchronize do + all_buckets.each_with_object({}) do |upper_limit, acc| + acc[upper_limit] = @store.get(labels: base_label_set.merge(le: upper_limit)) + end.tap do |acc| + acc["count"] = acc["+Inf"] + end + end + end + + # Returns all label sets with their values expressed as hashes with their buckets + def values + v = @store.all_values + + v.each_with_object({}) do |(label_set, v), acc| + actual_label_set = label_set.reject{|l| l == :le } + acc[actual_label_set] ||= @buckets.map{|b| [b, 0.0]}.to_h + acc[actual_label_set][label_set[:le]] = v + end end private @@ -67,10 +82,6 @@ def reserved_labels [:le] end - def default - Value.new(buckets: @buckets) - end - def sorted?(bucket) bucket.each_cons(2).all? { |i, j| i <= j } end diff --git a/lib/prometheus/client/metric.rb b/lib/prometheus/client/metric.rb index eafce142..d89069f1 100644 --- a/lib/prometheus/client/metric.rb +++ b/lib/prometheus/client/metric.rb @@ -9,35 +9,39 @@ module Client class Metric attr_reader :name, :docstring, :preset_labels - def initialize(name, docstring:, labels: [], preset_labels: {}) - @mutex = Mutex.new - @validator = LabelSetValidator.new(expected_labels: labels, - reserved_labels: reserved_labels) - @values = Hash.new { |hash, key| hash[key] = default } + def initialize(name, + docstring:, + labels: [], + preset_labels: {}, + store_settings: {}) validate_name(name) validate_docstring(docstring) + @validator = LabelSetValidator.new(expected_labels: labels, + reserved_labels: reserved_labels) @validator.valid?(labels) @validator.valid?(preset_labels) @name = name @docstring = docstring @preset_labels = preset_labels + + @store = Prometheus::Client.config.data_store.for_metric( + name, + metric_type: type, + metric_settings: store_settings + ) end # Returns the value for the given label set def get(labels: {}) label_set = label_set_for(labels) - @values[label_set] + @store.get(labels: label_set) end # Returns all label sets with their values def values - synchronize do - @values.each_with_object({}) do |(labels, value), memo| - memo[labels] = value - end - end + @store.all_values end private @@ -46,10 +50,6 @@ def reserved_labels [] end - def default - nil - end - def validate_name(name) unless name.is_a?(Symbol) raise ArgumentError, 'metric name must be a symbol' @@ -69,10 +69,6 @@ def validate_docstring(docstring) def label_set_for(labels) @validator.validate(preset_labels.merge(labels)) end - - def synchronize - @mutex.synchronize { yield } - end end end end diff --git a/lib/prometheus/client/registry.rb b/lib/prometheus/client/registry.rb index 58893b3c..6f13b1e0 100644 --- a/lib/prometheus/client/registry.rb +++ b/lib/prometheus/client/registry.rb @@ -37,34 +37,39 @@ def unregister(name) end end - def counter(name, docstring:, labels: [], preset_labels: {}) + def counter(name, docstring:, labels: [], preset_labels: {}, store_settings: {}) register(Counter.new(name, docstring: docstring, labels: labels, - preset_labels: preset_labels)) + preset_labels: preset_labels, + store_settings: {})) end - def summary(name, docstring:, labels: [], preset_labels: {}) + def summary(name, docstring:, labels: [], preset_labels: {}, store_settings: {}) register(Summary.new(name, docstring: docstring, labels: labels, - preset_labels: preset_labels)) + preset_labels: preset_labels, + store_settings: {})) end - def gauge(name, docstring:, labels: [], preset_labels: {}) + def gauge(name, docstring:, labels: [], preset_labels: {}, store_settings: {}) register(Gauge.new(name, docstring: docstring, labels: labels, - preset_labels: preset_labels)) + preset_labels: preset_labels, + store_settings: {})) end def histogram(name, docstring:, labels: [], preset_labels: {}, - buckets: Histogram::DEFAULT_BUCKETS) + buckets: Histogram::DEFAULT_BUCKETS, + store_settings: {}) register(Histogram.new(name, docstring: docstring, labels: labels, preset_labels: preset_labels, - buckets: buckets)) + buckets: buckets, + store_settings: {})) end def exist?(name) diff --git a/lib/prometheus/client/summary.rb b/lib/prometheus/client/summary.rb index 066ab0e8..9f65faa8 100644 --- a/lib/prometheus/client/summary.rb +++ b/lib/prometheus/client/summary.rb @@ -7,29 +7,42 @@ module Client # Summary is an accumulator for samples. It captures Numeric data and # provides the total count and sum of observations. class Summary < Metric - # Value represents the state of a Summary at a given point. - class Value - attr_accessor :sum, :total + def type + :summary + end - def initialize - @sum = 0.0 - @total = 0.0 - end + # Records a given value. + def observe(value, labels: {}) + base_label_set = label_set_for(labels) - def observe(value) - @sum += value - @total += 1 + @store.synchronize do + @store.increment(labels: base_label_set.merge(quantile: "count"), by: 1) + @store.increment(labels: base_label_set.merge(quantile: "sum"), by: value) end end - def type - :summary + # Returns a hash with "sum" and "count" as keys + def get(labels: {}) + base_label_set = label_set_for(labels) + + internal_counters = ["count", "sum"] + + @store.synchronize do + internal_counters.each_with_object({}) do |counter, acc| + acc[counter] = @store.get(labels: base_label_set.merge(quantile: counter)) + end + end end - # Records a given value. - def observe(value, labels: {}) - label_set = label_set_for(labels) - synchronize { @values[label_set].observe(value) } + # Returns all label sets with their values expressed as hashes with their sum/count + def values + v = @store.all_values + + v.each_with_object({}) do |(label_set, v), acc| + actual_label_set = label_set.reject{|l| l == :quantile } + acc[actual_label_set] ||= { "count" => 0.0, "sum" => 0.0 } + acc[actual_label_set][label_set[:quantile]] = v + end end private @@ -37,10 +50,6 @@ def observe(value, labels: {}) def reserved_labels [:quantile] end - - def default - Value.new - end end end end diff --git a/spec/examples/data_store_example.rb b/spec/examples/data_store_example.rb new file mode 100644 index 00000000..23e76d3d --- /dev/null +++ b/spec/examples/data_store_example.rb @@ -0,0 +1,58 @@ +# encoding: UTF-8 + +shared_examples_for Prometheus::Client::DataStores do + describe "MetricStore#set and #get" do + it "returns the value set for each labelset" do + metric_store.set(labels: { foo: "bar" }, val: 5) + metric_store.set(labels: { foo: "baz" }, val: 2) + expect(metric_store.get(labels: { foo: "bar" })).to eq(5) + expect(metric_store.get(labels: { foo: "baz" })).to eq(2) + expect(metric_store.get(labels: { foo: "bat" })).to eq(0) + end + end + + describe "MetricStore#increment" do + it "returns the value set for each labelset" do + metric_store.set(labels: { foo: "bar" }, val: 5) + metric_store.set(labels: { foo: "baz" }, val: 2) + + metric_store.increment(labels: { foo: "bar" }) + metric_store.increment(labels: { foo: "baz" }, by: 7) + metric_store.increment(labels: { foo: "zzz" }, by: 3) + + expect(metric_store.get(labels: { foo: "bar" })).to eq(6) + expect(metric_store.get(labels: { foo: "baz" })).to eq(9) + expect(metric_store.get(labels: { foo: "zzz" })).to eq(3) + end + end + + describe "MetricStore#synchronize" do + # I'm not sure it's possible to actually test that this synchronizes, but at least + # it should run the passed block + it "accepts a block and runs it" do + a = 0 + metric_store.synchronize{ a += 1 } + expect(a).to eq(1) + end + + # This is just a safety check that we're not getting "nested transaction" issues + it "allows modifying the store while in synchronized block" do + metric_store.synchronize do + metric_store.increment(labels: { foo: "bar" }) + metric_store.increment(labels: { foo: "baz" }) + end + end + end + + describe "MetricStore#all_values" do + it "returns all specified labelsets, with their associated value" do + metric_store.set(labels: { foo: "bar" }, val: 5) + metric_store.set(labels: { foo: "baz" }, val: 2) + + expect(metric_store.all_values).to eq( + { foo: "bar" } => 5.0, + { foo: "baz" } => 2.0, + ) + end + end +end diff --git a/spec/prometheus/client/counter_spec.rb b/spec/prometheus/client/counter_spec.rb index 8e5a47d1..29164cac 100644 --- a/spec/prometheus/client/counter_spec.rb +++ b/spec/prometheus/client/counter_spec.rb @@ -1,9 +1,15 @@ # encoding: UTF-8 +require 'prometheus/client' require 'prometheus/client/counter' require 'examples/metric_example' describe Prometheus::Client::Counter do + # Reset the data store + before do + Prometheus::Client.config.data_store = Prometheus::Client::DataStores::Synchronized.new + end + let(:expected_labels) { [] } let(:counter) do diff --git a/spec/prometheus/client/data_stores/synchronized_spec.rb b/spec/prometheus/client/data_stores/synchronized_spec.rb new file mode 100644 index 00000000..b69bdfa2 --- /dev/null +++ b/spec/prometheus/client/data_stores/synchronized_spec.rb @@ -0,0 +1,19 @@ +# encoding: UTF-8 + +require 'prometheus/client/data_stores/synchronized' +require 'examples/data_store_example' + +describe Prometheus::Client::DataStores::Synchronized do + subject { described_class.new } + let(:metric_store) { subject.for_metric(:metric_name, metric_type: :counter) } + + it_behaves_like Prometheus::Client::DataStores + + it "does not accept Metric Settings" do + expect do + subject.for_metric(:metric_name, + metric_type: :counter, + metric_settings: { some_setting: true }) + end.to raise_error(Prometheus::Client::DataStores::Synchronized::InvalidStoreSettingsError) + end +end diff --git a/spec/prometheus/client/formats/text_spec.rb b/spec/prometheus/client/formats/text_spec.rb index 0150fdf9..147f7d9a 100644 --- a/spec/prometheus/client/formats/text_spec.rb +++ b/spec/prometheus/client/formats/text_spec.rb @@ -4,13 +4,11 @@ describe Prometheus::Client::Formats::Text do let(:summary_value) do - Struct.new(:sum, :total).new(1243.21, 93.0) + { "count" => 93.0, "sum" => 1243.21 } end let(:histogram_value) do - { 10 => 1.0, 20 => 2.0, 30 => 2.0 }.tap do |value| - allow(value).to receive_messages(sum: 15.2, total: 2.0) - end + { 10 => 1.0, 20 => 2.0, 30 => 2.0, "+Inf" => 2.0, "sum" => 15.2 } end let(:registry) do diff --git a/spec/prometheus/client/gauge_spec.rb b/spec/prometheus/client/gauge_spec.rb index 14211c50..d8518956 100644 --- a/spec/prometheus/client/gauge_spec.rb +++ b/spec/prometheus/client/gauge_spec.rb @@ -1,9 +1,15 @@ # encoding: UTF-8 +require 'prometheus/client' require 'prometheus/client/gauge' require 'examples/metric_example' describe Prometheus::Client::Gauge do + # Reset the data store + before do + Prometheus::Client.config.data_store = Prometheus::Client::DataStores::Synchronized.new + end + let(:expected_labels) { [] } let(:gauge) do @@ -13,14 +19,14 @@ end it_behaves_like Prometheus::Client::Metric do - let(:type) { NilClass } + let(:type) { Float } end describe '#set' do it 'sets a metric value' do expect do gauge.set(42) - end.to change { gauge.get }.from(nil).to(42) + end.to change { gauge.get }.from(0).to(42) end it 'raises an InvalidLabelSetError if sending unexpected labels' do @@ -36,7 +42,7 @@ expect do expect do gauge.set(42, labels: { test: 'value' }) - end.to change { gauge.get(labels: { test: 'value' }) }.from(nil).to(42) + end.to change { gauge.get(labels: { test: 'value' }) }.from(0).to(42) end.to_not change { gauge.get(labels: { test: 'other' }) } end end diff --git a/spec/prometheus/client/histogram_spec.rb b/spec/prometheus/client/histogram_spec.rb index 32c329ac..b4472684 100644 --- a/spec/prometheus/client/histogram_spec.rb +++ b/spec/prometheus/client/histogram_spec.rb @@ -1,9 +1,15 @@ # encoding: UTF-8 +require 'prometheus/client' require 'prometheus/client/histogram' require 'examples/metric_example' describe Prometheus::Client::Histogram do + # Reset the data store + before do + Prometheus::Client.config.data_store = Prometheus::Client::DataStores::Synchronized.new + end + let(:expected_labels) { [] } let(:histogram) do @@ -75,18 +81,22 @@ it 'returns a set of buckets values' do expect(histogram.get(labels: { foo: 'bar' })) - .to eql(2.5 => 0.0, 5 => 2.0, 10 => 3.0) + .to eql( + 2.5 => 0.0, 5 => 2.0, 10 => 3.0, "+Inf" => 4.0, "count" => 4.0, "sum" => 25.2 + ) end - it 'returns a value which responds to #sum and #total' do + it 'returns a value which includes sum and count' do value = histogram.get(labels: { foo: 'bar' }) - expect(value.sum).to eql(25.2) - expect(value.total).to eql(4.0) + expect(value["sum"]).to eql(25.2) + expect(value["count"]).to eql(4.0) end it 'uses zero as default value' do - expect(histogram.get(labels: { foo: '' })).to eql(2.5 => 0.0, 5 => 0.0, 10 => 0.0) + expect(histogram.get(labels: { foo: '' })).to eql( + 2.5 => 0.0, 5 => 0.0, 10 => 0.0, "+Inf" => 0.0, "count" => 0.0, "sum" => 0.0 + ) end end @@ -98,8 +108,8 @@ histogram.observe(6, labels: { status: 'foo' }) expect(histogram.values).to eql( - { status: 'bar' } => { 2.5 => 0.0, 5 => 1.0, 10 => 1.0 }, - { status: 'foo' } => { 2.5 => 0.0, 5 => 0.0, 10 => 1.0 }, + { status: 'bar' } => { 2.5 => 0.0, 5 => 1.0, 10 => 1.0, "+Inf" => 1.0, "sum" => 3.0 }, + { status: 'foo' } => { 2.5 => 0.0, 5 => 0.0, 10 => 1.0, "+Inf" => 1.0, "sum" => 6.0 }, ) end end diff --git a/spec/prometheus/client/summary_spec.rb b/spec/prometheus/client/summary_spec.rb index 740a4d23..e01fa632 100644 --- a/spec/prometheus/client/summary_spec.rb +++ b/spec/prometheus/client/summary_spec.rb @@ -1,9 +1,15 @@ # encoding: UTF-8 +require 'prometheus/client' require 'prometheus/client/summary' require 'examples/metric_example' describe Prometheus::Client::Summary do + # Reset the data store + before do + Prometheus::Client.config.data_store = Prometheus::Client::DataStores::Synchronized.new + end + let(:expected_labels) { [] } let(:summary) do @@ -13,7 +19,7 @@ end it_behaves_like Prometheus::Client::Metric do - let(:type) { Prometheus::Client::Summary::Value } + let(:type) { Hash } end describe '#initialization' do @@ -27,10 +33,10 @@ describe '#observe' do it 'records the given value' do expect do - expect do - summary.observe(5) - end.to change { summary.get.sum }.from(0.0).to(5.0) - end.to change { summary.get.total }.from(0.0).to(1.0) + summary.observe(5) + end.to change { summary.get }. + from({ "count" => 0.0, "sum" => 0.0 }). + to({ "count" => 1.0, "sum" => 5.0 }) end it 'raise error for quantile labels' do @@ -52,8 +58,8 @@ expect do expect do summary.observe(5, labels: { test: 'value' }) - end.to change { summary.get(labels: { test: 'value' }).total } - end.to_not change { summary.get(labels: { test: 'other' }).total } + end.to change { summary.get(labels: { test: 'value' })["count"] } + end.to_not change { summary.get(labels: { test: 'other' })["count"] } end end end @@ -69,10 +75,8 @@ end it 'returns a value which responds to #sum and #total' do - value = summary.get(labels: { foo: 'bar' }) - - expect(value.sum).to eql(25.2) - expect(value.total).to eql(4.0) + expect(summary.get(labels: { foo: 'bar' })). + to eql({ "count" => 4.0, "sum" => 25.2 }) end end @@ -83,8 +87,10 @@ summary.observe(3, labels: { status: 'bar' }) summary.observe(5, labels: { status: 'foo' }) - expect(summary.values[{ status: 'bar' }].sum).to eql(3.0) - expect(summary.values[{ status: 'foo' }].sum).to eql(5.0) + expect(summary.values).to eql( + { status: 'bar' } => { "count" => 1.0, "sum" => 3.0 }, + { status: 'foo' } => { "count" => 1.0, "sum" => 5.0 }, + ) end end end diff --git a/spec/prometheus/middleware/collector_spec.rb b/spec/prometheus/middleware/collector_spec.rb index bbebd8fb..87ce2b60 100644 --- a/spec/prometheus/middleware/collector_spec.rb +++ b/spec/prometheus/middleware/collector_spec.rb @@ -6,6 +6,11 @@ describe Prometheus::Middleware::Collector do include Rack::Test::Methods + # Reset the data store + before do + Prometheus::Client.config.data_store = Prometheus::Client::DataStores::Synchronized.new + end + let(:registry) do Prometheus::Client::Registry.new end From 3178104710a912f282d0b13122483cb4d555a65f Mon Sep 17 00:00:00 2001 From: Daniel Magliola Date: Tue, 23 Oct 2018 17:43:26 +0100 Subject: [PATCH 08/18] Add `DataStores::SingleThreaded` This is the simplest possible store we can have, which simply has a hash for each metric, and no synchronization code whatsoever. It can only be used in single-threaded, single-process scenarios, but for those, it should be the fastest option. Signed-off-by: Daniel Magliola --- .../client/data_stores/single_threaded.rb | 58 +++++++++++++++++++ .../data_stores/single_threaded_spec.rb | 19 ++++++ 2 files changed, 77 insertions(+) create mode 100644 lib/prometheus/client/data_stores/single_threaded.rb create mode 100644 spec/prometheus/client/data_stores/single_threaded_spec.rb diff --git a/lib/prometheus/client/data_stores/single_threaded.rb b/lib/prometheus/client/data_stores/single_threaded.rb new file mode 100644 index 00000000..e4cb6a06 --- /dev/null +++ b/lib/prometheus/client/data_stores/single_threaded.rb @@ -0,0 +1,58 @@ +require 'concurrent' + +module Prometheus + module Client + module DataStores + # Stores all the data in a simple Hash for each Metric + # + # Has *no* synchronization primitives, making it the fastest store for single-threaded + # scenarios, but must absolutely not be used in multi-threaded scenarios. + class SingleThreaded + class InvalidStoreSettingsError < StandardError; end + + def for_metric(metric_name, metric_type:, metric_settings: {}) + # We don't need `metric_type` or `metric_settings` for this particular store + validate_metric_settings(metric_settings: metric_settings) + MetricStore.new + end + + private + + def validate_metric_settings(metric_settings:) + unless metric_settings.empty? + raise InvalidStoreSettingsError, + "SingleThreaded doesn't allow any metric_settings" + end + end + + class MetricStore + def initialize + @internal_store = Hash.new { |hash, key| hash[key] = 0.0 } + end + + def synchronize + yield + end + + def set(labels:, val:) + @internal_store[labels] = val.to_f + end + + def increment(labels:, by: 1) + @internal_store[labels] += by + end + + def get(labels:) + @internal_store[labels] + end + + def all_values + @internal_store.dup + end + end + + private_constant :MetricStore + end + end + end +end diff --git a/spec/prometheus/client/data_stores/single_threaded_spec.rb b/spec/prometheus/client/data_stores/single_threaded_spec.rb new file mode 100644 index 00000000..681eeb1e --- /dev/null +++ b/spec/prometheus/client/data_stores/single_threaded_spec.rb @@ -0,0 +1,19 @@ +# encoding: UTF-8 + +require 'prometheus/client/data_stores/single_threaded' +require 'examples/data_store_example' + +describe Prometheus::Client::DataStores::SingleThreaded do + subject { described_class.new } + let(:metric_store) { subject.for_metric(:metric_name, metric_type: :counter) } + + it_behaves_like Prometheus::Client::DataStores + + it "does not accept Metric Settings" do + expect do + subject.for_metric(:metric_name, + metric_type: :counter, + metric_settings: { some_setting: true }) + end.to raise_error(Prometheus::Client::DataStores::SingleThreaded::InvalidStoreSettingsError) + end +end From 5917ad849a46e9805297741df830b3d799323ab5 Mon Sep 17 00:00:00 2001 From: Daniel Magliola Date: Tue, 13 Nov 2018 15:52:53 +0000 Subject: [PATCH 09/18] Add `DataStores::DirectFileStore` This store keeps all its data in files, one file per process and per metric. These files have binary data mapping keys to Floats, and they're read/written at the specific offsets of these Floats. This is generally the recommended store to use to deal with pre-fork servers and other "multi-process" scenarios, at least until we crack `mmap`. This seems to be, for now, the fastest possible way to safely share data between all the processes. Because we never actually `fsync`, we never actually touch the disk, and FS caching makes this extremely fast. Almost as fast as the `mmaps` without all the added risk, or the burden of the C extensions. Each process gets their own file for a metric. When a Prometheus scrape is received in one of the processes, it finds all the files for a metric, reads their values and aggregates them. We use `flock` to guarantee consistency. Signed-off-by: Daniel Magliola --- .../client/data_stores/direct_file_store.rb | 313 ++++++++++++++++++ .../data_stores/direct_file_store_spec.rb | 169 ++++++++++ 2 files changed, 482 insertions(+) create mode 100644 lib/prometheus/client/data_stores/direct_file_store.rb create mode 100644 spec/prometheus/client/data_stores/direct_file_store_spec.rb diff --git a/lib/prometheus/client/data_stores/direct_file_store.rb b/lib/prometheus/client/data_stores/direct_file_store.rb new file mode 100644 index 00000000..62e72310 --- /dev/null +++ b/lib/prometheus/client/data_stores/direct_file_store.rb @@ -0,0 +1,313 @@ +require 'concurrent' +require 'fileutils' +require "cgi" + +module Prometheus + module Client + module DataStores + # Stores data in binary files, one file per process and per metric. + # This is generally the recommended store to use to deal with pre-fork servers and + # other "multi-process" scenarios. + # + # Each process will get a file for a metric, and it will manage its contents by + # storing keys next to binary-encoded Floats, and keeping track of the offsets of + # those Floats, to be able to update them directly as they increase. + # + # When exporting metrics, the process that gets scraped by Prometheus will find + # all the files that apply to a metric, read their contents, and aggregate them + # (generally that means SUMming the values for each labelset). + # + # In order to do this, each Metric needs an `:aggregation` setting, specifying how + # to aggregate the multiple possible values we can get for each labelset. By default, + # they are `SUM`med, which is what most use cases call for (counters and histograms, + # for example). + # However, for Gauges, it's possible to set `MAX` or `MIN` as aggregation, to get + # the highest value of all the processes / threads. + + class DirectFileStore + class InvalidStoreSettingsError < StandardError; end + AGGREGATION_MODES = [MAX = :max, MIN = :min, SUM = :sum] + DEFAULT_METRIC_SETTINGS = { aggregation: SUM } + + def initialize(dir:) + @store_settings = { dir: dir } + FileUtils.mkdir_p(dir) + end + + def for_metric(metric_name, metric_type:, metric_settings: {}) + settings = DEFAULT_METRIC_SETTINGS.merge(metric_settings) + validate_metric_settings(settings) + + MetricStore.new(metric_name: metric_name, + store_settings: @store_settings, + metric_settings: settings) + end + + private + + def validate_metric_settings(metric_settings) + unless metric_settings.has_key?(:aggregation) && + AGGREGATION_MODES.include?(metric_settings[:aggregation]) + raise InvalidStoreSettingsError, + "Metrics need a valid :aggregation key" + end + + unless (metric_settings.keys - [:aggregation]).empty? + raise InvalidStoreSettingsError, + "Only :aggregation setting can be specified" + end + end + + class MetricStore + attr_reader :metric_name, :store_settings + + def initialize(metric_name:, store_settings:, metric_settings:) + @metric_name = metric_name + @store_settings = store_settings + @values_aggregation_mode = metric_settings[:aggregation] + + @rwlock = Concurrent::ReentrantReadWriteLock.new + end + + # Synchronize is used to do a multi-process Mutex, when incrementing multiple + # values at once, so that the other process, reading the file for export, doesn't + # get incomplete increments. + # + # `in_process_sync`, instead, is just used so that two threads don't increment + # the same value and get a context switch between read and write leading to an + # inconsistency + def synchronize + in_process_sync do + internal_store.with_file_lock do + yield + end + end + end + + def set(labels:, val:) + in_process_sync do + internal_store.write_value(store_key(labels), val.to_f) + end + end + + def increment(labels:, by: 1) + key = store_key(labels) + in_process_sync do + value = internal_store.read_value(key) + internal_store.write_value(key, value + by.to_f) + end + end + + def get(labels:) + in_process_sync do + internal_store.read_value(store_key(labels)) + end + end + + def all_values + stores_data = Hash.new{ |hash, key| hash[key] = [] } + + # There's no need to call `synchronize` here. We're opening a second handle to + # the file, and `flock`ing it, which prevents inconsistent reads + stores_for_metric.each do |file_path| + begin + store = FileMappedDict.new(file_path, true) + store.all_values.each do |(labelset_qs, v)| + # Labels come as a query string, and CGI::parse returns arrays for each key + # "foo=bar&x=y" => { "foo" => ["bar"], "x" => ["y"] } + # Turn the keys back into symbols, and remove the arrays + label_set = CGI::parse(labelset_qs).map do |k, vs| + [k.to_sym, vs.first] + end.to_h + + stores_data[label_set] << v + end + ensure + store.close if store + end + end + + # Aggregate all the different values for each label_set + stores_data.each_with_object({}) do |(label_set, values), acc| + acc[label_set] = aggregate_values(values) + end + end + + private + + def in_process_sync + @rwlock.with_write_lock { yield } + end + + def store_key(labels) + labels.map{|k,v| "#{CGI::escape(k.to_s)}=#{CGI::escape(v.to_s)}"}.join('&') + end + + def internal_store + @internal_store ||= FileMappedDict.new(filemap_filename) + end + + # Filename for this metric's PStore (one per process) + def filemap_filename + filename = "metric_#{ metric_name }___#{ process_id }.bin" + File.join(@store_settings[:dir], filename) + end + + def stores_for_metric + Dir.glob(File.join(@store_settings[:dir], "metric_#{ metric_name }___*")) + end + + def process_id + Process.pid + end + + def aggregate_values(values) + if @values_aggregation_mode == SUM + values.inject { |sum, element| sum + element } + elsif @values_aggregation_mode == MAX + values.max + elsif @values_aggregation_mode == MIN + values.min + else + raise InvalidStoreSettingsError, + "Invalid Aggregation Mode: #{ @values_aggregation_mode }" + end + end + end + + private_constant :MetricStore + + # A dict of doubles, backed by an file we access directly a a byte array. + # + # The file starts with a 4 byte int, indicating how much of it is used. + # Then 4 bytes of padding. + # There's then a number of entries, consisting of a 4 byte int which is the + # size of the next field, a utf-8 encoded string key, padding to an 8 byte + # alignment, and then a 8 byte float which is the value. + class FileMappedDict + INITIAL_FILE_SIZE = 1024*1024 + + attr_reader :capacity, :used, :positions + + def initialize(filename, readonly = false) + @positions = {} + @used = 0 + + open_file(filename, readonly) + @used = @f.read(4).unpack('l')[0] if @capacity > 0 + + if @used > 0 + # File already has data. Read the existing values + with_file_lock do + read_all_values.each do |key, _, pos| + @positions[key] = pos + end + end + else + # File is empty. Init the `used` counter, if we're in write mode + if !readonly + @used = 8 + @f.seek(0) + @f.write([@used].pack('l')) + end + end + end + + # Yield (key, value, pos). No locking is performed. + def all_values + with_file_lock do + read_all_values.map { |k, v, p| [k, v] } + end + end + + def read_value(key) + if !@positions.has_key?(key) + init_value(key) + end + + pos = @positions[key] + @f.seek(pos) + @f.read(8).unpack('d')[0] + end + + def write_value(key, value) + if !@positions.has_key?(key) + init_value(key) + end + + pos = @positions[key] + @f.seek(pos) + @f.write([value].pack('d')) + @f.flush + end + + def close + @f.close + end + + def with_file_lock + @f.flock(File::LOCK_EX) + yield + ensure + @f.flock(File::LOCK_UN) + end + + private + + def open_file(filename, readonly) + mode = if readonly + "r" + elsif File.exist?(filename) + "r+b" + else + "w+b" + end + + @f = File.open(filename, mode) + if @f.size == 0 && !readonly + resize_file(INITIAL_FILE_SIZE) + end + @capacity = @f.size + end + + def resize_file(new_capacity) + @f.truncate(new_capacity) + end + + # Initialize a value. Lock must be held by caller. + def init_value(key) + # Pad to be 8-byte aligned. + padded = key + (' ' * (8 - (key.length + 4) % 8)) + value = [padded.length, padded, 0.0].pack("lA#{padded.length}d") + while @used + value.length > @capacity + @capacity *= 2 + resize_file(@capacity) + end + @f.seek(@used) + @f.write(value) + @used += value.length + @f.seek(0) + @f.write([@used].pack('l')) + @f.flush + @positions[key] = @used - 8 + end + + # Yield (key, value, pos). No locking is performed. + def read_all_values + @f.seek(8) + values = [] + while @f.pos < @used + padded_len = @f.read(4).unpack('l')[0] + encoded = @f.read(padded_len).unpack("A#{padded_len}")[0] + value = @f.read(8).unpack('d')[0] + values << [encoded.strip, value, @f.pos - 8] + end + values + end + end + end + end + end +end + + diff --git a/spec/prometheus/client/data_stores/direct_file_store_spec.rb b/spec/prometheus/client/data_stores/direct_file_store_spec.rb new file mode 100644 index 00000000..fa1d335b --- /dev/null +++ b/spec/prometheus/client/data_stores/direct_file_store_spec.rb @@ -0,0 +1,169 @@ +# encoding: UTF-8 + +require 'prometheus/client/data_stores/direct_file_store' +require 'examples/data_store_example' + +describe Prometheus::Client::DataStores::DirectFileStore do + subject { described_class.new(dir: "/tmp/prometheus_test") } + let(:metric_store) { subject.for_metric(:metric_name, metric_type: :counter) } + + # Reset the PStores + before do + Dir.glob('/tmp/prometheus_test/*').each { |file| File.delete(file) } + end + + it_behaves_like Prometheus::Client::DataStores + + it "only accepts valid :aggregation as Metric Settings" do + expect do + subject.for_metric(:metric_name, + metric_type: :counter, + metric_settings: { aggregation: Prometheus::Client::DataStores::DirectFileStore::SUM }) + end.not_to raise_error + + expect do + subject.for_metric(:metric_name, + metric_type: :counter, + metric_settings: { aggregation: :invalid }) + end.to raise_error(Prometheus::Client::DataStores::DirectFileStore::InvalidStoreSettingsError) + + expect do + subject.for_metric(:metric_name, + metric_type: :counter, + metric_settings: { some_setting: true }) + end.to raise_error(Prometheus::Client::DataStores::DirectFileStore::InvalidStoreSettingsError) + end + + it "raises when aggregating if we get to that that point with an invalid aggregation mode" do + # This is basically just for coverage of a safety clause that can never be reached + allow(subject).to receive(:validate_metric_settings) # turn off validation + + metric = subject.for_metric(:metric_name, + metric_type: :counter, + metric_settings: { aggregation: :invalid }) + metric.increment(labels: {}, by: 1) + + expect do + metric.all_values + end.to raise_error(Prometheus::Client::DataStores::DirectFileStore::InvalidStoreSettingsError) + end + + it "opens the same file twice, if it already exists" do + # Testing this simply for coverage + ms = metric_store + ms.increment(labels: {}, by: 1) + + ms2 = subject.for_metric(:metric_name, metric_type: :counter) + ms2.increment(labels: {}, by: 1) + end + + + it "sums values from different processes" do + allow(Process).to receive(:pid).and_return(12345) + metric_store1 = subject.for_metric(:metric_name, metric_type: :counter) + 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: :counter) + 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_store2.all_values).to eq( + { foo: "bar" } => 4.0, + { foo: "baz" } => 9.0, + { foo: "yyy" } => 3.0, + { foo: "zzz" } => 1.0, + ) + + # Both processes should return the same value + expect(metric_store1.all_values).to eq(metric_store2.all_values) + end + + context "with a metric that takes MAX instead of SUM" do + it "reports the maximum 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: :max } + ) + 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: :max } + ) + 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" } => 3.0, + { foo: "baz" } => 7.0, + { foo: "yyy" } => 3.0, + { foo: "zzz" } => 1.0, + ) + + # Both processes should return the same value + expect(metric_store1.all_values).to eq(metric_store2.all_values) + end + end + + context "with a metric that takes MIN instead of SUM" do + it "reports the minimum 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: :min } + ) + 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: :min } + ) + 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" } => 1.0, + { foo: "baz" } => 2.0, + { foo: "yyy" } => 3.0, + { foo: "zzz" } => 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). + to receive(:resize_file).and_wrap_original do |original_method, *args, &block| + + truncate_calls_count += 1 + original_method.call(*args, &block) + end + + really_long_string = "a" * 500_000 + 10.times do |i| + metric_store.set(labels: { foo: "#{ really_long_string }#{ i }" }, val: 1) + end + + expect(truncate_calls_count).to be >= 3 + end +end From 403b31a7e783768810ebd244b06a0feb3c74eada Mon Sep 17 00:00:00 2001 From: Daniel Magliola Date: Tue, 16 Oct 2018 15:34:03 +0100 Subject: [PATCH 10/18] Change `text_spec.rb` to be more like an integration test The existing spec on `Formats::Text` is based on mocking a registry with fake metrics and values inside, which means if anything changes in the interface for metrics, the test will not catch it. And there's no test validating that interface. This test is more realistic, and it actually catches the kind of bugs we introduced in the process of this refactor Signed-off-by: Daniel Magliola --- spec/prometheus/client/formats/text_spec.rb | 96 ++++++++++----------- 1 file changed, 44 insertions(+), 52 deletions(-) diff --git a/spec/prometheus/client/formats/text_spec.rb b/spec/prometheus/client/formats/text_spec.rb index 147f7d9a..d200ec1c 100644 --- a/spec/prometheus/client/formats/text_spec.rb +++ b/spec/prometheus/client/formats/text_spec.rb @@ -1,62 +1,54 @@ # encoding: UTF-8 +require 'prometheus/client' +require 'prometheus/client/registry' require 'prometheus/client/formats/text' describe Prometheus::Client::Formats::Text do - let(:summary_value) do - { "count" => 93.0, "sum" => 1243.21 } + # Reset the data store + before do + Prometheus::Client.config.data_store = Prometheus::Client::DataStores::Synchronized.new end - let(:histogram_value) do - { 10 => 1.0, 20 => 2.0, 30 => 2.0, "+Inf" => 2.0, "sum" => 15.2 } - end + let(:registry) { Prometheus::Client::Registry.new } + + before do + foo = registry.counter(:foo, + docstring: 'foo description', + labels: [:umlauts, :utf, :code], + preset_labels: {umlauts: 'Björn', utf: '佖佥'}) + foo.increment(labels: { code: 'red'}, by: 42) + foo.increment(labels: { code: 'green'}, by: 3.14E42) + foo.increment(labels: { code: 'blue'}, by: 1.23e-45) + + + bar = registry.gauge(:bar, + docstring: "bar description\nwith newline", + labels: [:status, :code]) + bar.set(15, labels: { status: 'success', code: 'pink'}) + + + baz = registry.counter(:baz, + docstring: 'baz "description" \\escaping', + labels: [:text]) + baz.increment(labels: { text: "with \"quotes\", \\escape \n and newline" }, by: 15.0) + + + qux = registry.summary(:qux, + docstring: 'qux description', + labels: [:for, :code], + preset_labels: { for: 'sake', code: '1' }) + 92.times { qux.observe(0) } + qux.observe(1243.21) + - let(:registry) do - metrics = [ - double( - name: :foo, - docstring: 'foo description', - type: :counter, - values: { - { umlauts: 'Björn', utf: '佖佥', code: 'red' } => 42.0, - { umlauts: 'Björn', utf: '佖佥', code: 'green' } => 3.14E42, - { umlauts: 'Björn', utf: '佖佥', code: 'blue' } => -1.23e-45, - }, - ), - double( - name: :bar, - docstring: "bar description\nwith newline", - type: :gauge, - values: { - { status: 'success', code: 'pink' } => 15.0, - }, - ), - double( - name: :baz, - docstring: 'baz "description" \\escaping', - type: :counter, - values: { - { text: "with \"quotes\", \\escape \n and newline" } => 15.0, - }, - ), - double( - name: :qux, - docstring: 'qux description', - type: :summary, - values: { - { for: 'sake', code: '1' } => summary_value, - }, - ), - double( - name: :xuq, - docstring: 'xuq description', - type: :histogram, - values: { - { code: 'ah' } => histogram_value, - }, - ), - ] - double(metrics: metrics) + xuq = registry.histogram(:xuq, + docstring: 'xuq description', + labels: [:code], + preset_labels: {code: 'ah'}, + buckets: [10, 20, 30]) + xuq.observe(12) + xuq.observe(3.2) end describe '.marshal' do @@ -66,7 +58,7 @@ # HELP foo foo description foo{umlauts="Björn",utf="佖佥",code="red"} 42.0 foo{umlauts="Björn",utf="佖佥",code="green"} 3.14e+42 -foo{umlauts="Björn",utf="佖佥",code="blue"} -1.23e-45 +foo{umlauts="Björn",utf="佖佥",code="blue"} 1.23e-45 # TYPE bar gauge # HELP bar bar description\nwith newline bar{status="success",code="pink"} 15.0 From ac9a5149a14421f0800526ff0183adc1ea18fb23 Mon Sep 17 00:00:00 2001 From: Daniel Magliola Date: Thu, 8 Nov 2018 16:23:23 +0000 Subject: [PATCH 11/18] Update Histogram to return buckets as strings In the Text export format, the `le` label is reported as a string. Moreover, some of our stores may coerce Float label values into Strings. This is fine, since Labelsets should be a hash of symbols to strings. It also makes sense since one of those "buckets" will be "+Inf". It's also somewhat unavoidable, since neither the Store nor the Metric should be coercing those back into Floats, so report the buckets as strings instead of Floats. Signed-off-by: Daniel Magliola --- lib/prometheus/client/histogram.rb | 6 +++--- spec/prometheus/client/histogram_spec.rb | 8 ++++---- spec/prometheus/middleware/collector_spec.rb | 6 +++--- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/prometheus/client/histogram.rb b/lib/prometheus/client/histogram.rb index 236cf3a6..2ee94e9e 100644 --- a/lib/prometheus/client/histogram.rb +++ b/lib/prometheus/client/histogram.rb @@ -58,7 +58,7 @@ def get(labels: {}) @store.synchronize do all_buckets.each_with_object({}) do |upper_limit, acc| - acc[upper_limit] = @store.get(labels: base_label_set.merge(le: upper_limit)) + acc[upper_limit.to_s] = @store.get(labels: base_label_set.merge(le: upper_limit)) end.tap do |acc| acc["count"] = acc["+Inf"] end @@ -71,8 +71,8 @@ def values v.each_with_object({}) do |(label_set, v), acc| actual_label_set = label_set.reject{|l| l == :le } - acc[actual_label_set] ||= @buckets.map{|b| [b, 0.0]}.to_h - acc[actual_label_set][label_set[:le]] = v + acc[actual_label_set] ||= @buckets.map{|b| [b.to_s, 0.0]}.to_h + acc[actual_label_set][label_set[:le].to_s] = v end end diff --git a/spec/prometheus/client/histogram_spec.rb b/spec/prometheus/client/histogram_spec.rb index b4472684..f90bce72 100644 --- a/spec/prometheus/client/histogram_spec.rb +++ b/spec/prometheus/client/histogram_spec.rb @@ -82,7 +82,7 @@ it 'returns a set of buckets values' do expect(histogram.get(labels: { foo: 'bar' })) .to eql( - 2.5 => 0.0, 5 => 2.0, 10 => 3.0, "+Inf" => 4.0, "count" => 4.0, "sum" => 25.2 + "2.5" => 0.0, "5" => 2.0, "10" => 3.0, "+Inf" => 4.0, "count" => 4.0, "sum" => 25.2 ) end @@ -95,7 +95,7 @@ it 'uses zero as default value' do expect(histogram.get(labels: { foo: '' })).to eql( - 2.5 => 0.0, 5 => 0.0, 10 => 0.0, "+Inf" => 0.0, "count" => 0.0, "sum" => 0.0 + "2.5" => 0.0, "5" => 0.0, "10" => 0.0, "+Inf" => 0.0, "count" => 0.0, "sum" => 0.0 ) end end @@ -108,8 +108,8 @@ histogram.observe(6, labels: { status: 'foo' }) expect(histogram.values).to eql( - { status: 'bar' } => { 2.5 => 0.0, 5 => 1.0, 10 => 1.0, "+Inf" => 1.0, "sum" => 3.0 }, - { status: 'foo' } => { 2.5 => 0.0, 5 => 0.0, 10 => 1.0, "+Inf" => 1.0, "sum" => 6.0 }, + { status: 'bar' } => { "2.5" => 0.0, "5" => 1.0, "10" => 1.0, "+Inf" => 1.0, "sum" => 3.0 }, + { status: 'foo' } => { "2.5" => 0.0, "5" => 0.0, "10" => 1.0, "+Inf" => 1.0, "sum" => 6.0 }, ) end end diff --git a/spec/prometheus/middleware/collector_spec.rb b/spec/prometheus/middleware/collector_spec.rb index 87ce2b60..1e916c72 100644 --- a/spec/prometheus/middleware/collector_spec.rb +++ b/spec/prometheus/middleware/collector_spec.rb @@ -50,7 +50,7 @@ metric = :http_server_request_duration_seconds labels = { method: 'get', path: '/foo' } - expect(registry.get(metric).get(labels: labels)).to include(0.1 => 0, 0.25 => 1) + expect(registry.get(metric).get(labels: labels)).to include("0.1" => 0, "0.25" => 1) end it 'normalizes paths containing numeric IDs by default' do @@ -64,7 +64,7 @@ metric = :http_server_request_duration_seconds labels = { method: 'get', path: '/foo/:id/bars' } - expect(registry.get(metric).get(labels: labels)).to include(0.1 => 0, 0.5 => 1) + expect(registry.get(metric).get(labels: labels)).to include("0.1" => 0, "0.5" => 1) end it 'normalizes paths containing UUIDs by default' do @@ -78,7 +78,7 @@ metric = :http_server_request_duration_seconds labels = { method: 'get', path: '/foo/:uuid/bars' } - expect(registry.get(metric).get(labels: labels)).to include(0.1 => 0, 0.5 => 1) + expect(registry.get(metric).get(labels: labels)).to include("0.1" => 0, "0.5" => 1) end context 'when the app raises an exception' do From 190f7706e6dc6827dab53832c6b05a77d8ba63b2 Mon Sep 17 00:00:00 2001 From: Daniel Magliola Date: Fri, 9 Nov 2018 13:35:54 +0000 Subject: [PATCH 12/18] Accumulate Histogram Buckets on Export, not Observe In a Histogram, each bucket's count includes also the counts of all buckets smaller than it. The original code for Histograms would handle this at `observe` time, by incrementing all buckets bigger than the observed sample. Since calls to the Data Store may be expensive, and also all these increments need to be done atomically to guarantee the export doesn't have inconsistent values, this can have a big effect on performance. With this change, at `observe` time, we only increase the bucket the sample falls in, and the `sum`, and do the accumulation when reading the Histogram, which makes observations about 30% to 40% faster with a "uniform" distribution of sample. A sample skewed towards smaller buckets should show a much larger performance gain. With this change, we also remove the `count` key from the result of calling `get`. This key is redundant (it's the same value as `+Inf`, and rendering this as `count` is the job of the Text Exporter already, `get` shouldn't be returning it. Signed-off-by: Daniel Magliola --- lib/prometheus/client/histogram.rb | 31 ++++++++++++++++++------ spec/prometheus/client/histogram_spec.rb | 7 +++--- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/lib/prometheus/client/histogram.rb b/lib/prometheus/client/histogram.rb index 2ee94e9e..25f137da 100644 --- a/lib/prometheus/client/histogram.rb +++ b/lib/prometheus/client/histogram.rb @@ -39,13 +39,11 @@ def type def observe(value, labels: {}) base_label_set = label_set_for(labels) + bucket = buckets.find {|upper_limit| upper_limit > value } + bucket = "+Inf" if bucket.nil? @store.synchronize do - buckets.each do |upper_limit| - next if value > upper_limit - @store.increment(labels: base_label_set.merge(le: upper_limit), by: 1) - end - @store.increment(labels: base_label_set.merge(le: "+Inf"), by: 1) + @store.increment(labels: base_label_set.merge(le: bucket.to_s), by: 1) @store.increment(labels: base_label_set.merge(le: "sum"), by: value) end end @@ -58,9 +56,9 @@ def get(labels: {}) @store.synchronize do all_buckets.each_with_object({}) do |upper_limit, acc| - acc[upper_limit.to_s] = @store.get(labels: base_label_set.merge(le: upper_limit)) + acc[upper_limit.to_s] = @store.get(labels: base_label_set.merge(le: upper_limit.to_s)) end.tap do |acc| - acc["count"] = acc["+Inf"] + accumulate_buckets(acc) end end end @@ -69,15 +67,32 @@ def get(labels: {}) def values v = @store.all_values - v.each_with_object({}) do |(label_set, v), acc| + result = v.each_with_object({}) do |(label_set, v), acc| actual_label_set = label_set.reject{|l| l == :le } acc[actual_label_set] ||= @buckets.map{|b| [b.to_s, 0.0]}.to_h acc[actual_label_set][label_set[:le].to_s] = v end + + result.each do |(label_set, v)| + accumulate_buckets(v) + end end private + # Modifies the passed in parameter + def accumulate_buckets(h) + bucket_acc = 0 + buckets.each do |upper_limit| + bucket_value = h[upper_limit.to_s] + h[upper_limit.to_s] += bucket_acc + bucket_acc += bucket_value + end + + inf_value = h["+Inf"] || 0.0 + h["+Inf"] = inf_value + bucket_acc + end + def reserved_labels [:le] end diff --git a/spec/prometheus/client/histogram_spec.rb b/spec/prometheus/client/histogram_spec.rb index f90bce72..85327f7d 100644 --- a/spec/prometheus/client/histogram_spec.rb +++ b/spec/prometheus/client/histogram_spec.rb @@ -82,20 +82,19 @@ it 'returns a set of buckets values' do expect(histogram.get(labels: { foo: 'bar' })) .to eql( - "2.5" => 0.0, "5" => 2.0, "10" => 3.0, "+Inf" => 4.0, "count" => 4.0, "sum" => 25.2 + "2.5" => 0.0, "5" => 2.0, "10" => 3.0, "+Inf" => 4.0, "sum" => 25.2 ) end - it 'returns a value which includes sum and count' do + it 'returns a value which includes sum' do value = histogram.get(labels: { foo: 'bar' }) expect(value["sum"]).to eql(25.2) - expect(value["count"]).to eql(4.0) end it 'uses zero as default value' do expect(histogram.get(labels: { foo: '' })).to eql( - "2.5" => 0.0, "5" => 0.0, "10" => 0.0, "+Inf" => 0.0, "count" => 0.0, "sum" => 0.0 + "2.5" => 0.0, "5" => 0.0, "10" => 0.0, "+Inf" => 0.0, "sum" => 0.0 ) end end From 9d39313db55eaf863f9822650112686f20337c3a Mon Sep 17 00:00:00 2001 From: Daniel Magliola Date: Fri, 9 Nov 2018 14:02:03 +0000 Subject: [PATCH 13/18] Improve performance of Histograms Merging the `le` key into the `base_label_set` is where most time is spent, surprisingly. Also somewhat surprisingly, `dup` + setting a key is faster than `.merge`. This version is less pretty, but makes `observe` significantly faster For some stores (the ones that serialize the hash into a string), we could simply set the key without the `dup`, which makes this method about 3x faster. However, this would require calling `dup` into all Hash-based stores (or serializing the Hash into a string), which would make THEM significantly slower. Signed-off-by: Daniel Magliola --- lib/prometheus/client/histogram.rb | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/prometheus/client/histogram.rb b/lib/prometheus/client/histogram.rb index 25f137da..37f83e56 100644 --- a/lib/prometheus/client/histogram.rb +++ b/lib/prometheus/client/histogram.rb @@ -38,13 +38,20 @@ def type end def observe(value, labels: {}) - base_label_set = label_set_for(labels) bucket = buckets.find {|upper_limit| upper_limit > value } bucket = "+Inf" if bucket.nil? + base_label_set = label_set_for(labels) + + # This is basically faster than doing `.merge` + bucket_label_set = base_label_set.dup + bucket_label_set[:le] = bucket.to_s + sum_label_set = base_label_set.dup + sum_label_set[:le] = "sum" + @store.synchronize do - @store.increment(labels: base_label_set.merge(le: bucket.to_s), by: 1) - @store.increment(labels: base_label_set.merge(le: "sum"), by: value) + @store.increment(labels: bucket_label_set, by: 1) + @store.increment(labels: sum_label_set, by: value) end end From c1d6126d28de457854d45f7c1e29d5183bb7ebb8 Mon Sep 17 00:00:00 2001 From: Daniel Magliola Date: Tue, 16 Oct 2018 15:52:03 +0100 Subject: [PATCH 14/18] Add `Metric#with_labels` method `with_labels` returns a metric object, augmented with some pre-set labels. This is useful to be able to reduce repetition if a certain part of the code is passing the same label over and over again, but the label can have different values in different parts of the codebase (so passing in `preset_labels` when declaring the metric is not appropriate. Signed-off-by: Daniel Magliola --- README.md | 40 ++++++++++++++++++++++++ lib/prometheus/client/histogram.rb | 9 ++++++ lib/prometheus/client/metric.rb | 11 +++++++ spec/prometheus/client/counter_spec.rb | 6 ++++ spec/prometheus/client/gauge_spec.rb | 6 ++++ spec/prometheus/client/histogram_spec.rb | 6 ++++ spec/prometheus/client/summary_spec.rb | 6 ++++ 7 files changed, 84 insertions(+) diff --git a/README.md b/README.md index b3f2e167..0df41d1c 100644 --- a/README.md +++ b/README.md @@ -214,7 +214,47 @@ https_requests_total = Counter.new(:http_requests_total, https_requests_total.increment(labels: { status_code: response.status_code }) ``` +### `with_labels` +Similar to pre-setting labels, you can get a new instance of an existing metric object, +with a subset (or full set) of labels set, so that you can increment / observe the metric +without having to specify the labels for every call. + +Moreover, if all the labels the metric can take have been pre-set, validation of the labels +is done on the call to `with_labels`, and then skipped for each observation, which can +lead to performance improvements. If you are incrementing a counter in a fast loop, you +definitely want to be doing this. + + +Examples: + +**Pre-setting labels for ease of use:** + +```ruby +# in the file where you define your metrics: +records_processed_total = registry.counter.new(:records_processed_total, + docstring: '...', + labels: [:service, :component], + preset_labels: { service: "my_service" }) + +# in one-off calls, you'd specify the missing labels (component in this case) +records_processed_total.increment(labels: { component: 'a_component' }) + +# you can also have a "view" on this metric for a specific component where this label is +# pre-set: +class MyComponent + def metric + @metric ||= records_processed_total.with_labels(component: "my_component") + end + + def process + records.each do |record| + # process the record + metric.increment + end + end +end +``` ## Data Stores diff --git a/lib/prometheus/client/histogram.rb b/lib/prometheus/client/histogram.rb index 37f83e56..893282ef 100644 --- a/lib/prometheus/client/histogram.rb +++ b/lib/prometheus/client/histogram.rb @@ -33,6 +33,15 @@ def initialize(name, store_settings: store_settings) end + def with_labels(labels) + self.class.new(name, + docstring: docstring, + labels: @labels, + preset_labels: preset_labels.merge(labels), + buckets: @buckets, + store_settings: @store_settings) + end + def type :histogram end diff --git a/lib/prometheus/client/metric.rb b/lib/prometheus/client/metric.rb index d89069f1..45ed5700 100644 --- a/lib/prometheus/client/metric.rb +++ b/lib/prometheus/client/metric.rb @@ -22,6 +22,9 @@ def initialize(name, @validator.valid?(labels) @validator.valid?(preset_labels) + @labels = labels + @store_settings = store_settings + @name = name @docstring = docstring @preset_labels = preset_labels @@ -39,6 +42,14 @@ def get(labels: {}) @store.get(labels: label_set) end + def with_labels(labels) + self.class.new(name, + docstring: docstring, + labels: @labels, + preset_labels: preset_labels.merge(labels), + store_settings: @store_settings) + end + # Returns all label sets with their values def values @store.all_values diff --git a/spec/prometheus/client/counter_spec.rb b/spec/prometheus/client/counter_spec.rb index 29164cac..806c55b6 100644 --- a/spec/prometheus/client/counter_spec.rb +++ b/spec/prometheus/client/counter_spec.rb @@ -45,6 +45,12 @@ end.to change { counter.get(labels: { test: 'label' }) }.by(1.0) end.to_not change { counter.get(labels: { test: 'other' }) } end + + it 'can pre-set labels using `with_labels`' do + expect { counter.increment } + .to raise_error(Prometheus::Client::LabelSetValidator::InvalidLabelSetError) + expect { counter.with_labels(test: 'label').increment }.not_to raise_error + end end it 'increments the counter by a given value' do diff --git a/spec/prometheus/client/gauge_spec.rb b/spec/prometheus/client/gauge_spec.rb index d8518956..9476272f 100644 --- a/spec/prometheus/client/gauge_spec.rb +++ b/spec/prometheus/client/gauge_spec.rb @@ -45,6 +45,12 @@ end.to change { gauge.get(labels: { test: 'value' }) }.from(0).to(42) end.to_not change { gauge.get(labels: { test: 'other' }) } end + + it 'can pre-set labels using `with_labels`' do + expect { gauge.set(10) } + .to raise_error(Prometheus::Client::LabelSetValidator::InvalidLabelSetError) + expect { gauge.with_labels(test: 'value').set(10) }.not_to raise_error + end end context 'given an invalid value' do diff --git a/spec/prometheus/client/histogram_spec.rb b/spec/prometheus/client/histogram_spec.rb index 85327f7d..c64c6218 100644 --- a/spec/prometheus/client/histogram_spec.rb +++ b/spec/prometheus/client/histogram_spec.rb @@ -66,6 +66,12 @@ end.to change { histogram.get(labels: { test: 'value' }) } end.to_not change { histogram.get(labels: { test: 'other' }) } end + + it 'can pre-set labels using `with_labels`' do + expect { histogram.observe(2) } + .to raise_error(Prometheus::Client::LabelSetValidator::InvalidLabelSetError) + expect { histogram.with_labels(test: 'value').observe(2) }.not_to raise_error + end end end diff --git a/spec/prometheus/client/summary_spec.rb b/spec/prometheus/client/summary_spec.rb index e01fa632..c69f754f 100644 --- a/spec/prometheus/client/summary_spec.rb +++ b/spec/prometheus/client/summary_spec.rb @@ -61,6 +61,12 @@ end.to change { summary.get(labels: { test: 'value' })["count"] } end.to_not change { summary.get(labels: { test: 'other' })["count"] } end + + it 'can pre-set labels using `with_labels`' do + expect { summary.observe(2) } + .to raise_error(Prometheus::Client::LabelSetValidator::InvalidLabelSetError) + expect { summary.with_labels(test: 'value').observe(2) }.not_to raise_error + end end end From 3862c1cbd92072272f677c9e914ff7f9a7b16f4d Mon Sep 17 00:00:00 2001 From: Daniel Magliola Date: Tue, 16 Oct 2018 15:54:25 +0100 Subject: [PATCH 15/18] Short-circuit label validation on Metric observations Every time a metric gets observed / incremented, we are composing the labels hash, merging the labels that were just passed in as part of the observation with the metric's `preset_labels`, and validating that the labelset is valid. If all labels are pre-set already, however (either as `preset_labels` when declaring the metric, or by use of `with_labels`), we can validate when the labels are pre-set, and then skip the validation on every observation, which can lead to some decent time savings if a metric is observed often. Signed-off-by: Daniel Magliola --- lib/prometheus/client/metric.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/prometheus/client/metric.rb b/lib/prometheus/client/metric.rb index 45ed5700..00c47ee0 100644 --- a/lib/prometheus/client/metric.rb +++ b/lib/prometheus/client/metric.rb @@ -34,6 +34,11 @@ def initialize(name, metric_type: type, metric_settings: store_settings ) + + if preset_labels.keys.length == labels.length + @validator.validate(preset_labels) + @all_labels_preset = true + end end # Returns the value for the given label set @@ -78,6 +83,8 @@ def validate_docstring(docstring) end def label_set_for(labels) + # We've already validated, and there's nothing to merge. Save some cycles + return preset_labels if @all_labels_preset && labels.empty? @validator.validate(preset_labels.merge(labels)) end end From 2347e2aea1678dbbf57424806eb5e5df15571a0a Mon Sep 17 00:00:00 2001 From: Daniel Magliola Date: Tue, 16 Oct 2018 16:14:17 +0100 Subject: [PATCH 16/18] Rename confusing methods in `LabelSetValidator` LabelSetValidator has methods `valid?` and `validate`. It's not very clear what these do, what's the difference between them, and the name `valid?` would imply (by convention) that it returns a Boolean, rather than raising an exception on invalid input. Renamed these to `validate_symbols!` and `validate_labelset!`, to make it clearer what each of them do. The `!` also follows the usual Ruby convention of "do X, and if that doesn't work, Raise" This commit also starts drawing the distinction between an array of `labels`, and a hash of label keys and values (which we call a `labelset`). The term `labels` is used interchangeably for both concepts throughout the code. This commit doesn't fix all instances, but it's a step in that direction. Signed-off-by: Daniel Magliola --- lib/prometheus/client/label_set_validator.rb | 18 ++++++------ lib/prometheus/client/metric.rb | 8 +++--- .../client/label_set_validator_spec.rb | 28 +++++++++---------- 3 files changed, 27 insertions(+), 27 deletions(-) diff --git a/lib/prometheus/client/label_set_validator.rb b/lib/prometheus/client/label_set_validator.rb index 943f9a77..aae97e03 100644 --- a/lib/prometheus/client/label_set_validator.rb +++ b/lib/prometheus/client/label_set_validator.rb @@ -21,7 +21,7 @@ def initialize(expected_labels:, reserved_labels: []) @validated = {} end - def valid?(labels) + def validate_symbols!(labels) unless labels.respond_to?(:all?) raise InvalidLabelSetError, "#{labels} is not a valid label set" end @@ -33,24 +33,24 @@ def valid?(labels) end end - def validate(labels) - return labels if @validated.key?(labels.hash) + def validate_labelset!(labelset) + return labelset if @validated.key?(labelset.hash) - valid?(labels) + validate_symbols!(labelset) - unless keys_match?(labels) + unless keys_match?(labelset) raise InvalidLabelSetError, "labels must have the same signature " \ - "(keys given: #{labels.keys.sort} vs." \ + "(keys given: #{labelset.keys.sort} vs." \ " keys expected: #{expected_labels}" end - @validated[labels.hash] = labels + @validated[labelset.hash] = labelset end private - def keys_match?(labels) - labels.keys.sort == expected_labels + def keys_match?(labelset) + labelset.keys.sort == expected_labels end def validate_symbol(key) diff --git a/lib/prometheus/client/metric.rb b/lib/prometheus/client/metric.rb index 00c47ee0..1bb43347 100644 --- a/lib/prometheus/client/metric.rb +++ b/lib/prometheus/client/metric.rb @@ -19,8 +19,8 @@ def initialize(name, validate_docstring(docstring) @validator = LabelSetValidator.new(expected_labels: labels, reserved_labels: reserved_labels) - @validator.valid?(labels) - @validator.valid?(preset_labels) + @validator.validate_symbols!(labels) + @validator.validate_symbols!(preset_labels) @labels = labels @store_settings = store_settings @@ -36,7 +36,7 @@ def initialize(name, ) if preset_labels.keys.length == labels.length - @validator.validate(preset_labels) + @validator.validate_labelset!(preset_labels) @all_labels_preset = true end end @@ -85,7 +85,7 @@ def validate_docstring(docstring) def label_set_for(labels) # We've already validated, and there's nothing to merge. Save some cycles return preset_labels if @all_labels_preset && labels.empty? - @validator.validate(preset_labels.merge(labels)) + @validator.validate_labelset!(preset_labels.merge(labels)) end end end diff --git a/spec/prometheus/client/label_set_validator_spec.rb b/spec/prometheus/client/label_set_validator_spec.rb index b6f959d5..770b9d38 100644 --- a/spec/prometheus/client/label_set_validator_spec.rb +++ b/spec/prometheus/client/label_set_validator_spec.rb @@ -13,67 +13,67 @@ end end - describe '#valid?' do + describe '#validate_symbols!' do it 'returns true for a valid label check' do - expect(validator.valid?(version: 'alpha')).to eql(true) + expect(validator.validate_symbols!(version: 'alpha')).to eql(true) end it 'raises Invaliddescribed_classError if a label set is not a hash' do expect do - validator.valid?('invalid') + validator.validate_symbols!('invalid') end.to raise_exception invalid end it 'raises InvalidLabelError if a label key is not a symbol' do expect do - validator.valid?('key' => 'value') + validator.validate_symbols!('key' => 'value') end.to raise_exception(described_class::InvalidLabelError) end it 'raises InvalidLabelError if a label key starts with __' do expect do - validator.valid?(__reserved__: 'key') + validator.validate_symbols!(__reserved__: 'key') end.to raise_exception(described_class::ReservedLabelError) end it 'raises ReservedLabelError if a label key is reserved' do [:job, :instance].each do |label| expect do - validator.valid?(label => 'value') + validator.validate_symbols!(label => 'value') end.to raise_exception(described_class::ReservedLabelError) end end end - describe '#validate' do + describe '#validate_labelset!' do let(:expected_labels) { [:method, :code] } it 'returns a given valid label set' do hash = { method: 'get', code: '200' } - expect(validator.validate(hash)).to eql(hash) + expect(validator.validate_labelset!(hash)).to eql(hash) end - it 'raises an exception if a given label set is not `valid?`' do + it 'raises an exception if a given label set is not `validate_symbols!`' do input = 'broken' - expect(validator).to receive(:valid?).with(input).and_raise(invalid) + expect(validator).to receive(:validate_symbols!).with(input).and_raise(invalid) - expect { validator.validate(input) }.to raise_exception(invalid) + expect { validator.validate_labelset!(input) }.to raise_exception(invalid) end it 'raises an exception if there are unexpected labels' do expect do - validator.validate(method: 'get', code: '200', exception: 'NoMethodError') + validator.validate_labelset!(method: 'get', code: '200', exception: 'NoMethodError') end.to raise_exception(invalid, /keys given: \[:code, :exception, :method\] vs. keys expected: \[:code, :method\]/) end it 'raises an exception if there are missing labels' do expect do - validator.validate(method: 'get') + validator.validate_labelset!(method: 'get') end.to raise_exception(invalid, /keys given: \[:method\] vs. keys expected: \[:code, :method\]/) expect do - validator.validate(code: '200') + validator.validate_labelset!(code: '200') end.to raise_exception(invalid, /keys given: \[:code\] vs. keys expected: \[:code, :method\]/) end end From 5cae3b47ead3a050efba7089db8ac9fd964f8237 Mon Sep 17 00:00:00 2001 From: Daniel Magliola Date: Tue, 13 Nov 2018 18:35:07 +0000 Subject: [PATCH 17/18] Add Performance Benchmarks These benchmarks are useful to know what kind of performance to expect from metrics, in scenarios where consumers may be sensitive to performance, and also to help teams that need to build their own stores, to benchmark them and test them. Signed-off-by: Daniel Magliola --- lib/prometheus/client/data_stores/README.md | 21 ++ prometheus-client.gemspec | 2 + spec/benchmarks/README.md | 67 ++++ spec/benchmarks/data_stores.rb | 329 ++++++++++++++++++++ spec/benchmarks/labels.rb | 127 ++++++++ 5 files changed, 546 insertions(+) create mode 100644 spec/benchmarks/README.md create mode 100644 spec/benchmarks/data_stores.rb create mode 100644 spec/benchmarks/labels.rb diff --git a/lib/prometheus/client/data_stores/README.md b/lib/prometheus/client/data_stores/README.md index e4243839..72250958 100644 --- a/lib/prometheus/client/data_stores/README.md +++ b/lib/prometheus/client/data_stores/README.md @@ -169,6 +169,27 @@ The tests for `DirectFileStore` have a good example at the top of the file. This has some examples on testing multi-process stores, checking that aggregation between processes works correctly. +## Benchmarking your custom data store + +If you are developing your own data store, you probably want to benchmark it to see how +it compares to the built-in ones, and to make sure it achieves the performance you want. + +The Prometheus Ruby Client includes some benchmarks (in the `spec/benchmarks` directory) +to help you with this, and also with validating that your store works correctly. + +The `README` in that directory contains more information what these benchmarks are for, +and how to use them. + +## Extra Stores and Research + +In the process of abstracting stores away, and creating the built-in ones, GoCardless +has created a good amount of research, benchmarks, and experimental stores, which +weren't useful to include in this repo, but may be a useful resource or starting point +if you are building your own store. + +Check out the [GoCardless Data Stores Experiments](gocardless/prometheus-client-ruby-data-stores-experiments) +repository for these. + ## Sample, imaginary multi-process Data Store This is just an example of how one could implement a data store, and a clarification on diff --git a/prometheus-client.gemspec b/prometheus-client.gemspec index 9642a7d0..434f5651 100644 --- a/prometheus-client.gemspec +++ b/prometheus-client.gemspec @@ -16,4 +16,6 @@ Gem::Specification.new do |s| s.require_paths = ['lib'] s.add_dependency 'concurrent-ruby' + + s.add_development_dependency 'benchmark-ips' end diff --git a/spec/benchmarks/README.md b/spec/benchmarks/README.md new file mode 100644 index 00000000..5d9d96fc --- /dev/null +++ b/spec/benchmarks/README.md @@ -0,0 +1,67 @@ +# Performance Benchmarks + +The intention behind these benchmarks is twofold: + +- On the one hand, if you have performance concerns for your counters, they'll allow you + to simulate a reasonably realistic scenario, with your particular runtime characteristics, + so you can know what kind of performance to expect under different circumstances, and pick + settings accordingly. + +- On the other hand, if you are developing your own Custom Data Store (more on this in + `/lib/prometheus/client/data_stores/README.md), this will allow you to test how it + performs compared to the built-in ones, and also "system test" it to validate that it + behaves appropriately. + +## Benchmarks included + +### Data Stores Performance + +The Prometheus Ruby Client ships with different built-in data stores, optimized for +different common scenarios (more on this on the repo's main README, under `Data Stores`). + +This benchmark can show you, for your particular runtime environment, what kind of +performance you can expect from each, to pick the one that's best for you. + +More importantly, in a case where the built-in stores may not be useful for your +particular circumstances, you might want to make your own Data Store. If that is the case, +this benchmark will help you compare its performance characteristics to the built-in +stores, and will also run an export after the observations are made, and compare it with +the built-in ones, helping you catch potential bugs in your store, if the output doesn't +match. + +The benchmark was made to try and simulate a somewhat realistic scenario, with plenty of +high-cardinality metrics, which is what you should be aiming for. It has a balance of +counters and histograms, different label counts for different metrics, different thread +counts, etc. All this should be easy to customize to your particular needs by modifying +the constants in the benchmark to tailor to what you need to measure. + +In particular, if going for the goal of "how long it should take to increment a counter", +you probably want to have no labels and no histograms, since that's the reference +performance measurement we use. + +### Labels Performance + +Adding labels to your metrics can have significant performance impact, on two fronts: + +- Labels passed in on every observation need to be validated. This may be alleviated by + using `with_labels`. If used to pre-set *all* labels, you can save a good + amount of processing time, by skipping validation on each observation. This may be + important if you're incrementing metrics on a tight loop, and this benchmark can help + with establishing what's to be expected. + +- Even when caching them, these labels are keys to Hashes, they need to sometimes be + serialized into strings, sometimes merged into other hashes. All this incurs performance + costs. This benchmark will allow you to estimate how much impact they can have. Again, + if incrementing metrics on a tight loop, this will let you estimate whether you might + want to have fewer labels instead. + +It should be easy to modify the constants in this benchmark to your particular situation, +if necessary. + +## Running the benchmarks + +Simply run, from the repo's root directory: + +`bundle exec ruby spec/benchmarks/labels.rb` +`bundle exec ruby spec/benchmarks/data_stores.rb` + diff --git a/spec/benchmarks/data_stores.rb b/spec/benchmarks/data_stores.rb new file mode 100644 index 00000000..940d38a1 --- /dev/null +++ b/spec/benchmarks/data_stores.rb @@ -0,0 +1,329 @@ +require 'benchmark' +require 'prometheus/client' +require 'prometheus/client/counter' +require 'prometheus/client/histogram' +require 'prometheus/client/formats/text' +require 'prometheus/client/data_stores/single_threaded' +require 'prometheus/client/data_stores/synchronized' +require 'prometheus/client/data_stores/direct_file_store' + +# Compare the time it takes different stores to observe a large number of data points, in +# a multi-threaded environment. +# +# If you create a new store and want to benchmark it, add it to the `STORES` array, +# and run the benchmark to see how it compares to the other options. +# +# Each test instantiates a number of Histograms and Counters, with a random number of +# labels, instantiates a number of threads, and then prepares a a large number of +# observations, which it distributes randomly between the different metrics and threads +# created. +# +# It does this for each of the STORES specified and different THREAD_COUNTS, then once +# all that is ready, it starts the benchmark test and lets the threads run to observe +# those data points. +# +# In addition to timing the observation of data points, the benchmark also runs the Text +# Exporter on the results, and compares them between stores to make sure all stores +# result in the same output being generated. If this output doesn't match exactly, +# something is going wrong, and it probably indicates a bug in the store, so this +# benchmark also acts as a sort of system test for stores. If a mismatch is found, a +# WARNING will show up in the output, and both the expected and actual results will be +# dumped to text files, for help in debugging. +# +# Data generation involves randomness, but the RNG is seeded so that different stores are +# exposed to the same pattern of access (as long as two test cases have the same number +# of threads), reducing the effects on the result of randomness in lock contention. +# +# NOTE: If you leave the default of 1_000_000 DATA_POINTS, then the timing result is +# showing "microseconds per observation", which is the unit we care about. +# We're aiming for 1 microsecond per observation, which is not quite achievable in Ruby, +# but that's what we're trying to approach. If you're trying to compare against this +# goal, set NUM_HISTOGRAMS and MAX_LABELS to 0, for a fair comparison, as both labels +# and histograms are much slower than label-less counters. +#----------------------------------------------------------------------------------- + +# Store class that follows the required interface but does nothing. Used as a baseline +# of how much time is spent outside the store. +class NoopStore + def for_metric(metric_name, metric_type:, metric_settings: {}) + MetricStore.new + end + + class MetricStore + def synchronize + yield + end + + def set(labels:, val:); end + def increment(labels:, by: 1); end + def get(labels:); end + def all_values; {}; end + end +end + +#----------------------------------------------------------------------------------- + +RANDOM_SEED = 12345678 +NUM_COUNTERS = 80 +NUM_HISTOGRAMS = 20 +DATA_POINTS = 1_000_000 +MIN_LABELS = 0 +MAX_LABELS = 4 +THREAD_COUNTS = [1, 2, 4, 8, 12, 16, 20] + +TMP_DIR = "/tmp/prometheus_benchmark" + +STORES = [ + { store: NoopStore.new }, + { store: Prometheus::Client::DataStores::SingleThreaded.new, max_threads: 1 }, + { store: Prometheus::Client::DataStores::Synchronized.new }, + { + store: Prometheus::Client::DataStores::DirectFileStore.new(dir: TMP_DIR), + before: -> () { cleanup_dir(TMP_DIR) }, + } +] + +#----------------------------------------------------------------------------------- + +class TestSetup + attr_reader :random, :num_threads, :registry + attr_reader :metrics, :threads # Simple arrays + attr_reader :data_points # Hash, indexed by Thread ID, with an array of points to observe + attr_reader :start_event + + def initialize(store, num_threads) + Prometheus::Client.config.data_store = @store = store + + @random = Random.new(RANDOM_SEED) # Repeatable random numbers for each test + @start_event = Concurrent::Event.new # Event all threads wait on to start, once set up + @num_threads = num_threads + @threads = [] + @metrics = [] + @data_points = {} + @registry = Prometheus::Client::Registry.new + + setup_threads + setup_metrics + create_datapoints + end + + def observe! + start_event.set # Release the threads to process their events + threads.each { |thr| thr.join } # Wait for all threads to finish and die + end + + def export!(expected_output) + output = Prometheus::Client::Formats::Text.marshal(registry) + + # Output validation doesn't work for NoopStore + return nil if @store.is_a?(NoopStore) + + puts "\nWARNING: Empty output" if !output || output.empty? + + # If this is the first store to run for this number of threads, store expected_output + return output if expected_output.nil? + + # Otherwise, make sure this store's output was the same as the previous one. + # If it isn't, there's probably a bug in the store + return output if output == expected_output + + # Outputs don't match. Report + expected_filename = "data_mismatch_#{ @store.class.name }_#{ num_threads }thr_expected.txt" + actual_filename = "data_mismatch_#{ @store.class.name }_#{ num_threads }thr_actual.txt" + puts "\nWARNING: Output Mismatch.\nSee #{ expected_filename }\nand #{ actual_filename }" + + File.open(expected_filename, 'w') {|f| f.write(expected_output) } + File.open(actual_filename, 'w') {|f| f.write(output) } + + return expected_output + end + + private + + def setup_threads + latch = Concurrent::CountDownLatch.new(num_threads) + + num_threads.times do |i| + threads << Thread.new(i) do |thread_id| + latch.count_down + start_event.wait # Wait for the test to start + thread_run(thread_id) # Process this thread's events + end + end + + latch.wait # Wait for all threads to have started + end + + def setup_metrics + NUM_COUNTERS.times do |i| + labelset = generate_labelset + counter = Prometheus::Client::Counter.new( + "counter#{ i }".to_sym, + docstring: "Counter #{ i }", + labels: labelset.keys, + preset_labels: labelset + ) + + metrics << counter + end + + NUM_HISTOGRAMS.times do |i| + labelset = generate_labelset + histogram = Prometheus::Client::Histogram.new( + "histogram#{ i }".to_sym, + docstring: "Histogram #{ i }", + labels: labelset.keys, + preset_labels: labelset + ) + + metrics << histogram + end + + metrics.each { |metric| registry.register(metric) } + end + + def create_datapoints + num_threads.times do |i| + data_points[i] = [] + end + + thread_id = 0 + DATA_POINTS.times do |i| + thread_id = (thread_id + 1) % num_threads + metric = random_metric + + if metric.type == :counter + data_points[thread_id] << [metric] + else + data_points[thread_id] << [metric, random.rand * 10] + end + end + end + + def thread_run(thread_id) + thread_points = data_points[thread_id] + thread_points.each do |point| + metric = point[0] + if metric.type == :counter + metric.increment + else + metric.observe(point[1]) + end + end + end + + def generate_labelset + num_labels = random.rand(MAX_LABELS - MIN_LABELS + 1) + MIN_LABELS + (1..num_labels).map {|j| ["label#{ j }".to_sym, "foo"] }.to_h + end + + def random_metric + metrics[random.rand(metrics.count)] + end +end + +def cleanup_dir(dir) + Dir.glob("#{ dir }/*").each { |file| File.delete(file) } +end + +#----------------------------------------------------------------------------------- + +# Monkey-patch the exporter to round Float numbers +# This is necessary in order to compare outputs from different stores, and make sure +# the user-built stores are working correctly. +# +# In multi-threaded scenarios, adding up a large amount of floats in different orders +# results in small rounding errors when adding the same numbers. This is not a bug +# in the store, or anywhere, it's the nature of Floats. +# E.g.: 4909.026018536727 +# vs 4909.026018536722 +# +# In the real exporter, this is not a problem, because the exported numbers are still +# correct, but when comparing one to the other, these tiny deltas result in false +# alarms for *all* stores under multiple threads. +# +# Monkey-patching the output line to round the number allows us to compare these outputs +# without any noticeable downside. +module Prometheus + module Client + module Formats + module Text + def self.metric(name, labels, value) + format(METRIC_LINE, name, labels, value.round(6)) + end + end + end + end +end + +#----------------------------------------------------------------------------------- + +Benchmark.bm(45) do |bm| + THREAD_COUNTS.each do |num_threads| + expected_exporter_output = nil + + STORES.each do |store_test| + # Single Threaded stores can't run in multiple threads + next if store_test[:max_threads] && num_threads > store_test[:max_threads] + + # Cleanup before test + store_test[:before].call if store_test[:before] + + test_setup = TestSetup.new(store_test[:store], num_threads) + store_name = store_test[:store].class.name.split('::').last + test_name ="#{ (store_test[:name] || store_name).ljust(25) } x#{ num_threads }" + + bm.report("Observe #{test_name}") { test_setup.observe! } + bm.report("Export #{test_name}") do + expected_exporter_output = test_setup.export!(expected_exporter_output) + end + end + + puts "-" * 80 + end +end + + +#-------------------------------------------------------------------------------------- +# Sample Results: +# +# Only counters, no labels, DirectFileStore stored in TMPFS, Ruby 2.5.1 +# ---------------------------------------------------------------- +# user system total real +# Observe NoopStore x1 0.390845 0.019915 0.410760 ( 0.413240) +# Export NoopStore x1 0.000462 0.000029 0.000491 ( 0.000489) +# Observe SingleThreaded x1 0.946516 0.044122 0.990638 ( 0.990801) +# Export SingleThreaded x1 0.000837 0.000000 0.000837 ( 0.000838) +# Observe Synchronized x1 4.038891 0.000000 4.038891 ( 4.039304) +# Export Synchronized x1 0.001227 0.000000 0.001227 ( 0.001229) +# Observe DirectFileStore x1 7.414242 1.732539 9.146781 ( 9.147389) +# Export DirectFileStore x1 0.009920 0.000243 0.010163 ( 0.010170) +# -------------------------------------------------------------------------------- +# Observe NoopStore x2 0.337919 0.000000 0.337919 ( 0.337575) +# Export NoopStore x2 0.000404 0.000000 0.000404 ( 0.000379) +# Observe Synchronized x2 4.313595 0.008714 4.322309 ( 4.314901) +# Export Synchronized x2 0.001649 0.000155 0.001804 ( 0.001809) +# Observe DirectFileStore x2 22.193105 12.739370 34.932475 ( 21.503215) +# Export DirectFileStore x2 0.005982 0.008480 0.014462 ( 0.014471) +# +# +# +# Default benchmark (Mix of Counters and Histograms, and up to 4 labels), +# DirectFileStore stored in TMPFS, Ruby 2.5.1 +# ------------------------------------------ +# user system total real +# Observe NoopStore x1 0.994314 0.027816 1.022130 ( 1.025121) +# Export NoopStore x1 0.000537 0.000032 0.000569 ( 0.000574) +# Observe SingleThreaded x1 4.439427 0.027929 4.467356 ( 4.470777) +# Export SingleThreaded x1 0.006244 0.000000 0.006244 ( 0.006250) +# Observe Synchronized x1 8.292962 0.000000 8.292962 ( 8.293737) +# Export Synchronized x1 0.006698 0.000000 0.006698 ( 0.006706) +# Observe DirectFileStore x1 13.448161 2.517563 15.965724 ( 15.967281) +# Export DirectFileStore x1 0.020115 0.004012 0.024127 ( 0.024135) +# -------------------------------------------------------------------------------- +# Observe NoopStore x2 1.342963 0.020541 1.363504 ( 1.354383) +# Export NoopStore x2 0.002923 0.000000 0.002923 ( 0.002927) +# Observe Synchronized x2 8.810914 0.029352 8.840266 ( 8.828600) +# Export Synchronized x2 0.007535 0.000000 0.007535 ( 0.007540) +# Observe DirectFileStore x2 41.483649 19.362639 60.846288 ( 39.026703) +# Export DirectFileStore x2 0.010133 0.013159 0.023292 ( 0.023302) diff --git a/spec/benchmarks/labels.rb b/spec/benchmarks/labels.rb new file mode 100644 index 00000000..42f8ebda --- /dev/null +++ b/spec/benchmarks/labels.rb @@ -0,0 +1,127 @@ +require 'benchmark/ips' +require 'prometheus/client' +require 'prometheus/client/counter' +require 'prometheus/client/data_stores/single_threaded' + +# Compare the time it takes to observe metrics that have labels (disregarding the actual +# data store) +# +# This benchmark compares 3 different metrics, with 0, 2 and 100 labels respectively, +# and how using `with_values` for some, or all their label values affects performance. +# +# The hypothesis here is that, once labels are introduced, we're validating those labels +# in every observation, but if those labels are "cached" using `with_labels`, we skip that +# validation which should be *considerably* faster. +# +# This completely disregards the storage of this data in memory, and it's highly likely +# that more labels will make things slower in the data store, even if the metrics themselves +# don't add overhead. So the fact that using `with_labels` with all labels adds no overhead +# to the metric itself doesn't mean labels have no overhead. +# +# To see what it looks like with the best-case scenario data store, uncomment the line +# that sets the `data_store` to `SingleThreaded` +#------------------------------------------------------------------------------------- +# Store that doesn't do anything, so we can focus as much as possible on the timings of +# the Metric itself +class NoopStore + def for_metric(metric_name, metric_type:, metric_settings: {}) + MetricStore.new + end + + class MetricStore + def synchronize + yield + end + + def set(labels:, val:); end + def increment(labels:, by: 1); end + def get(labels:); end + def all_values; end + end +end + +Prometheus::Client.config.data_store = NoopStore.new # No data storage +# Prometheus::Client.config.data_store = Prometheus::Client::DataStores::SingleThreaded.new # Simple data storage + +#------------------------------------------------------------------------------------- +# Set up of the 3 metrics, plus their half-cached and full-cached versions +NO_LABELS_COUNTER = Prometheus::Client::Counter.new( + :no_labels, + docstring: "Counter with no labels" +) + +TWO_LABELSET = { label1: "a", label2: "b"} +LAST_ONE_LABELSET = { label2: "b"} +TWO_LABELS_COUNTER = Prometheus::Client::Counter.new( + :two_labels, + docstring: "Counter with 2 labels", + labels: [:label1, :label2] +) +TWO_LABELS_ONE_CACHED = TWO_LABELS_COUNTER.with_labels(label1: "a") +TWO_LABELS_ALL_CACHED = TWO_LABELS_COUNTER.with_labels(label1: "a", label2: "b") + + +HUNDRED_LABELS = (1..100).map{|i| "label#{ i }".to_sym } +HUNDRED_LABELSET = (1..100).map{|i| ["label#{ i }".to_sym, i.to_s] }.to_h +FIRST_FIFTY_LABELSET = (1..50).map{|i| ["label#{ i }".to_sym, i.to_s] }.to_h +LAST_FIFTY_LABELSET = (51..100).map{|i| ["label#{ i }".to_sym, i.to_s] }.to_h + +HUNDRED_LABELS_COUNTER = Prometheus::Client::Counter.new( + :hundred_labels, + docstring: "Counter with 100 labels", + labels: HUNDRED_LABELS +) +HUNDRED_LABELS_HALF_CACHED = HUNDRED_LABELS_COUNTER.with_labels(FIRST_FIFTY_LABELSET) +HUNDRED_LABELS_ALL_CACHED = HUNDRED_LABELS_COUNTER.with_labels(HUNDRED_LABELSET) + +#------------------------------------------------------------------------------------- +# Actual Benchmark + +Benchmark.ips do |x| + x.config(:time => 5, :warmup => 2) + + x.report("0 labels") { NO_LABELS_COUNTER.increment } + x.report("2 labels") { TWO_LABELS_COUNTER.increment(labels: TWO_LABELSET) } + x.report("100 labels") { HUNDRED_LABELS_COUNTER.increment(labels: HUNDRED_LABELSET) } + + x.report("2 lab, half cached") { TWO_LABELS_ONE_CACHED.increment(labels: LAST_ONE_LABELSET) } + x.report("100 lab, half cached") { HUNDRED_LABELS_HALF_CACHED.increment(labels: LAST_FIFTY_LABELSET) } + + x.report("2 lab, all cached") { TWO_LABELS_ALL_CACHED.increment } + x.report("100 lab, all cached") { HUNDRED_LABELS_ALL_CACHED.increment } +end + +#------------------------------------------------------------------------------------- +# Conclusion: +# +# Without a data store: +# +# 0 labels 3.592M (± 3.7%) i/s - 18.081M in 5.039832s +# 2 labels 502.898k (± 3.2%) i/s - 2.536M in 5.048618s +# 100 labels 19.467k (± 4.8%) i/s - 98.280k in 5.061444s +# 2 lab, half cached 432.844k (± 3.0%) i/s - 2.180M in 5.041123s +# 100 lab, half cached 20.444k (± 3.4%) i/s - 103.636k in 5.075070s +# 2 lab, all cached 3.668M (± 3.3%) i/s - 18.338M in 5.004442s +# 100 lab, all cached 3.711M (± 4.0%) i/s - 18.544M in 5.005362s +# +# As we expected, labels introduce a significant overhead, even in small numbers, but +# if they are all pre-set, the effect is negligible. +# Pre-setting *some* labels, however, has no performance impact. It may still be desirable +# to avoid repetition, though. +# +# So, if observing measurements in a tight loop, it's highly recommended to use `with_labels` +# and pre-set all labels. +# +# +# With the simplest possible data store: +# +# 0 labels 1.275M (± 3.1%) i/s - 6.419M in 5.038946s +# 2 labels 195.293k (± 4.3%) i/s - 974.600k in 5.000375s +# 100 labels 6.410k (± 7.5%) i/s - 32.022k in 5.028417s +# 2 lab, half cached 187.255k (± 3.5%) i/s - 948.618k in 5.072189s +# 100 lab, half cached 6.846k (± 2.7%) i/s - 34.424k in 5.031776s +# 2 lab, all cached 376.353k (± 3.3%) i/s - 1.890M in 5.025963s +# 100 lab, all cached 11.669k (± 3.0%) i/s - 58.752k in 5.039468s +# +# As mentioned above, once we're storing the data, labels *can* have a serious impact, +# and that impact will be highly store dependent. \ No newline at end of file From 5dce3e5855f2aa67c65874c953878e124c23edcd Mon Sep 17 00:00:00 2001 From: Chris Sinjakli Date: Tue, 12 Mar 2019 20:11:34 +0000 Subject: [PATCH 18/18] Improve wording around example data store implementation Signed-off-by: Chris Sinjakli --- lib/prometheus/client/data_stores/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/prometheus/client/data_stores/README.md b/lib/prometheus/client/data_stores/README.md index 72250958..920e5833 100644 --- a/lib/prometheus/client/data_stores/README.md +++ b/lib/prometheus/client/data_stores/README.md @@ -195,11 +195,11 @@ repository for these. This is just an example of how one could implement a data store, and a clarification on the "aggregation" point -Important: This is **VAPORWARE**, intended simply to show how this could work / how to +Important: This is a **toy example**, intended simply to show how this could work / how to implement these interfaces. There are some key pieces of code missing, which are fairly uninteresting, this only shows -the parts that illustrate the idea of storing multiple different values, and aggregate +the parts that illustrate the idea of storing multiple different values, and aggregating them ```ruby