From feccff3429cac056913ad7414b8744e61db57d85 Mon Sep 17 00:00:00 2001 From: Daniel Magliola Date: Thu, 24 Dec 2020 18:29:54 +0000 Subject: [PATCH] Automatically initialize time series without labels According to [Prometheus Best Practices](https://prometheus.io/docs/practices/instrumentation/#avoid-missing-metrics), client libraries are expected to automatically export a 0 value when declaring a metric that has no labels. We missed this so far in the Ruby client, this PR rectifies that. NOTE: This can be considered a breaking change. On the one hand, it's a bug fix, but on the other, it will make many time series materialize when scraping apps that use the client, that previously wouldn't have. Depending on how many label-less metrics the app is declaring, this may have a significant impact, so we should probably cut a new major version and warn about this in the Release Notes. Signed-off-by: Daniel Magliola --- lib/prometheus/client/metric.rb | 2 ++ spec/prometheus/client/counter_spec.rb | 18 ++++++++++----- spec/prometheus/client/gauge_spec.rb | 18 ++++++++++----- spec/prometheus/client/histogram_spec.rb | 28 ++++++++++++++++-------- spec/prometheus/client/summary_spec.rb | 28 ++++++++++++++++-------- 5 files changed, 66 insertions(+), 28 deletions(-) diff --git a/lib/prometheus/client/metric.rb b/lib/prometheus/client/metric.rb index 100e455b..c492b08d 100644 --- a/lib/prometheus/client/metric.rb +++ b/lib/prometheus/client/metric.rb @@ -40,6 +40,8 @@ def initialize(name, metric_type: type, metric_settings: store_settings ) + + init_label_set({}) if labels.empty? end # Returns the value for the given label set diff --git a/spec/prometheus/client/counter_spec.rb b/spec/prometheus/client/counter_spec.rb index 856a60b8..338b1cfe 100644 --- a/spec/prometheus/client/counter_spec.rb +++ b/spec/prometheus/client/counter_spec.rb @@ -104,14 +104,22 @@ end describe '#init_label_set' do - let(:expected_labels) { [:test] } + context "with labels" do + let(:expected_labels) { [:test] } - it 'initializes the metric for a given label set' do - expect(counter.values).to eql({}) + it 'initializes the metric for a given label set' do + expect(counter.values).to eql({}) - counter.init_label_set(test: 'value') + counter.init_label_set(test: 'value') - expect(counter.values).to eql({test: 'value'} => 0.0) + expect(counter.values).to eql({test: 'value'} => 0.0) + end + end + + context "without labels" do + it 'automatically initializes the metric' do + expect(counter.values).to eql({} => 0.0) + end end end end diff --git a/spec/prometheus/client/gauge_spec.rb b/spec/prometheus/client/gauge_spec.rb index 37568aa0..318e0552 100644 --- a/spec/prometheus/client/gauge_spec.rb +++ b/spec/prometheus/client/gauge_spec.rb @@ -186,14 +186,22 @@ end describe '#init_label_set' do - let(:expected_labels) { [:test] } + context "with labels" do + let(:expected_labels) { [:test] } - it 'initializes the metric for a given label set' do - expect(gauge.values).to eql({}) + it 'initializes the metric for a given label set' do + expect(gauge.values).to eql({}) - gauge.init_label_set(test: 'value') + gauge.init_label_set(test: 'value') - expect(gauge.values).to eql({test: 'value'} => 0.0) + expect(gauge.values).to eql({test: 'value'} => 0.0) + end + end + + context "without labels" do + it 'automatically initializes the metric' do + expect(gauge.values).to eql({} => 0.0) + end end end end diff --git a/spec/prometheus/client/histogram_spec.rb b/spec/prometheus/client/histogram_spec.rb index 780875f9..b0988b36 100644 --- a/spec/prometheus/client/histogram_spec.rb +++ b/spec/prometheus/client/histogram_spec.rb @@ -165,18 +165,28 @@ end describe '#init_label_set' do - let(:expected_labels) { [:status] } + context "with labels" do + let(:expected_labels) { [:status] } - it 'initializes the metric for a given label set' do - expect(histogram.values).to eql({}) + it 'initializes the metric for a given label set' do + expect(histogram.values).to eql({}) - histogram.init_label_set(status: 'bar') - histogram.init_label_set(status: 'foo') + histogram.init_label_set(status: 'bar') + histogram.init_label_set(status: 'foo') - expect(histogram.values).to eql( - { status: 'bar' } => { "2.5" => 0.0, "5" => 0.0, "10" => 0.0, "+Inf" => 0.0, "sum" => 0.0 }, - { status: 'foo' } => { "2.5" => 0.0, "5" => 0.0, "10" => 0.0, "+Inf" => 0.0, "sum" => 0.0 }, - ) + expect(histogram.values).to eql( + { status: 'bar' } => { "2.5" => 0.0, "5" => 0.0, "10" => 0.0, "+Inf" => 0.0, "sum" => 0.0 }, + { status: 'foo' } => { "2.5" => 0.0, "5" => 0.0, "10" => 0.0, "+Inf" => 0.0, "sum" => 0.0 }, + ) + end + end + + context "without labels" do + it 'automatically initializes the metric' do + expect(histogram.values).to eql( + {} => { "2.5" => 0.0, "5" => 0.0, "10" => 0.0, "+Inf" => 0.0, "sum" => 0.0 }, + ) + end end end end diff --git a/spec/prometheus/client/summary_spec.rb b/spec/prometheus/client/summary_spec.rb index 86883ab9..fbf8f2ce 100644 --- a/spec/prometheus/client/summary_spec.rb +++ b/spec/prometheus/client/summary_spec.rb @@ -128,18 +128,28 @@ end describe '#init_label_set' do - let(:expected_labels) { [:status] } + context "with labels" do + let(:expected_labels) { [:status] } - it 'initializes the metric for a given label set' do - expect(summary.values).to eql({}) + it 'initializes the metric for a given label set' do + expect(summary.values).to eql({}) - summary.init_label_set(status: 'bar') - summary.init_label_set(status: 'foo') + summary.init_label_set(status: 'bar') + summary.init_label_set(status: 'foo') - expect(summary.values).to eql( - { status: 'bar' } => { "count" => 0.0, "sum" => 0.0 }, - { status: 'foo' } => { "count" => 0.0, "sum" => 0.0 }, - ) + expect(summary.values).to eql( + { status: 'bar' } => { "count" => 0.0, "sum" => 0.0 }, + { status: 'foo' } => { "count" => 0.0, "sum" => 0.0 }, + ) + end + end + + context "without labels" do + it 'automatically initializes the metric' do + expect(summary.values).to eql( + {} => { "count" => 0.0, "sum" => 0.0 }, + ) + end end end end