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 +Inf bucket from total_inf, the value object Histogram::Value creates for <name>_bucket{le="+Inf"} and the entry marshal_multiprocess renders for it,
  • the sum from sum and the count from total.

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.rb renders a real Histogram (on SimpleValue) in place of the stubbed double. The expected counts change from 1 to 1.0 because they now come from the histogram's value objects, which hold floats, instead of the integers the double carried.
  • spec/prometheus/client/push_spec.rb passes a registry holding a histogram to Push#add and 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 pending

On 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 pending

Master 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 job

On 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

Merge request reports

Loading
Loading