Reject le on histograms and quantile on summaries
What does this MR do and why?
Prometheus::Client::Metric#initialize gives LabelSetValidator the label names it should refuse as strings: ['le'] for a histogram and ['quantile'] for a summary. The validator accepts only symbol keys and checks each one against that list with include?, so neither name ever matched. As the issue says, Histogram#observe({ le: '5' }, 42) and Summary#observe({ quantile: 0.5 }, 42) do not raise: they record a value under that label set. The same keys are accepted as base labels too.
This MR passes the names as symbols (%i[le] and %i[quantile], as the issue suggests), so both calls, and both names as base labels, now raise LabelSetValidator::ReservedLabelError. get checks labels with the same validator, so reading a histogram with le or a summary with quantile raises the same error. Counters and gauges still accept both names, a histogram still accepts quantile and a summary still accepts le.
The histogram spec for this case has been pending since b8a23764 ("Skip tests that are not working"). It expected the ArgumentError that Histogram#observe raised itself until acf06f9b replaced that check with the validator and the string names. I enabled it again and changed it to expect ReservedLabelError, and added specs for the summary and for the two base-label cases.
This is a behavior change for any caller that passes le to a histogram or quantile to a summary today: those calls now raise instead of recording a value. What they recorded already came out inconsistent: a histogram stores each bucket under the caller's labels with le replaced by the bucket's bound, so the caller's le is lost from every bucket series and kept only on _sum and _count, and a summary exports its _sum and _count with a quantile label (master's export of both is below). I left CHANGELOG.md alone; if you want a line under ## Unreleased for this, I will add one.
Closes #78 (closed)
How I tested it
I ran the specs the way the ruby CI job does (bundle install, bundle exec rake compile, bundle exec rake spec with prometheus_multiproc_dir=tmp/ and SKIP_C_EXTENSION_TESTING=1) in ruby images with Rust 1.89.0, the version .tool-versions pins.
- The four new or re-enabled specs fail on master and pass with the change, on Ruby 3.2.11 and 4.0.6.
- The whole suite passes with the change on Ruby 3.2.11, 3.3.12, 3.4.11 and 4.0.6: 182 examples, 0 failures, 19 pending. Master gives 179 examples and 20 pending; the difference is the three new specs and the one enabled again.
- I ran the issue's snippets,
getwith the same keys, and the multiprocess text export of those label sets on master and on this branch, on Ruby 3.2.11 (output below). - RuboCop is not run in CI, and the project's
.rubocop.ymldoes not load with the RuboCop versionGemfile.lockpins (1.75.6), which refuses two of its cop names as obsolete. With those two keys renamed as RuboCop suggests, the three files changed here have the same 9 offenses before and after the change.
The four specs against master's library (Ruby 3.2.11)
Failures:
1) Prometheus::Client::Histogram#initialization raise error for a le base label
Failure/Error:
expect do
described_class.new(:bar, 'bar description', { le: '1' })
end.to raise_error Prometheus::Client::LabelSetValidator::ReservedLabelError
expected Prometheus::Client::LabelSetValidator::ReservedLabelError but nothing was raised
2) Prometheus::Client::Histogram#observe raise error for le labels
Failure/Error:
expect do
histogram.observe({ le: 1 }, 5)
end.to raise_error Prometheus::Client::LabelSetValidator::ReservedLabelError
expected Prometheus::Client::LabelSetValidator::ReservedLabelError but nothing was raised
3) Prometheus::Client::Summary#initialization raise error for a quantile base label
Failure/Error:
expect do
described_class.new(:bar, 'bar description', { quantile: '0.5' })
end.to raise_error Prometheus::Client::LabelSetValidator::ReservedLabelError
expected Prometheus::Client::LabelSetValidator::ReservedLabelError but nothing was raised
4) Prometheus::Client::Summary#observe raise error for quantile labels
Failure/Error:
expect do
summary.observe({ quantile: 0.5 }, 5)
end.to raise_error Prometheus::Client::LabelSetValidator::ReservedLabelError
expected Prometheus::Client::LabelSetValidator::ReservedLabelError but nothing was raised
34 examples, 4 failures, 8 pendingThe issue's snippets, before and after
Run with Prometheus::Client::SimpleValue as the value class, on Ruby 3.2.11.
Master:
Histogram#observe({ le: "5" }, 42): accepted, label sets [{:le=>"5"}], base labels {}
Summary#observe({ quantile: 0.5 }, 42): accepted, label sets [{:quantile=>0.5}], base labels {}
Histogram.new(:sample, "...", { le: "5" }): accepted, label sets [], base labels {:le=>"5"}
Summary.new(:sample, "...", { quantile: "0.5" }): accepted, label sets [], base labels {:quantile=>"0.5"}This branch:
Histogram#observe({ le: "5" }, 42): Prometheus::Client::LabelSetValidator::ReservedLabelError: le is reserved
Summary#observe({ quantile: 0.5 }, 42): Prometheus::Client::LabelSetValidator::ReservedLabelError: quantile is reserved
Histogram.new(:sample, "...", { le: "5" }): Prometheus::Client::LabelSetValidator::ReservedLabelError: le is reserved
Summary.new(:sample, "...", { quantile: "0.5" }): Prometheus::Client::LabelSetValidator::ReservedLabelError: quantile is reservedAnd the names the change leaves alone, on this branch:
histogram quantile: [{:quantile=>"0.5"}]
summary le: [{:le=>"5"}]
counter: [{:le=>"5", :quantile=>"0.5"}]
gauge: [{:le=>"5", :quantile=>"0.5"}]get with the same keys, before and after
Run with Prometheus::Client::SimpleValue as the value class, on Ruby 3.2.11, on a new histogram and a new summary.
Master:
Histogram#get(le: '5'): returned {0.005=>0.0, 0.01=>0.0, 0.025=>0.0, 0.05=>0.0, 0.1=>0.0, 0.25=>0.0, 0.5=>0.0, 1=>0.0, 2.5=>0.0, 5=>0.0, 10=>0.0}
Summary#get(quantile: 0.5): returned 0.0This branch:
Histogram#get(le: '5'): Prometheus::Client::LabelSetValidator::ReservedLabelError: le is reserved
Summary#get(quantile: 0.5): Prometheus::Client::LabelSetValidator::ReservedLabelError: quantile is reservedWhat master exports for those label sets (multiprocess text format, Ruby 3.2.11)
# HELP lat Multiprocess metric
# TYPE lat summary
lat_count{quantile="0.5"} 1
lat_sum{quantile="0.5"} 3
# HELP req Multiprocess metric
# TYPE req histogram
req_bucket{le="1"} 0
req_bucket{le="10"} 1
req_bucket{le="+Inf"} 1
req_count{le="5"} 1
req_sum{le="5"} 3From registry.histogram(:req, 'req', {}, [1, 10]).observe({ le: '5' }, 3) and registry.summary(:lat, 'lat').observe({ quantile: '0.5' }, 3) with the default MmapedValue, rendered with Formats::Text.marshal_multiprocess. On this branch both observe calls raise ReservedLabelError.