Render histograms in the text format from their value objects
Prometheus::Client::Push renders the registry with Formats::Text.marshal, which read a histogram's sum and count as value.get.sum and value.get.total. Histogram::Value#get returns a plain Hash of bucket counts, so a registry holding a histogram with any observation raised NoMethodError: undefined method 'total' for an instance of Hash, and a histogram could not be pushed at all. The format spec passed because it rendered a double whose Hash was stubbed to answer sum and total.
Formats::Text.histogram now reads each sample from the value object that holds it:
- the buckets from
Histogram::Value#get, - the
+Infbucket fromtotal_inf, the value objectHistogram::Valuecreates for<name>_bucket{le="+Inf"}and the entrymarshal_multiprocessrenders for it, - the sum from
sumand the count fromtotal.
It also names the bucket samples <name>_bucket instead of <name>, as the text exposition format specifies and as marshal_multiprocess already does. The Pushgateway I tested against (1.11.4) accepts either spelling and stores the same histogram, so this is about following the format rather than about the push being refused.
I left Histogram::Value#get as it is. #79 (closed) suggests returning +Inf from it, but Histogram#get is how a caller reads the buckets (the README shows it without +Inf), and the format can read total_inf directly.
Tests
spec/prometheus/client/formats/text_spec.rbrenders a realHistogram(onSimpleValue) in place of the stubbed double. The expected counts change from1to1.0because they now come from the histogram's value objects, which hold floats, instead of the integers the double carried.spec/prometheus/client/push_spec.rbpasses a registry holding a histogram toPush#addand checks the body it sends, which is the call #75 (closed) reports.
Both examples fail on master with the NoMethodError from the issues and pass with this change. The whole suite passes on Ruby 3.2.11, 3.3.12, 3.4.11 and 4.0.6, the four versions the ruby CI job runs. I also pushed the reproduction from #75 (closed) to a real Pushgateway before and after the change (below).
One thing this does not change: a registry holding a summary still fails in Formats::Text, because Summary#values hands out only the sum's value object and the format cannot reach the count. That needs a change to Summary itself, so I kept it out of this MR.
I did not add a CHANGELOG.md entry, since entries are written at release time (as for !245 (merged) and !280 (merged)), but I can add one under Unreleased if you prefer.
Closes #75 (closed)
Closes #79 (closed)
The new specs on master and on this branch
With the two spec files from this branch and lib/prometheus/client/formats/text.rb from master b4cf84b, Ruby 3.4.11:
$ bundle exec rspec spec/prometheus/client/formats/text_spec.rb spec/prometheus/client/push_spec.rb
1) Prometheus::Client::Formats::Text single process metrics .marshal returns a Text format version 0.0.4 compatible representation
Failure/Error: rv << [name, set.merge(le: '+Inf'), value.get.total]
NoMethodError:
undefined method 'total' for an instance of Hash
# ./lib/prometheus/client/formats/text.rb:76:in 'Prometheus::Client::Formats::Text.histogram'
2) Prometheus::Client::Push#add with a histogram in the registry sends the histogram in the text format
Failure/Error: rv << [name, set.merge(le: '+Inf'), value.get.total]
NoMethodError:
undefined method 'total' for an instance of Hash
# ./lib/prometheus/client/formats/text.rb:76:in 'Prometheus::Client::Formats::Text.histogram'
# ./lib/prometheus/client/push.rb:124:in 'Prometheus::Client::Push#request'
# ./lib/prometheus/client/push.rb:58:in 'Prometheus::Client::Push#add'
29 examples, 2 failures, 1 pendingOn this branch:
$ bundle exec rspec spec/prometheus/client/formats/text_spec.rb spec/prometheus/client/push_spec.rb
29 examples, 0 failures, 1 pending
$ bundle exec rake spec # Ruby 3.2.11, 3.3.12, 3.4.11 and 4.0.6, after bundle exec rake compile
180 examples, 0 failures, 20 pendingMaster gives 179 examples, 0 failures, 20 pending on Ruby 3.4.11; the extra example is the new Push spec.
The reproduction from #75 (closed) against a Pushgateway
The issue's code (with buckets [1, 5, 10] and observations of 10 and 3), the gem's default MmapedValue, pushed to prom/pushgateway 1.11.4 in Docker.
On master both calls raise, so nothing reaches the Pushgateway:
--- Formats::Text.marshal(registry)
NoMethodError: undefined method 'total' for an instance of Hash
/src/lib/prometheus/client/formats/text.rb:76:in 'Prometheus::Client::Formats::Text.histogram'
--- Push#add
NoMethodError: undefined method 'total' for an instance of Hash
/src/lib/prometheus/client/formats/text.rb:76:in 'Prometheus::Client::Formats::Text.histogram'
--- what the Pushgateway serves for the jobOn this branch:
--- Formats::Text.marshal(registry)
# HELP processing_time_metric Time taken to process a file
# TYPE processing_time_metric histogram
processing_time_metric_bucket{test="test",le="1"} 0.0
processing_time_metric_bucket{test="test",le="5"} 1.0
processing_time_metric_bucket{test="test",le="10"} 2.0
processing_time_metric_bucket{test="test",le="+Inf"} 2.0
processing_time_metric_sum{test="test"} 13.0
processing_time_metric_count{test="test"} 2.0
--- Push#add
HTTP 200
--- what the Pushgateway serves for the job
# HELP processing_time_metric Time taken to process a file
# TYPE processing_time_metric histogram
processing_time_metric_bucket{instance="",job="test-metrics-job",test="test",le="1"} 0
processing_time_metric_bucket{instance="",job="test-metrics-job",test="test",le="5"} 1
processing_time_metric_bucket{instance="",job="test-metrics-job",test="test",le="10"} 2
processing_time_metric_bucket{instance="",job="test-metrics-job",test="test",le="+Inf"} 2
processing_time_metric_sum{instance="",job="test-metrics-job",test="test"} 13
processing_time_metric_count{instance="",job="test-metrics-job",test="test"} 2