Skip to content

Commit c8ad029

Browse files
committed
Add tests to with_labels
This reproduces the error reported in issue #225, where `with_labels` doesn't work at all the way it is intended, and in addition to that, if you're using DirectFileStore, it can very easily corrupt your files. The problem is the new metric received after calling `with_labels` is a copy of the original, with the labels pre-set, but it has **its own internal store**, so it doesn't actually impact the values of the original metric, which is what gets exported by the registry. Signed-off-by: Daniel Magliola <dmagliola@crystalgears.com>
1 parent 7d9d45f commit c8ad029

4 files changed

Lines changed: 119 additions & 24 deletions

File tree

spec/prometheus/client/counter_spec.rb

Lines changed: 54 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
require 'prometheus/client'
44
require 'prometheus/client/counter'
55
require 'examples/metric_example'
6+
require 'prometheus/client/data_stores/direct_file_store'
67

78
describe Prometheus::Client::Counter do
89
# Reset the data store
@@ -45,12 +46,6 @@
4546
end.to change { counter.get(labels: { test: 'label' }) }.by(1.0)
4647
end.to_not change { counter.get(labels: { test: 'other' }) }
4748
end
48-
49-
it 'can pre-set labels using `with_labels`' do
50-
expect { counter.increment }
51-
.to raise_error(Prometheus::Client::LabelSetValidator::InvalidLabelSetError)
52-
expect { counter.with_labels(test: 'label').increment }.not_to raise_error
53-
end
5449
end
5550

5651
it 'increments the counter by a given value' do
@@ -122,4 +117,57 @@
122117
end
123118
end
124119
end
120+
121+
describe '#with_labels' do
122+
let(:expected_labels) { [:foo] }
123+
124+
it 'pre-sets labels for observations' do
125+
expect { counter.increment }
126+
.to raise_error(Prometheus::Client::LabelSetValidator::InvalidLabelSetError)
127+
expect { counter.with_labels(foo: 'label').increment }.not_to raise_error
128+
end
129+
130+
it 'registers `with_labels` observations in the original metric store' do
131+
counter.increment(labels: { foo: 'value1'})
132+
counter_with_labels = counter.with_labels({ foo: 'value2'})
133+
counter_with_labels.increment(by: 2)
134+
135+
expect(counter_with_labels.values).to eql({foo: 'value1'} => 1.0, {foo: 'value2'} => 2.0)
136+
expect(counter.values).to eql({foo: 'value1'} => 1.0, {foo: 'value2'} => 2.0)
137+
end
138+
139+
context 'when using DirectFileStore' do
140+
before do
141+
Dir.glob('/tmp/prometheus_test/*').each { |file| File.delete(file) }
142+
Prometheus::Client.config.data_store = Prometheus::Client::DataStores::DirectFileStore.new(dir: '/tmp/prometheus_test')
143+
end
144+
145+
let(:expected_labels) { [:foo, :bar] }
146+
147+
it "doesn't corrupt the data files" do
148+
counter_with_labels = counter.with_labels({ foo: 'longervalue'})
149+
150+
# Initialize / read the files for both views of the metric
151+
counter.increment(labels: { foo: 'value1', bar: 'zzz'})
152+
counter_with_labels.increment(by: 2, labels: {bar: 'zzz'})
153+
154+
# After both MetricStores have their files, add a new entry to both
155+
counter.increment(labels: { foo: 'value1', bar: 'aaa'})
156+
counter_with_labels.increment(by: 2, labels: {bar: 'aaa'})
157+
158+
expect { counter.values }.not_to raise_error # Check it hasn't corrupted our files
159+
expect { counter_with_labels.values }.not_to raise_error # Check it hasn't corrupted our files
160+
161+
expected_values = {
162+
{foo: 'value1', bar: 'zzz'} => 1.0,
163+
{foo: 'value1', bar: 'aaa'} => 1.0,
164+
{foo: 'longervalue', bar: 'zzz'} => 2.0,
165+
{foo: 'longervalue', bar: 'aaa'} => 2.0,
166+
}
167+
168+
expect(counter.values).to eql(expected_values)
169+
expect(counter_with_labels.values).to eql(expected_values)
170+
end
171+
end
172+
end
125173
end

spec/prometheus/client/gauge_spec.rb

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,6 @@
4545
end.to change { gauge.get(labels: { test: 'value' }) }.from(0).to(42)
4646
end.to_not change { gauge.get(labels: { test: 'other' }) }
4747
end
48-
49-
it 'can pre-set labels using `with_labels`' do
50-
expect { gauge.set(10) }
51-
.to raise_error(Prometheus::Client::LabelSetValidator::InvalidLabelSetError)
52-
expect { gauge.with_labels(test: 'value').set(10) }.not_to raise_error
53-
end
5448
end
5549

5650
context 'given an invalid value' do
@@ -204,4 +198,23 @@
204198
end
205199
end
206200
end
201+
202+
describe '#with_labels' do
203+
let(:expected_labels) { [:foo] }
204+
205+
it 'pre-sets labels for observations' do
206+
expect { gauge.set(10) }
207+
.to raise_error(Prometheus::Client::LabelSetValidator::InvalidLabelSetError)
208+
expect { gauge.with_labels(foo: 'value').set(10) }.not_to raise_error
209+
end
210+
211+
it 'registers `with_labels` observations in the original metric store' do
212+
gauge.set(1, labels: { foo: 'value1'})
213+
gauge_with_labels = gauge.with_labels({ foo: 'value2'})
214+
gauge_with_labels.set(2)
215+
216+
expect(gauge_with_labels.values).to eql({foo: 'value1'} => 1.0, {foo: 'value2'} => 2.0)
217+
expect(gauge.values).to eql({foo: 'value1'} => 1.0, {foo: 'value2'} => 2.0)
218+
end
219+
end
207220
end

spec/prometheus/client/histogram_spec.rb

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -80,12 +80,6 @@
8080
end.to change { histogram.get(labels: { test: 'value' }) }
8181
end.to_not change { histogram.get(labels: { test: 'other' }) }
8282
end
83-
84-
it 'can pre-set labels using `with_labels`' do
85-
expect { histogram.observe(2) }
86-
.to raise_error(Prometheus::Client::LabelSetValidator::InvalidLabelSetError)
87-
expect { histogram.with_labels(test: 'value').observe(2) }.not_to raise_error
88-
end
8983
end
9084

9185
context "with non-string label values" do
@@ -189,4 +183,27 @@
189183
end
190184
end
191185
end
186+
187+
describe '#with_labels' do
188+
let(:expected_labels) { [:foo] }
189+
190+
it 'pre-sets labels for observations' do
191+
expect { histogram.observe(2) }
192+
.to raise_error(Prometheus::Client::LabelSetValidator::InvalidLabelSetError)
193+
expect { histogram.with_labels(foo: 'value').observe(2) }.not_to raise_error
194+
end
195+
196+
it 'registers `with_labels` observations in the original metric store' do
197+
histogram.observe(7, labels: { foo: 'value1'})
198+
histogram_with_labels = histogram.with_labels({ foo: 'value2'})
199+
histogram_with_labels.observe(20)
200+
201+
expected_values = {
202+
{foo: 'value1'} => {'2.5' => 0.0, '5' => 0.0, '10' => 1.0, '+Inf' => 1.0, 'sum' => 7.0},
203+
{foo: 'value2'} => {'2.5' => 0.0, '5' => 0.0, '10' => 0.0, '+Inf' => 1.0, 'sum' => 20.0}
204+
}
205+
expect(histogram_with_labels.values).to eql(expected_values)
206+
expect(histogram.values).to eql(expected_values)
207+
end
208+
end
192209
end

spec/prometheus/client/summary_spec.rb

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,6 @@
6161
end.to change { summary.get(labels: { test: 'value' })["count"] }
6262
end.to_not change { summary.get(labels: { test: 'other' })["count"] }
6363
end
64-
65-
it 'can pre-set labels using `with_labels`' do
66-
expect { summary.observe(2) }
67-
.to raise_error(Prometheus::Client::LabelSetValidator::InvalidLabelSetError)
68-
expect { summary.with_labels(test: 'value').observe(2) }.not_to raise_error
69-
end
7064
end
7165

7266
context "with non-string label values" do
@@ -152,4 +146,27 @@
152146
end
153147
end
154148
end
149+
150+
describe '#with_labels' do
151+
let(:expected_labels) { [:foo] }
152+
153+
it 'pre-sets labels for observations' do
154+
expect { summary.observe(2) }
155+
.to raise_error(Prometheus::Client::LabelSetValidator::InvalidLabelSetError)
156+
expect { summary.with_labels(foo: 'value').observe(2) }.not_to raise_error
157+
end
158+
159+
it 'registers `with_labels` observations in the original metric store' do
160+
summary.observe(1, labels: { foo: 'value1'})
161+
summary_with_labels = summary.with_labels({ foo: 'value2'})
162+
summary_with_labels.observe(2)
163+
164+
expected_values = {
165+
{foo: 'value1'} => { 'count' => 1.0, 'sum' => 1.0 },
166+
{foo: 'value2'} => { 'count' => 1.0, 'sum' => 2.0 }
167+
}
168+
expect(summary_with_labels.values).to eql(expected_values)
169+
expect(summary.values).to eql(expected_values)
170+
end
171+
end
155172
end

0 commit comments

Comments
 (0)