From 31443a029f5f9c64a571f89504a68e4c40068fd2 Mon Sep 17 00:00:00 2001 From: Jeremiah Hemphill Date: Fri, 2 Jun 2023 15:14:43 -0400 Subject: [PATCH 01/21] Initial sketch at open metrics generation --- lib/prometheus/client/formats/open_metrics.rb | 79 +++++++++++++++++ lib/prometheus/middleware/exporter.rb | 1 + .../client/formats/open_metrics_spec.rb | 84 +++++++++++++++++++ 3 files changed, 164 insertions(+) create mode 100644 lib/prometheus/client/formats/open_metrics.rb create mode 100644 spec/prometheus/client/formats/open_metrics_spec.rb diff --git a/lib/prometheus/client/formats/open_metrics.rb b/lib/prometheus/client/formats/open_metrics.rb new file mode 100644 index 00000000..865e9008 --- /dev/null +++ b/lib/prometheus/client/formats/open_metrics.rb @@ -0,0 +1,79 @@ +# encoding: UTF-8 + +module Prometheus + module Client + module Formats + module OpenMetrics + # used by the middleware to determine if this format works for the request + MEDIA_TYPE = 'text/plain'.freeze + VERSION = '0.0.1'.freeze + CONTENT_TYPE = "#{MEDIA_TYPE}; version=#{VERSION}".freeze + + # public interface to generate out the /metrics payload + def self.marshal(registry) + lines = [] + + registry.metrics.each do |metric| + # generate metric and put it in lines + lines << Writer.new(metric).to_open_metrics + end + + (lines << nil).join(DELIMITER) + end + + # big questions + # - how to pull the timestamp out of the metrics repo + # - how to pull the right number of metrics rows for the given number of timestamps out of + # the metrics repo + # - how to pull out exemplars (and the right number of metric rows for exemplars) + # - label formatting (copy from the other file) + # - what does a sample mean in the docs, who decides that we should sample a specific value? + class Writer + attr_reader :metric + def initialize(metric) + @metric = metric + end + + def name + metric.name + end + + def docstring + metric.docstring + end + + def unit + metric.unit rescue "hotdogs" + end + + # the spec has a weird conversion with hard coded constants + # I am not sure if they are necessary + # for example counter converts to %d99.111.117.110.116.101.114 which really looks like + # character encodings for the word counter + def type + metric.type.to_s + end + + def metrics_to_s + # special case for summaries + # special case for histograms + # maybe start with gauges/counters because they are easy + metric.values.collect do |label_set, value| + "#{name}#{label_formatter(label_set)} #{value} #{metric.timestamp}" + end + end + + def description + "# TYPE #{name} #{type}\n" \ + "# UNIT #{name} #{unit}\n" \ + "# HELP #{name} #{docstring}\n" + end + + def write + "#{description}#{metrics_to_s}" + end + end + end + end + end +end diff --git a/lib/prometheus/middleware/exporter.rb b/lib/prometheus/middleware/exporter.rb index a377525c..bad5189c 100644 --- a/lib/prometheus/middleware/exporter.rb +++ b/lib/prometheus/middleware/exporter.rb @@ -2,6 +2,7 @@ require 'prometheus/client' require 'prometheus/client/formats/text' +require 'prometheus/client/formats/open_metrics' module Prometheus module Middleware diff --git a/spec/prometheus/client/formats/open_metrics_spec.rb b/spec/prometheus/client/formats/open_metrics_spec.rb new file mode 100644 index 00000000..154313aa --- /dev/null +++ b/spec/prometheus/client/formats/open_metrics_spec.rb @@ -0,0 +1,84 @@ +# encoding: UTF-8 + +require 'prometheus/client' +require 'prometheus/client/registry' +require 'prometheus/client/formats/open_metrics' + +describe Prometheus::Client::Formats::OpenMetrics do + # Reset the data store + before do + Prometheus::Client.config.data_store = Prometheus::Client::DataStores::Synchronized.new + end + + let(:registry) { Prometheus::Client::Registry.new } + + describe "metric writers" do + describe "counter" do + before do + @foo = registry.counter(:foo, + docstring: 'foo description', + labels: [:umlauts, :utf, :code], + preset_labels: {umlauts: 'Björn', utf: '佖佥'}) + @foo.increment(labels: { code: 'red'}, by: 42) + @foo.increment(labels: { code: 'green'}, by: 3.14E42) + @foo.increment(labels: { code: 'blue'}, by: 1.23e-45) + end + + it "generates a metric description" do + writer = Prometheus::Client::Formats::OpenMetrics::Writer.new(@foo) + + lines = writer.write.split("\n") + + expect(lines).to include("# TYPE foo counter") + expect(lines).to include("# UNIT foo hotdogs") + expect(lines).to include("# HELP foo foo description") + end + + it "generates a metric without a timestamp" + it "generates a metric with a timestamp" + it "generates a metric with an exemplar" + end + + describe "histogram" do + it "generates a metric description" + it "generates a metric without a timestamp" + it "generates a metric with a timestamp" + it "generates a metric with an exemplar" + end + + describe "gaugehistogram" do + it "generates a metric description" + it "generates a metric without a timestamp" + it "generates a metric with a timestamp" + it "generates a metric with an exemplar" + end + + describe "stateset" do + it "generates a metric description" + it "generates a metric without a timestamp" + it "generates a metric with a timestamp" + it "generates a metric with an exemplar" + end + + describe "summary" do + it "generates a metric description" + it "generates a metric without a timestamp" + it "generates a metric with a timestamp" + it "generates a metric with an exemplar" + end + + describe "info" do + it "generates a metric description" + it "generates a metric without a timestamp" + it "generates a metric with a timestamp" + it "generates a metric with an exemplar" + end + + describe "unknown" do + it "generates a metric description" + it "generates a metric without a timestamp" + it "generates a metric with a timestamp" + it "generates a metric with an exemplar" + end + end +end From 39538d88e0867c6a8093f910881d9caf8ef37386 Mon Sep 17 00:00:00 2001 From: Jeremiah Hemphill Date: Tue, 6 Jun 2023 13:06:31 -0400 Subject: [PATCH 02/21] Hacked together histograms --- lib/prometheus/client/formats/open_metrics.rb | 65 ++++++++++++++++-- .../client/formats/open_metrics_spec.rb | 67 +++++++++++++++---- 2 files changed, 113 insertions(+), 19 deletions(-) diff --git a/lib/prometheus/client/formats/open_metrics.rb b/lib/prometheus/client/formats/open_metrics.rb index 865e9008..ab1461f0 100644 --- a/lib/prometheus/client/formats/open_metrics.rb +++ b/lib/prometheus/client/formats/open_metrics.rb @@ -51,26 +51,77 @@ def unit # for example counter converts to %d99.111.117.110.116.101.114 which really looks like # character encodings for the word counter def type - metric.type.to_s + metric.type.to_sym end - def metrics_to_s + def metrics_to_a # special case for summaries # special case for histograms # maybe start with gauges/counters because they are easy + output = [] + metric.values.collect do |label_set, value| - "#{name}#{label_formatter(label_set)} #{value} #{metric.timestamp}" + if type == :histogram + output << histogram(metric.name, label_set, value) + else + output << metric_line(name, label_set, value) # timestamp + end + end + + output.flatten + end + + def histogram(name, label_set, value) + output = [] + + bucket = "#{name}_bucket" + value.each do |quantile, v| + next if quantile == "sum" + output << metric_line(bucket, label_set.merge(le: quantile), v) end + + output << metric_line("#{name}_sum", label_set, value["sum"]) + output << metric_line("#{name}_count", label_set, value["+Inf"]) + + output + end + + def metric_line(name, label_set, value, timestamp = nil) + output = "#{name}#{labels(label_set)} #{value}" + output += " #{timestamp}" if timestamp + + output + end + + def labels(set) + return if set.empty? + + output = [] + + set.each do |key, value| + output << "#{key}=\"#{escape(value, :label)}\"" + end + + "{#{output.join(",")}}" + end + + # to be rewritten + REGEX = { doc: /[\n\\]/, label: /[\n\\"]/ }.freeze + REPLACE = { "\n" => '\n', '\\' => '\\\\', '"' => '\"' }.freeze + def escape(string, format = :doc) + string.to_s.gsub(REGEX[format], REPLACE) end def description - "# TYPE #{name} #{type}\n" \ - "# UNIT #{name} #{unit}\n" \ - "# HELP #{name} #{docstring}\n" + [ + "# TYPE #{name} #{type}", + "# UNIT #{name} #{unit}", + "# HELP #{name} #{docstring}" + ] end def write - "#{description}#{metrics_to_s}" + (description + metrics_to_a).join("\n") end end end diff --git a/spec/prometheus/client/formats/open_metrics_spec.rb b/spec/prometheus/client/formats/open_metrics_spec.rb index 154313aa..dfa6d431 100644 --- a/spec/prometheus/client/formats/open_metrics_spec.rb +++ b/spec/prometheus/client/formats/open_metrics_spec.rb @@ -14,34 +14,77 @@ describe "metric writers" do describe "counter" do - before do - @foo = registry.counter(:foo, + let(:counter_without_ts) do + counter_without_ts = registry.counter(:counter_without_ts, docstring: 'foo description', labels: [:umlauts, :utf, :code], preset_labels: {umlauts: 'Björn', utf: '佖佥'}) - @foo.increment(labels: { code: 'red'}, by: 42) - @foo.increment(labels: { code: 'green'}, by: 3.14E42) - @foo.increment(labels: { code: 'blue'}, by: 1.23e-45) + counter_without_ts.increment(labels: { code: 'red'}, by: 42) + counter_without_ts.increment(labels: { code: 'green'}, by: 3.14E42) + counter_without_ts.increment(labels: { code: 'blue'}, by: 1.23e-45) + + counter_without_ts end it "generates a metric description" do - writer = Prometheus::Client::Formats::OpenMetrics::Writer.new(@foo) + writer = Prometheus::Client::Formats::OpenMetrics::Writer.new(counter_without_ts) lines = writer.write.split("\n") - expect(lines).to include("# TYPE foo counter") - expect(lines).to include("# UNIT foo hotdogs") - expect(lines).to include("# HELP foo foo description") + expect(lines).to include("# TYPE counter_without_ts counter") + expect(lines).to include("# UNIT counter_without_ts hotdogs") + expect(lines).to include("# HELP counter_without_ts foo description") + end + + it "generates a metric without a timestamp" do + writer = Prometheus::Client::Formats::OpenMetrics::Writer.new(counter_without_ts) + + lines = writer.write.split("\n") + + expect(lines).to include("counter_without_ts{umlauts=\"Björn\",utf=\"佖佥\",code=\"red\"} 42.0") + expect(lines).to include("counter_without_ts{umlauts=\"Björn\",utf=\"佖佥\",code=\"green\"} 3.14e+42") + expect(lines).to include("counter_without_ts{umlauts=\"Björn\",utf=\"佖佥\",code=\"blue\"} 1.23e-45") end - it "generates a metric without a timestamp" it "generates a metric with a timestamp" it "generates a metric with an exemplar" end describe "histogram" do - it "generates a metric description" - it "generates a metric without a timestamp" + let(:histogram_without_ts) do + xuq = registry.histogram(:histogram_without_ts, + docstring: 'xuq description', + labels: [:code], + preset_labels: {code: 'ah'}, + buckets: [10, 20, 30]) + xuq.observe(12) + xuq.observe(3.2) + + xuq + end + + it "generates a metric description" do + writer = Prometheus::Client::Formats::OpenMetrics::Writer.new(histogram_without_ts) + + lines = writer.write.split("\n") + + expect(lines).to include("# TYPE histogram_without_ts histogram") + expect(lines).to include("# UNIT histogram_without_ts hotdogs") + expect(lines).to include("# HELP histogram_without_ts xuq description") + end + + it "generates a metric without a timestamp" do + writer = Prometheus::Client::Formats::OpenMetrics::Writer.new(histogram_without_ts) + + lines = writer.write.split("\n") + + expect(lines).to include("histogram_without_ts_bucket{code=\"ah\",le=\"10\"} 1.0") + expect(lines).to include("histogram_without_ts_bucket{code=\"ah\",le=\"20\"} 2.0") + expect(lines).to include("histogram_without_ts_bucket{code=\"ah\",le=\"30\"} 2.0") + expect(lines).to include("histogram_without_ts_bucket{code=\"ah\",le=\"+Inf\"} 2.0") + expect(lines).to include("histogram_without_ts_sum{code=\"ah\"} 15.2") + expect(lines).to include("histogram_without_ts_count{code=\"ah\"} 2.0") + end it "generates a metric with a timestamp" it "generates a metric with an exemplar" end From b86b43713ec5f288254326a547bae5a46239ceaa Mon Sep 17 00:00:00 2001 From: Rob Stringer <41843577+Mycobee@users.noreply.github.com> Date: Tue, 6 Jun 2023 22:51:52 -0600 Subject: [PATCH 03/21] WIP...working on timestamps and figuring out how to pass them through the existing code --- lib/prometheus/client/formats/open_metrics.rb | 8 ++- lib/prometheus/client/metric.rb | 5 +- lib/prometheus/client/registry.rb | 5 +- .../client/formats/open_metrics_spec.rb | 50 ++++++++++++++++--- 4 files changed, 55 insertions(+), 13 deletions(-) diff --git a/lib/prometheus/client/formats/open_metrics.rb b/lib/prometheus/client/formats/open_metrics.rb index ab1461f0..6cf86d8a 100644 --- a/lib/prometheus/client/formats/open_metrics.rb +++ b/lib/prometheus/client/formats/open_metrics.rb @@ -60,11 +60,12 @@ def metrics_to_a # maybe start with gauges/counters because they are easy output = [] + require 'debug'; debugger metric.values.collect do |label_set, value| if type == :histogram output << histogram(metric.name, label_set, value) else - output << metric_line(name, label_set, value) # timestamp + output << metric_line(name, label_set, value, timestamp) # timestamp end end @@ -88,6 +89,7 @@ def histogram(name, label_set, value) def metric_line(name, label_set, value, timestamp = nil) output = "#{name}#{labels(label_set)} #{value}" + # require 'debug'; debugger output += " #{timestamp}" if timestamp output @@ -123,6 +125,10 @@ def description def write (description + metrics_to_a).join("\n") end + + def timestamp + + end end end end diff --git a/lib/prometheus/client/metric.rb b/lib/prometheus/client/metric.rb index 2094ed53..90b30b1f 100644 --- a/lib/prometheus/client/metric.rb +++ b/lib/prometheus/client/metric.rb @@ -7,13 +7,14 @@ module Prometheus module Client # Metric class Metric - attr_reader :name, :docstring, :labels, :preset_labels + attr_reader :name, :docstring, :labels, :preset_labels, :timestamp def initialize(name, docstring:, labels: [], preset_labels: {}, - store_settings: {}) + store_settings: {}, + timestamp: nil) validate_name(name) validate_docstring(docstring) diff --git a/lib/prometheus/client/registry.rb b/lib/prometheus/client/registry.rb index 0b2f6e9a..e166eae6 100644 --- a/lib/prometheus/client/registry.rb +++ b/lib/prometheus/client/registry.rb @@ -37,12 +37,13 @@ def unregister(name) end end - def counter(name, docstring:, labels: [], preset_labels: {}, store_settings: {}) + def counter(name, docstring:, labels: [], preset_labels: {}, store_settings: {}, timestamp: nil) register(Counter.new(name, docstring: docstring, labels: labels, preset_labels: preset_labels, - store_settings: store_settings)) + store_settings: store_settings, + timestamp: timestamp)) end def summary(name, docstring:, labels: [], preset_labels: {}, store_settings: {}) diff --git a/spec/prometheus/client/formats/open_metrics_spec.rb b/spec/prometheus/client/formats/open_metrics_spec.rb index dfa6d431..236c44fe 100644 --- a/spec/prometheus/client/formats/open_metrics_spec.rb +++ b/spec/prometheus/client/formats/open_metrics_spec.rb @@ -46,7 +46,28 @@ expect(lines).to include("counter_without_ts{umlauts=\"Björn\",utf=\"佖佥\",code=\"blue\"} 1.23e-45") end - it "generates a metric with a timestamp" + let(:counter_with_ts) do + counter_with_ts = registry.counter(:counter_with_ts, + docstring: 'foo description', + labels: [:umlauts, :utf, :code], + preset_labels: {umlauts: 'Björn', utf: '佖佥'}, + timestamp: 1686111748) + counter_with_ts.increment(labels: { code: 'red'}, by: 42) + counter_with_ts.increment(labels: { code: 'green'}, by: 3.14E42) + counter_with_ts.increment(labels: { code: 'blue'}, by: 1.23e-45) + counter_with_ts + + end + + it "generates a metric with a timestamp" do + require 'debug'; debugger + writer = Prometheus::Client::Formats::OpenMetrics::Writer.new(counter_with_ts) + + lines = writer.write.split("\n") + + + end + it "generates a metric with an exemplar" end @@ -85,7 +106,9 @@ expect(lines).to include("histogram_without_ts_sum{code=\"ah\"} 15.2") expect(lines).to include("histogram_without_ts_count{code=\"ah\"} 2.0") end - it "generates a metric with a timestamp" + it "generates a metric with a timestamp" do + + end it "generates a metric with an exemplar" end @@ -103,12 +126,23 @@ it "generates a metric with an exemplar" end - describe "summary" do - it "generates a metric description" - it "generates a metric without a timestamp" - it "generates a metric with a timestamp" - it "generates a metric with an exemplar" - end + # describe "summary" do + # let(:registry.summary(:summary)) do + # counter_without_ts = registry.counter(:counter_without_ts, + # docstring: 'foo description', + # labels: [:umlauts, :utf, :code], + # preset_labels: {umlauts: 'Björn', utf: '佖佥'}) + # counter_without_ts.increment(labels: { code: 'red'}, by: 42) + # counter_without_ts.increment(labels: { code: 'green'}, by: 3.14E42) + # counter_without_ts.increment(labels: { code: 'blue'}, by: 1.23e-45) + # + # counter_without_ts + # end + # it "generates a metric description" + # it "generates a metric without a timestamp" + # it "generates a metric with a timestamp" + # it "generates a metric with an exemplar" + # end describe "info" do it "generates a metric description" From 15583a3730fe5265e92d1ec709753664763a0509 Mon Sep 17 00:00:00 2001 From: Jeremiah Hemphill Date: Wed, 7 Jun 2023 12:13:13 -0400 Subject: [PATCH 04/21] Added gauge tests. They sort of work --- lib/prometheus/client/formats/open_metrics.rb | 1 - .../client/formats/open_metrics_spec.rb | 42 ++++++++++++++++--- 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/lib/prometheus/client/formats/open_metrics.rb b/lib/prometheus/client/formats/open_metrics.rb index 6cf86d8a..06fd33d3 100644 --- a/lib/prometheus/client/formats/open_metrics.rb +++ b/lib/prometheus/client/formats/open_metrics.rb @@ -60,7 +60,6 @@ def metrics_to_a # maybe start with gauges/counters because they are easy output = [] - require 'debug'; debugger metric.values.collect do |label_set, value| if type == :histogram output << histogram(metric.name, label_set, value) diff --git a/spec/prometheus/client/formats/open_metrics_spec.rb b/spec/prometheus/client/formats/open_metrics_spec.rb index 236c44fe..ea5a691e 100644 --- a/spec/prometheus/client/formats/open_metrics_spec.rb +++ b/spec/prometheus/client/formats/open_metrics_spec.rb @@ -52,22 +52,54 @@ labels: [:umlauts, :utf, :code], preset_labels: {umlauts: 'Björn', utf: '佖佥'}, timestamp: 1686111748) - counter_with_ts.increment(labels: { code: 'red'}, by: 42) - counter_with_ts.increment(labels: { code: 'green'}, by: 3.14E42) - counter_with_ts.increment(labels: { code: 'blue'}, by: 1.23e-45) + counter_with_ts.increment(labels: { code: 'red'}, by: 42, timestamp: Time.now.to_i) + counter_with_ts.increment(labels: { code: 'green'}, by: 3.14E42, timestamp: Time.now.to_i) + counter_with_ts.increment(labels: { code: 'blue'}, by: 1.23e-45, timestamp: Time.now.to_i + 1) counter_with_ts end - it "generates a metric with a timestamp" do - require 'debug'; debugger + xit "generates a metric with a timestamp" do writer = Prometheus::Client::Formats::OpenMetrics::Writer.new(counter_with_ts) lines = writer.write.split("\n") + expect(lines).to include("???") + end + + xit "generates a metric with an exemplar" + end + + describe "gauge" do + let :gauge_without_ts do + bar = registry.gauge(:gauge_without_ts, + docstring: "bar description\nwith newline", + labels: [:status, :code]) + bar.set(15, labels: { status: 'success', code: 'pink'}) + + bar + end + + it "generates a metric description" do + writer = Prometheus::Client::Formats::OpenMetrics::Writer.new(gauge_without_ts) + + lines = writer.write.split("\n") + + expect(lines).to include("# TYPE gauge_without_ts gauge") + expect(lines).to include("# UNIT gauge_without_ts hotdogs") + # I think the \n should be escaped + expect(lines).to include("# HELP gauge_without_ts bar description\nwith newline") + end + it "generates a metric without a timestamp" do + writer = Prometheus::Client::Formats::OpenMetrics::Writer.new(gauge_without_ts) + + lines = writer.write.split("\n") + + expect(lines).to include("gauge_without_ts{status=\"success\",code=\"pink\"} 15.0") end + it "generates a metric with a timestamp" it "generates a metric with an exemplar" end From 458649826fef89a4e2233818eba90de70abee7c2 Mon Sep 17 00:00:00 2001 From: Jeremiah Hemphill Date: Wed, 7 Jun 2023 12:16:43 -0400 Subject: [PATCH 05/21] Fixed docstring escaping --- lib/prometheus/client/formats/open_metrics.rb | 2 +- spec/prometheus/client/formats/open_metrics_spec.rb | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/prometheus/client/formats/open_metrics.rb b/lib/prometheus/client/formats/open_metrics.rb index 06fd33d3..797c41ba 100644 --- a/lib/prometheus/client/formats/open_metrics.rb +++ b/lib/prometheus/client/formats/open_metrics.rb @@ -117,7 +117,7 @@ def description [ "# TYPE #{name} #{type}", "# UNIT #{name} #{unit}", - "# HELP #{name} #{docstring}" + "# HELP #{name} #{escape(docstring, :doc)}" ] end diff --git a/spec/prometheus/client/formats/open_metrics_spec.rb b/spec/prometheus/client/formats/open_metrics_spec.rb index ea5a691e..787f767d 100644 --- a/spec/prometheus/client/formats/open_metrics_spec.rb +++ b/spec/prometheus/client/formats/open_metrics_spec.rb @@ -87,8 +87,7 @@ expect(lines).to include("# TYPE gauge_without_ts gauge") expect(lines).to include("# UNIT gauge_without_ts hotdogs") - # I think the \n should be escaped - expect(lines).to include("# HELP gauge_without_ts bar description\nwith newline") + expect(lines).to include("# HELP gauge_without_ts bar description\\nwith newline") end it "generates a metric without a timestamp" do From 88be230c7092cc2f102b71b24d9135cb4efc7be7 Mon Sep 17 00:00:00 2001 From: Jeremiah Hemphill Date: Wed, 7 Jun 2023 13:56:17 -0400 Subject: [PATCH 06/21] Added toy feature to store timestamp in label and counter tests to prove it works --- lib/prometheus/client/formats/open_metrics.rb | 18 +++++++++++------- .../client/formats/open_metrics_spec.rb | 18 +++++++++--------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/lib/prometheus/client/formats/open_metrics.rb b/lib/prometheus/client/formats/open_metrics.rb index 797c41ba..e3b925f9 100644 --- a/lib/prometheus/client/formats/open_metrics.rb +++ b/lib/prometheus/client/formats/open_metrics.rb @@ -64,7 +64,7 @@ def metrics_to_a if type == :histogram output << histogram(metric.name, label_set, value) else - output << metric_line(name, label_set, value, timestamp) # timestamp + output << metric_line(name, label_set, value) end end @@ -86,20 +86,27 @@ def histogram(name, label_set, value) output end - def metric_line(name, label_set, value, timestamp = nil) + def metric_line(name, label_set, value) output = "#{name}#{labels(label_set)} #{value}" # require 'debug'; debugger - output += " #{timestamp}" if timestamp + ts = timestamp(label_set) + output += " #{ts}" if ts output end + def timestamp(set) + return unless set.has_key?(:_timestamp) + + set[:_timestamp] + end + def labels(set) return if set.empty? output = [] - set.each do |key, value| + set.except(:_timestamp).each do |key, value| output << "#{key}=\"#{escape(value, :label)}\"" end @@ -125,9 +132,6 @@ def write (description + metrics_to_a).join("\n") end - def timestamp - - end end end end diff --git a/spec/prometheus/client/formats/open_metrics_spec.rb b/spec/prometheus/client/formats/open_metrics_spec.rb index 787f767d..4a19f94f 100644 --- a/spec/prometheus/client/formats/open_metrics_spec.rb +++ b/spec/prometheus/client/formats/open_metrics_spec.rb @@ -49,22 +49,22 @@ let(:counter_with_ts) do counter_with_ts = registry.counter(:counter_with_ts, docstring: 'foo description', - labels: [:umlauts, :utf, :code], - preset_labels: {umlauts: 'Björn', utf: '佖佥'}, - timestamp: 1686111748) - counter_with_ts.increment(labels: { code: 'red'}, by: 42, timestamp: Time.now.to_i) - counter_with_ts.increment(labels: { code: 'green'}, by: 3.14E42, timestamp: Time.now.to_i) - counter_with_ts.increment(labels: { code: 'blue'}, by: 1.23e-45, timestamp: Time.now.to_i + 1) - counter_with_ts + labels: [:umlauts, :utf, :code, :_timestamp], + preset_labels: {umlauts: 'Björn', utf: '佖佥'}) + counter_with_ts.increment(labels: { code: 'red', _timestamp: 1000000}, by: 42) + counter_with_ts.increment(labels: { code: 'red', _timestamp: 1000000}, by: 1) + counter_with_ts.increment(labels: { code: 'blue', _timestamp: 1000001}, by: 1.23e-45) + counter_with_ts end - xit "generates a metric with a timestamp" do + it "generates a metric with a timestamp" do writer = Prometheus::Client::Formats::OpenMetrics::Writer.new(counter_with_ts) lines = writer.write.split("\n") - expect(lines).to include("???") + expect(lines).to include("counter_with_ts{umlauts=\"Björn\",utf=\"佖佥\",code=\"red\"} 43.0 1000000") + expect(lines).to include("counter_with_ts{umlauts=\"Björn\",utf=\"佖佥\",code=\"blue\"} 1.23e-45 1000001") end xit "generates a metric with an exemplar" From 802c4f823321dc85d536e0cdd08c79e657b9ef2c Mon Sep 17 00:00:00 2001 From: Jeremiah Hemphill Date: Wed, 7 Jun 2023 14:14:01 -0400 Subject: [PATCH 07/21] More requirementsg --- .../client/formats/open_metrics_spec.rb | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/spec/prometheus/client/formats/open_metrics_spec.rb b/spec/prometheus/client/formats/open_metrics_spec.rb index 4a19f94f..3d48df54 100644 --- a/spec/prometheus/client/formats/open_metrics_spec.rb +++ b/spec/prometheus/client/formats/open_metrics_spec.rb @@ -12,6 +12,20 @@ let(:registry) { Prometheus::Client::Registry.new } + it "If a unit is specified it MUST be provided in a UNIT metadata line. In addition, an underscore and the unit MUST be the suffix of the MetricFamily name." + it "If more than one MetricPoint is exposed for a Metric, the ordering should be by label permutation, then by oldest to newest timestamp" + # for example + # # TYPE foo_seconds summary + # # UNIT foo_seconds seconds + # foo_seconds_count{a="bb"} 0 123 + # foo_seconds_sum{a="bb"} 0 123 + # foo_seconds_count{a="bb"} 0 456 + # foo_seconds_sum{a="bb"} 0 456 + # foo_seconds_count{a="ccc"} 0 123 + # foo_seconds_sum{a="ccc"} 0 123 + # foo_seconds_count{a="ccc"} 0 456 + # foo_seconds_sum{a="ccc"} 0 456 + describe "metric writers" do describe "counter" do let(:counter_without_ts) do @@ -67,6 +81,12 @@ expect(lines).to include("counter_with_ts{umlauts=\"Björn\",utf=\"佖佥\",code=\"blue\"} 1.23e-45 1000001") end + it "A MetricPoint in a Metric with the type Counter MUST have one value called Total. A Total is a non-NaN and MUST be monotonically non-decreasing over time, starting from 0." + it "A MetricPoint in a Metric with the type Counter SHOULD have a Timestamp value called Created. This can help ingestors discern between new metrics and long-running ones it did not see before. Created does not have a value except the timestamp." + + it "A MetricPoint in a Metric's Counter's Total MAY reset to 0. If present, the corresponding Created time MUST also be set to the timestamp of the reset." + it "A MetricPoint in a Metric's Counter's Total MAY have an exemplar." + xit "generates a metric with an exemplar" end @@ -100,6 +120,8 @@ it "generates a metric with a timestamp" it "generates a metric with an exemplar" + + it "A MetricPoint in a Metric with the type gauge MUST have a single value. I am pretty sure this means a single metric per label permutation per gauge but not 100% (JH)" end describe "histogram" do @@ -141,6 +163,16 @@ end it "generates a metric with an exemplar" + + it "A Histogram MetricPoint MUST contain at least one bucket, and SHOULD contain Sum, and Created values. Every bucket MUST have a threshold and a value." + it "Histogram MetricPoints MUST have one bucket with an +Inf threshold." + it "Buckets MUST be cumulative. As an example for a metric representing request latency in seconds its values for buckets with thresholds 1, 2, 3, and +Inf MUST follow value_1 <= value_2 <= value_3 <= value_+Inf. If ten requests took 1 second each, the values of the 1, 2, 3, and +Inf buckets MUST equal 10." + it "The +Inf bucket counts all requests. If present, the Sum value MUST equal the Sum of all the measured event values. Bucket thresholds within a MetricPoint MUST be unique." + it "Semantically, Sum, and buckets values are counters so MUST NOT be NaN or negative. Negative threshold buckets MAY be used, but then the Histogram MetricPoint MUST NOT contain a sum value as it would no longer be a counter semantically. Bucket thresholds MUST NOT equal NaN. Count and bucket values MUST be integers." + it "A Histogram MetricPoint SHOULD have a Timestamp value called Created. This can help ingestors discern between new metrics and long-running ones it did not see before." + it "A Histogram's Metric's LabelSet MUST NOT have a 'le' label name." + it "Bucket values MAY have exemplars. Buckets are cumulative to allow monitoring systems to drop any non-+Inf bucket for performance/anti-denial-of-service reasons in a way that loses granularity but is still a valid Histogram." + it "Each bucket covers the values less and or equal to it, and the value of the exemplar MUST be within this range. Exemplars SHOULD be put into the bucket with the highest value. A bucket MUST NOT have more than one exemplar." end describe "gaugehistogram" do @@ -155,6 +187,11 @@ it "generates a metric without a timestamp" it "generates a metric with a timestamp" it "generates a metric with an exemplar" + + it "A point of a StateSet metric MAY contain multiple states and MUST contain one boolean per State. States have a name which are Strings." + it "A StateSet Metric's LabelSet MUST NOT have a label name which is the same as the name of its MetricFamily." + it "If encoded as a StateSet, ENUMs MUST have exactly one Boolean which is true within a MetricPoint." + it "MetricFamilies of type StateSets MUST have an empty Unit string." end # describe "summary" do @@ -180,6 +217,10 @@ it "generates a metric without a timestamp" it "generates a metric with a timestamp" it "generates a metric with an exemplar" + + it "A MetricPoint of an Info Metric contains a LabelSet. An Info MetricPoint's LabelSet MUST NOT have a label name which is the same as the name of a label of the LabelSet of its Metric." + it "Info MAY be used to encode ENUMs whose values do not change over time, such as the type of a network interface." + it "MetricFamilies of type Info MUST have an empty Unit string." end describe "unknown" do From 4be5c1aed072dea499208d9f1ec2209a491c2b0f Mon Sep 17 00:00:00 2001 From: Jeremiah Hemphill Date: Thu, 8 Jun 2023 12:53:08 -0400 Subject: [PATCH 08/21] Rewrite some tests --- .../prometheus/client/formats/open_metrics_spec.rb | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/spec/prometheus/client/formats/open_metrics_spec.rb b/spec/prometheus/client/formats/open_metrics_spec.rb index 3d48df54..27a63b31 100644 --- a/spec/prometheus/client/formats/open_metrics_spec.rb +++ b/spec/prometheus/client/formats/open_metrics_spec.rb @@ -12,6 +12,7 @@ let(:registry) { Prometheus::Client::Registry.new } + it "created should not have any labels" it "If a unit is specified it MUST be provided in a UNIT metadata line. In addition, an underscore and the unit MUST be the suffix of the MetricFamily name." it "If more than one MetricPoint is exposed for a Metric, the ordering should be by label permutation, then by oldest to newest timestamp" # for example @@ -164,12 +165,13 @@ end it "generates a metric with an exemplar" - it "A Histogram MetricPoint MUST contain at least one bucket, and SHOULD contain Sum, and Created values. Every bucket MUST have a threshold and a value." - it "Histogram MetricPoints MUST have one bucket with an +Inf threshold." - it "Buckets MUST be cumulative. As an example for a metric representing request latency in seconds its values for buckets with thresholds 1, 2, 3, and +Inf MUST follow value_1 <= value_2 <= value_3 <= value_+Inf. If ten requests took 1 second each, the values of the 1, 2, 3, and +Inf buckets MUST equal 10." - it "The +Inf bucket counts all requests. If present, the Sum value MUST equal the Sum of all the measured event values. Bucket thresholds within a MetricPoint MUST be unique." - it "Semantically, Sum, and buckets values are counters so MUST NOT be NaN or negative. Negative threshold buckets MAY be used, but then the Histogram MetricPoint MUST NOT contain a sum value as it would no longer be a counter semantically. Bucket thresholds MUST NOT equal NaN. Count and bucket values MUST be integers." - it "A Histogram MetricPoint SHOULD have a Timestamp value called Created. This can help ingestors discern between new metrics and long-running ones it did not see before." + it "generates a metric with at least one bucket, sum, created, and count metric points" + it "generates a bucket with a +Inf threshold that counts all values" + it "generates cumulative buckets, a low value increments the count in all buckets with higher values" + it "generates a sum that equals the sum of all measured event values" + it "if there is a negative valued bucket, there should be no sum metric" + it "is not clear if we should print the bucket multiple times with different timestamps to expose multiple exemplars" + it "A Histogram's Metric's LabelSet MUST NOT have a 'le' label name." it "Bucket values MAY have exemplars. Buckets are cumulative to allow monitoring systems to drop any non-+Inf bucket for performance/anti-denial-of-service reasons in a way that loses granularity but is still a valid Histogram." it "Each bucket covers the values less and or equal to it, and the value of the exemplar MUST be within this range. Exemplars SHOULD be put into the bucket with the highest value. A bucket MUST NOT have more than one exemplar." From 705d1c04b5e87a811d0b55b03627a102f137fcb9 Mon Sep 17 00:00:00 2001 From: Jeremiah Hemphill Date: Thu, 8 Jun 2023 17:25:04 -0400 Subject: [PATCH 09/21] Exemplars working on counters --- lib/prometheus/client/counter.rb | 4 +- .../client/data_stores/direct_file_store.rb | 8 +-- .../client/data_stores/single_threaded.rb | 36 ++++++++--- .../client/data_stores/synchronized.rb | 34 +++++++--- lib/prometheus/client/exemplar.rb | 17 +++++ lib/prometheus/client/exemplar_collection.rb | 63 +++++++++++++++++++ lib/prometheus/client/formats/open_metrics.rb | 33 ++++------ lib/prometheus/client/gauge.rb | 12 ++-- lib/prometheus/client/histogram.rb | 6 +- lib/prometheus/client/metric.rb | 13 ++-- lib/prometheus/client/registry.rb | 5 +- lib/prometheus/client/summary.rb | 6 +- lib/prometheus/client/value_with_exemplars.rb | 44 +++++++++++++ prometheus-client.gemspec | 1 + .../client/formats/open_metrics_spec.rb | 35 +++++++---- 15 files changed, 242 insertions(+), 75 deletions(-) create mode 100644 lib/prometheus/client/exemplar.rb create mode 100644 lib/prometheus/client/exemplar_collection.rb create mode 100644 lib/prometheus/client/value_with_exemplars.rb diff --git a/lib/prometheus/client/counter.rb b/lib/prometheus/client/counter.rb index 28ec2f1e..07933182 100644 --- a/lib/prometheus/client/counter.rb +++ b/lib/prometheus/client/counter.rb @@ -10,11 +10,11 @@ def type :counter end - def increment(by: 1, labels: {}) + def increment(by: 1, labels: {}, exemplar: nil) raise ArgumentError, 'increment must be a non-negative number' if by < 0 label_set = label_set_for(labels) - @store.increment(labels: label_set, by: by) + @store.increment(labels: label_set, by: by, exemplar: exemplar) end end end diff --git a/lib/prometheus/client/data_stores/direct_file_store.rb b/lib/prometheus/client/data_stores/direct_file_store.rb index 1c09dc4d..5f41abf2 100644 --- a/lib/prometheus/client/data_stores/direct_file_store.rb +++ b/lib/prometheus/client/data_stores/direct_file_store.rb @@ -99,13 +99,13 @@ def synchronize end end - def set(labels:, val:) + def set(labels:, val:, exemplar: nil) in_process_sync do internal_store.write_value(store_key(labels), val.to_f) end end - def increment(labels:, by: 1) + def increment(labels:, by: 1, exemplar: nil) if @values_aggregation_mode == DirectFileStore::MOST_RECENT raise InvalidStoreSettingsError, "The :most_recent aggregation does not support the use of increment"\ @@ -118,13 +118,13 @@ def increment(labels:, by: 1) end end - def get(labels:) + def get(labels:, with_exemplars: false) in_process_sync do internal_store.read_value(store_key(labels)) end end - def all_values + def all_values(with_exemplars: false) stores_data = Hash.new{ |hash, key| hash[key] = [] } # There's no need to call `synchronize` here. We're opening a second handle to diff --git a/lib/prometheus/client/data_stores/single_threaded.rb b/lib/prometheus/client/data_stores/single_threaded.rb index f05cf813..fc58703e 100644 --- a/lib/prometheus/client/data_stores/single_threaded.rb +++ b/lib/prometheus/client/data_stores/single_threaded.rb @@ -1,3 +1,7 @@ +require "prometheus/client/value_with_exemplars" +require "prometheus/client/exemplar_collection" +require "prometheus/client/exemplar" + module Prometheus module Client module DataStores @@ -25,27 +29,41 @@ def validate_metric_settings(metric_settings:) class MetricStore def initialize - @internal_store = Hash.new { |hash, key| hash[key] = 0.0 } + @internal_store = Hash.new { |hash, key| hash[key] = ValueWithExemplars.new } end def synchronize yield end - def set(labels:, val:) - @internal_store[labels] = val.to_f + def set(labels:, val:, exemplar: nil) + @internal_store[labels].set(value: val, exemplar: exemplar) end - def increment(labels:, by: 1) - @internal_store[labels] += by + def increment(labels:, by: 1, exemplar: nil) + @internal_store[labels].increment(by: by, exemplar: exemplar) end - def get(labels:) - @internal_store[labels] + def get(labels:, with_exemplars: false) + if with_exemplars + @internal_store[labels] + else + @internal_store[labels].value + end end - def all_values - @internal_store.dup + def all_values(with_exemplars: false) + if with_exemplars + @internal_store.dup + else + # this mess is just for backwards compatibility + output = Hash.new { |hash, key| hash[key] = 0.0 } + @internal_store.keys.each do |k| + output[k] = @internal_store[k].value + end + + output + end end end diff --git a/lib/prometheus/client/data_stores/synchronized.rb b/lib/prometheus/client/data_stores/synchronized.rb index d0a74608..868a0ef3 100644 --- a/lib/prometheus/client/data_stores/synchronized.rb +++ b/lib/prometheus/client/data_stores/synchronized.rb @@ -24,7 +24,7 @@ def validate_metric_settings(metric_settings:) class MetricStore def initialize - @internal_store = Hash.new { |hash, key| hash[key] = 0.0 } + @internal_store = Hash.new { |hash, key| hash[key] = ValueWithExemplars.new } @lock = Monitor.new end @@ -32,26 +32,42 @@ def synchronize @lock.synchronize { yield } end - def set(labels:, val:) + def set(labels:, val:, exemplar: nil) synchronize do - @internal_store[labels] = val.to_f + @internal_store[labels].set(value: val, exemplar: exemplar) end end - def increment(labels:, by: 1) + def increment(labels:, by: 1, exemplar: nil) synchronize do - @internal_store[labels] += by + @internal_store[labels].increment(by: by, exemplar: exemplar) end end - def get(labels:) + def get(labels:, with_exemplars: false) synchronize do - @internal_store[labels] + if with_exemplars + @internal_store[labels] + else + @internal_store[labels].value + end end end - def all_values - synchronize { @internal_store.dup } + def all_values(with_exemplars: false) + synchronize do + if with_exemplars + @internal_store.dup + else + # this mess is just for backwards compatibility + output = Hash.new { |hash, key| hash[key] = 0.0 } + @internal_store.keys.each do |k| + output[k] = @internal_store[k].value + end + + output + end + end end end diff --git a/lib/prometheus/client/exemplar.rb b/lib/prometheus/client/exemplar.rb new file mode 100644 index 00000000..b939e127 --- /dev/null +++ b/lib/prometheus/client/exemplar.rb @@ -0,0 +1,17 @@ +# encoding: UTF-8 + +module Prometheus + module Client + + # essentially a wrapper for a hash and a timestamp + # maybe will hold validity checks eventually + class Exemplar + attr_reader :labels, :timestamp, :value + attr_writer :value + def initialize(labels: {}, timestamp: nil) + @labels = labels + @timestamp = timestamp || Time.now.to_i + end + end + end +end diff --git a/lib/prometheus/client/exemplar_collection.rb b/lib/prometheus/client/exemplar_collection.rb new file mode 100644 index 00000000..f1456ae5 --- /dev/null +++ b/lib/prometheus/client/exemplar_collection.rb @@ -0,0 +1,63 @@ +# encoding: UTF-8 + +module Prometheus + module Client + class ExemplarCollection + extend Enumerable + + # store the exemplars attached to a metric + label set + def initialize + @collection = {} + + # theoretical way to avoid causing a major memory leak + # + # This could store two minutes of requests with 2 requests per second with these settings. + # I am not clear on the situation where this wouldn't be enough but I am sure I will find it + @max_timestamps = 120 + @max_exemplars = 240 + @exemplar_count = 0 + end + + def add(exemplar) + cleanup_if_necessary + @collection[exemplar.timestamp] ||= [] + @collection[exemplar.timestamp] << exemplar + + exemplar + end + + def most_recent + @collection[last_key]&.last + end + + def last + most_recent + end + + def each + @collection.keys.sort.each do |key| + @collection[key].each do |exemplar| + yield(exemplar) + end + end + end + + private + + def cleanup_if_necessary + @exemplar_count += 1 + if @exemplar_count > @max_exemplars || @collection.keys.size > @max_timestamps + @exemplar_count -= @collection.delete(first_key)&.size + end + end + + def first_key + @collection.keys.sort&.first + end + + def last_key + @collection.keys.sort&.last + end + end + end +end diff --git a/lib/prometheus/client/formats/open_metrics.rb b/lib/prometheus/client/formats/open_metrics.rb index e3b925f9..905d1c9e 100644 --- a/lib/prometheus/client/formats/open_metrics.rb +++ b/lib/prometheus/client/formats/open_metrics.rb @@ -21,13 +21,6 @@ def self.marshal(registry) (lines << nil).join(DELIMITER) end - # big questions - # - how to pull the timestamp out of the metrics repo - # - how to pull the right number of metrics rows for the given number of timestamps out of - # the metrics repo - # - how to pull out exemplars (and the right number of metric rows for exemplars) - # - label formatting (copy from the other file) - # - what does a sample mean in the docs, who decides that we should sample a specific value? class Writer attr_reader :metric def initialize(metric) @@ -60,11 +53,13 @@ def metrics_to_a # maybe start with gauges/counters because they are easy output = [] - metric.values.collect do |label_set, value| - if type == :histogram + if type == :histogram + metric.values.collect do |label_set, value| output << histogram(metric.name, label_set, value) - else - output << metric_line(name, label_set, value) + end + else + metric.values(with_exemplars: true).collect do |label_set, value_with_exemplars| + output << metric_line(name, label_set, value_with_exemplars.value, value_with_exemplars.most_recent_exemplar) end end @@ -86,19 +81,17 @@ def histogram(name, label_set, value) output end - def metric_line(name, label_set, value) + def metric_line(name, label_set, value, exemplar = nil) output = "#{name}#{labels(label_set)} #{value}" - # require 'debug'; debugger - ts = timestamp(label_set) - output += " #{ts}" if ts + output += " #{timestamp}" if timestamp + output += " # #{labels(exemplar.labels)} #{exemplar.value} #{exemplar.timestamp}" if exemplar output end - def timestamp(set) - return unless set.has_key?(:_timestamp) - - set[:_timestamp] + def timestamp + # not implemented yet + return nil end def labels(set) @@ -106,7 +99,7 @@ def labels(set) output = [] - set.except(:_timestamp).each do |key, value| + set.each do |key, value| output << "#{key}=\"#{escape(value, :label)}\"" end diff --git a/lib/prometheus/client/gauge.rb b/lib/prometheus/client/gauge.rb index e0f76521..eaab57de 100644 --- a/lib/prometheus/client/gauge.rb +++ b/lib/prometheus/client/gauge.rb @@ -12,26 +12,26 @@ def type end # Sets the value for the given label set - def set(value, labels: {}) + def set(value, labels: {}, exemplar: nil) unless value.is_a?(Numeric) raise ArgumentError, 'value must be a number' end - @store.set(labels: label_set_for(labels), val: value) + @store.set(labels: label_set_for(labels), val: value, exemplar: nil) end # Increments Gauge value by 1 or adds the given value to the Gauge. # (The value can be negative, resulting in a decrease of the Gauge.) - def increment(by: 1, labels: {}) + def increment(by: 1, labels: {}, exemplar: nil) label_set = label_set_for(labels) - @store.increment(labels: label_set, by: by) + @store.increment(labels: label_set, by: by, exemplar: nil) end # Decrements Gauge value by 1 or subtracts the given value from the Gauge. # (The value can be negative, resulting in a increase of the Gauge.) - def decrement(by: 1, labels: {}) + def decrement(by: 1, labels: {}, exemplar: nil) label_set = label_set_for(labels) - @store.increment(labels: label_set, by: -by) + @store.increment(labels: label_set, by: -by, exemplar: nil) end end end diff --git a/lib/prometheus/client/histogram.rb b/lib/prometheus/client/histogram.rb index 6963f673..ff142a52 100644 --- a/lib/prometheus/client/histogram.rb +++ b/lib/prometheus/client/histogram.rb @@ -66,7 +66,7 @@ def type # in the sum of observations. See # https://prometheus.io/docs/practices/histograms/#count-and-sum-of-observations # for details. - def observe(value, labels: {}) + def observe(value, labels: {}, exemplar: nil) bucket = buckets.find {|upper_limit| upper_limit >= value } bucket = "+Inf" if bucket.nil? @@ -79,8 +79,8 @@ def observe(value, labels: {}) sum_label_set[:le] = "sum" @store.synchronize do - @store.increment(labels: bucket_label_set, by: 1) - @store.increment(labels: sum_label_set, by: value) + @store.increment(labels: bucket_label_set, by: 1, exemplar: exemplar) + @store.increment(labels: sum_label_set, by: value, exemplar: exemplar) end end diff --git a/lib/prometheus/client/metric.rb b/lib/prometheus/client/metric.rb index 90b30b1f..9eda2c8c 100644 --- a/lib/prometheus/client/metric.rb +++ b/lib/prometheus/client/metric.rb @@ -2,19 +2,20 @@ require 'thread' require 'prometheus/client/label_set_validator' +require 'prometheus/client/exemplar' +require 'prometheus/client/exemplar_collection' module Prometheus module Client # Metric class Metric - attr_reader :name, :docstring, :labels, :preset_labels, :timestamp + attr_reader :name, :docstring, :labels, :preset_labels, :created_at def initialize(name, docstring:, labels: [], preset_labels: {}, - store_settings: {}, - timestamp: nil) + store_settings: {}) validate_name(name) validate_docstring(docstring) @@ -42,6 +43,8 @@ def initialize(name, metric_settings: store_settings ) + @created_at = Time.now.to_i + # WARNING: Our internal store can be replaced later by `with_labels` # Everything we do after this point needs to still work if @store gets replaced init_label_set({}) if labels.empty? @@ -77,8 +80,8 @@ def init_label_set(labels) end # Returns all label sets with their values - def values - @store.all_values + def values(with_exemplars: false) + @store.all_values(with_exemplars: with_exemplars) end private diff --git a/lib/prometheus/client/registry.rb b/lib/prometheus/client/registry.rb index e166eae6..0b2f6e9a 100644 --- a/lib/prometheus/client/registry.rb +++ b/lib/prometheus/client/registry.rb @@ -37,13 +37,12 @@ def unregister(name) end end - def counter(name, docstring:, labels: [], preset_labels: {}, store_settings: {}, timestamp: nil) + def counter(name, docstring:, labels: [], preset_labels: {}, store_settings: {}) register(Counter.new(name, docstring: docstring, labels: labels, preset_labels: preset_labels, - store_settings: store_settings, - timestamp: timestamp)) + store_settings: store_settings)) end def summary(name, docstring:, labels: [], preset_labels: {}, store_settings: {}) diff --git a/lib/prometheus/client/summary.rb b/lib/prometheus/client/summary.rb index dff2f360..fbbed0a5 100644 --- a/lib/prometheus/client/summary.rb +++ b/lib/prometheus/client/summary.rb @@ -17,12 +17,12 @@ def type # in the sum of observations. See # https://prometheus.io/docs/practices/histograms/#count-and-sum-of-observations # for details. - def observe(value, labels: {}) + def observe(value, labels: {}, exemplar: nil) base_label_set = label_set_for(labels) @store.synchronize do - @store.increment(labels: base_label_set.merge(quantile: "count"), by: 1) - @store.increment(labels: base_label_set.merge(quantile: "sum"), by: value) + @store.increment(labels: base_label_set.merge(quantile: "count"), by: 1, exemplar: exemplar) + @store.increment(labels: base_label_set.merge(quantile: "sum"), by: value, exemplar: exemplar) end end diff --git a/lib/prometheus/client/value_with_exemplars.rb b/lib/prometheus/client/value_with_exemplars.rb new file mode 100644 index 00000000..20fdcdfe --- /dev/null +++ b/lib/prometheus/client/value_with_exemplars.rb @@ -0,0 +1,44 @@ +# encoding: UTF-8 + +module Prometheus + module Client + # stores a value for a label permutation on a metric along with all the exemplars that match the + # labels. + # + # The labels are actually stored somewhere else. Maybe we could duplicate them here? + # + # no idea how this is going to work with histograms + class ValueWithExemplars + attr_reader :value, :exemplar + + def initialize + @exemplars = ExemplarCollection.new + @value = 0.0 + end + + def most_recent_exemplar + @exemplars.most_recent + end + + def set(value:, exemplar:) + @value = value.to_f + if exemplar + exemplar.value = @value # not convinced this line goes in this file + @exemplars.add(exemplar) + end + + @value + end + + def increment(by: 1, exemplar:) + @value += by + if exemplar + exemplar.value = @value # not convinced this line goes in this file + @exemplars.add(exemplar) + end + + @value + end + end + end +end diff --git a/prometheus-client.gemspec b/prometheus-client.gemspec index 6083a16e..11bec07f 100644 --- a/prometheus-client.gemspec +++ b/prometheus-client.gemspec @@ -17,4 +17,5 @@ Gem::Specification.new do |s| s.add_development_dependency 'benchmark-ips' s.add_development_dependency 'concurrent-ruby' + s.add_development_dependency 'pry-byebug' end diff --git a/spec/prometheus/client/formats/open_metrics_spec.rb b/spec/prometheus/client/formats/open_metrics_spec.rb index 27a63b31..6c077c72 100644 --- a/spec/prometheus/client/formats/open_metrics_spec.rb +++ b/spec/prometheus/client/formats/open_metrics_spec.rb @@ -3,6 +3,9 @@ require 'prometheus/client' require 'prometheus/client/registry' require 'prometheus/client/formats/open_metrics' +require "prometheus/client/value_with_exemplars" +require "prometheus/client/exemplar_collection" +require "prometheus/client/exemplar" describe Prometheus::Client::Formats::OpenMetrics do # Reset the data store @@ -61,25 +64,35 @@ expect(lines).to include("counter_without_ts{umlauts=\"Björn\",utf=\"佖佥\",code=\"blue\"} 1.23e-45") end - let(:counter_with_ts) do - counter_with_ts = registry.counter(:counter_with_ts, + let(:counter_with_exemplars) do + counter_with_exemplars = registry.counter(:counter_with_exemplars, docstring: 'foo description', - labels: [:umlauts, :utf, :code, :_timestamp], + labels: [:umlauts, :utf, :code], preset_labels: {umlauts: 'Björn', utf: '佖佥'}) - counter_with_ts.increment(labels: { code: 'red', _timestamp: 1000000}, by: 42) - counter_with_ts.increment(labels: { code: 'red', _timestamp: 1000000}, by: 1) - counter_with_ts.increment(labels: { code: 'blue', _timestamp: 1000001}, by: 1.23e-45) - counter_with_ts + counter_with_exemplars.increment( + labels: { code: 'red'}, + by: 42, + exemplar: Prometheus::Client::Exemplar.new(labels: {trace_id: 12345}, timestamp: 1000) + ) + counter_with_exemplars.increment(labels: { code: 'red'}, by: 1) + counter_with_exemplars.increment( + labels: { code: 'blue'}, + by: 1.23e-45, + exemplar: Prometheus::Client::Exemplar.new(labels: {trace_id: 23456}, timestamp: 2000) + ) + + counter_with_exemplars end - it "generates a metric with a timestamp" do - writer = Prometheus::Client::Formats::OpenMetrics::Writer.new(counter_with_ts) + it "generates a metric with an exemplar" do + writer = Prometheus::Client::Formats::OpenMetrics::Writer.new(counter_with_exemplars) lines = writer.write.split("\n") + puts lines - expect(lines).to include("counter_with_ts{umlauts=\"Björn\",utf=\"佖佥\",code=\"red\"} 43.0 1000000") - expect(lines).to include("counter_with_ts{umlauts=\"Björn\",utf=\"佖佥\",code=\"blue\"} 1.23e-45 1000001") + expect(lines).to include("counter_with_exemplars{umlauts=\"Björn\",utf=\"佖佥\",code=\"red\"} 43.0 # {trace_id=\"12345\"} 42.0 1000") + expect(lines).to include("counter_with_exemplars{umlauts=\"Björn\",utf=\"佖佥\",code=\"blue\"} 1.23e-45 # {trace_id=\"23456\"} 1.23e-45 2000") end it "A MetricPoint in a Metric with the type Counter MUST have one value called Total. A Total is a non-NaN and MUST be monotonically non-decreasing over time, starting from 0." From d247afc1104e8967fb09e1458c7a818ca4632361 Mon Sep 17 00:00:00 2001 From: Jeremiah Hemphill Date: Fri, 9 Jun 2023 10:30:49 -0400 Subject: [PATCH 10/21] Added _created and _total support to counters --- lib/prometheus/client/formats/open_metrics.rb | 45 +++++++++--- lib/prometheus/client/metric.rb | 4 +- lib/prometheus/client/value_with_exemplars.rb | 10 ++- .../client/formats/open_metrics_spec.rb | 72 +++++++++++-------- 4 files changed, 88 insertions(+), 43 deletions(-) diff --git a/lib/prometheus/client/formats/open_metrics.rb b/lib/prometheus/client/formats/open_metrics.rb index 905d1c9e..24082725 100644 --- a/lib/prometheus/client/formats/open_metrics.rb +++ b/lib/prometheus/client/formats/open_metrics.rb @@ -54,9 +54,9 @@ def metrics_to_a output = [] if type == :histogram - metric.values.collect do |label_set, value| - output << histogram(metric.name, label_set, value) - end + output << histogram + elsif type == :counter + output << counter else metric.values(with_exemplars: true).collect do |label_set, value_with_exemplars| output << metric_line(name, label_set, value_with_exemplars.value, value_with_exemplars.most_recent_exemplar) @@ -66,17 +66,42 @@ def metrics_to_a output.flatten end - def histogram(name, label_set, value) + def histogram output = [] - bucket = "#{name}_bucket" - value.each do |quantile, v| - next if quantile == "sum" - output << metric_line(bucket, label_set.merge(le: quantile), v) + metric.values.collect do |label_set, value| + bucket = "#{name}_bucket" + value.each do |quantile, v| + next if quantile == "sum" + output << metric_line(bucket, label_set.merge(le: quantile), v) + end + + output << metric_line("#{name}_sum", label_set, value["sum"]) + output << metric_line("#{name}_count", label_set, value["+Inf"]) end - output << metric_line("#{name}_sum", label_set, value["sum"]) - output << metric_line("#{name}_count", label_set, value["+Inf"]) + output + end + + def counter + output = [] + total_value = 0 + most_recent_total_exemplar = Exemplar.new(labels: {}, timestamp: 0) + + metric.values(with_exemplars: true).collect do |label_set, value_with_exemplars| + value = value_with_exemplars.value + exemplar = value_with_exemplars.most_recent_exemplar + created = value_with_exemplars.created + + output << metric_line(name, label_set, value, exemplar) + output << metric_line("#{name}_created", label_set, created) + + total_value += value + most_recent_total_exemplar = exemplar if exemplar && exemplar.timestamp > most_recent_total_exemplar.timestamp + end + + # assume any exemplar fits here (regardless of labels) as long as it is the most recent + output << metric_line("#{name}_total", {}, total_value, most_recent_total_exemplar) output end diff --git a/lib/prometheus/client/metric.rb b/lib/prometheus/client/metric.rb index 9eda2c8c..8f662def 100644 --- a/lib/prometheus/client/metric.rb +++ b/lib/prometheus/client/metric.rb @@ -9,7 +9,7 @@ module Prometheus module Client # Metric class Metric - attr_reader :name, :docstring, :labels, :preset_labels, :created_at + attr_reader :name, :docstring, :labels, :preset_labels def initialize(name, docstring:, @@ -43,8 +43,6 @@ def initialize(name, metric_settings: store_settings ) - @created_at = Time.now.to_i - # WARNING: Our internal store can be replaced later by `with_labels` # Everything we do after this point needs to still work if @store gets replaced init_label_set({}) if labels.empty? diff --git a/lib/prometheus/client/value_with_exemplars.rb b/lib/prometheus/client/value_with_exemplars.rb index 20fdcdfe..33e40d39 100644 --- a/lib/prometheus/client/value_with_exemplars.rb +++ b/lib/prometheus/client/value_with_exemplars.rb @@ -9,11 +9,19 @@ module Client # # no idea how this is going to work with histograms class ValueWithExemplars - attr_reader :value, :exemplar + attr_reader :value, :exemplar, :created def initialize @exemplars = ExemplarCollection.new @value = 0.0 + + # slightly confused on the spec for this one + # One reading is that the created timestamp is on a per label set/metricpoint basis, not per + # metric. I also wrote a parallel change that adds the created to the constructor in + # metric.rb and decided to remove it based on the spec interpretation. + # + # Floating point time is used to match the spec examples + @created = Time.now.to_f end def most_recent_exemplar diff --git a/spec/prometheus/client/formats/open_metrics_spec.rb b/spec/prometheus/client/formats/open_metrics_spec.rb index 6c077c72..07e91225 100644 --- a/spec/prometheus/client/formats/open_metrics_spec.rb +++ b/spec/prometheus/client/formats/open_metrics_spec.rb @@ -32,38 +32,38 @@ describe "metric writers" do describe "counter" do - let(:counter_without_ts) do - counter_without_ts = registry.counter(:counter_without_ts, + let(:counter_metric) do + counter_metric = registry.counter(:counter_metric, docstring: 'foo description', labels: [:umlauts, :utf, :code], preset_labels: {umlauts: 'Björn', utf: '佖佥'}) - counter_without_ts.increment(labels: { code: 'red'}, by: 42) - counter_without_ts.increment(labels: { code: 'green'}, by: 3.14E42) - counter_without_ts.increment(labels: { code: 'blue'}, by: 1.23e-45) + counter_metric.increment(labels: { code: 'red'}, by: 42) + counter_metric.increment(labels: { code: 'green'}, by: 3.14E42) + counter_metric.increment(labels: { code: 'blue'}, by: 1.23e-45) - counter_without_ts + counter_metric end it "generates a metric description" do - writer = Prometheus::Client::Formats::OpenMetrics::Writer.new(counter_without_ts) + writer = Prometheus::Client::Formats::OpenMetrics::Writer.new(counter_metric) lines = writer.write.split("\n") - expect(lines).to include("# TYPE counter_without_ts counter") - expect(lines).to include("# UNIT counter_without_ts hotdogs") - expect(lines).to include("# HELP counter_without_ts foo description") + expect(lines).to include("# TYPE counter_metric counter") + expect(lines).to include("# UNIT counter_metric hotdogs") + expect(lines).to include("# HELP counter_metric foo description") end - it "generates a metric without a timestamp" do - writer = Prometheus::Client::Formats::OpenMetrics::Writer.new(counter_without_ts) + it "generates a metric without exemplars" do + writer = Prometheus::Client::Formats::OpenMetrics::Writer.new(counter_metric) lines = writer.write.split("\n") - expect(lines).to include("counter_without_ts{umlauts=\"Björn\",utf=\"佖佥\",code=\"red\"} 42.0") - expect(lines).to include("counter_without_ts{umlauts=\"Björn\",utf=\"佖佥\",code=\"green\"} 3.14e+42") - expect(lines).to include("counter_without_ts{umlauts=\"Björn\",utf=\"佖佥\",code=\"blue\"} 1.23e-45") + expect(lines).to include("counter_metric{umlauts=\"Björn\",utf=\"佖佥\",code=\"red\"} 42.0") + expect(lines).to include("counter_metric{umlauts=\"Björn\",utf=\"佖佥\",code=\"green\"} 3.14e+42") + expect(lines).to include("counter_metric{umlauts=\"Björn\",utf=\"佖佥\",code=\"blue\"} 1.23e-45") end - + let(:counter_with_exemplars) do counter_with_exemplars = registry.counter(:counter_with_exemplars, docstring: 'foo description', @@ -78,7 +78,7 @@ counter_with_exemplars.increment(labels: { code: 'red'}, by: 1) counter_with_exemplars.increment( labels: { code: 'blue'}, - by: 1.23e-45, + by: 1000, exemplar: Prometheus::Client::Exemplar.new(labels: {trace_id: 23456}, timestamp: 2000) ) @@ -89,19 +89,33 @@ writer = Prometheus::Client::Formats::OpenMetrics::Writer.new(counter_with_exemplars) lines = writer.write.split("\n") - puts lines expect(lines).to include("counter_with_exemplars{umlauts=\"Björn\",utf=\"佖佥\",code=\"red\"} 43.0 # {trace_id=\"12345\"} 42.0 1000") - expect(lines).to include("counter_with_exemplars{umlauts=\"Björn\",utf=\"佖佥\",code=\"blue\"} 1.23e-45 # {trace_id=\"23456\"} 1.23e-45 2000") + expect(lines).to include("counter_with_exemplars{umlauts=\"Björn\",utf=\"佖佥\",code=\"blue\"} 1000.0 # {trace_id=\"23456\"} 1000.0 2000") end - it "A MetricPoint in a Metric with the type Counter MUST have one value called Total. A Total is a non-NaN and MUST be monotonically non-decreasing over time, starting from 0." - it "A MetricPoint in a Metric with the type Counter SHOULD have a Timestamp value called Created. This can help ingestors discern between new metrics and long-running ones it did not see before. Created does not have a value except the timestamp." + it "generates a total metric point with exemplar and without labels" do + writer = Prometheus::Client::Formats::OpenMetrics::Writer.new(counter_with_exemplars) - it "A MetricPoint in a Metric's Counter's Total MAY reset to 0. If present, the corresponding Created time MUST also be set to the timestamp of the reset." - it "A MetricPoint in a Metric's Counter's Total MAY have an exemplar." + lines = writer.write.split("\n") + + expect(lines).to include("counter_with_exemplars_total 1043.0 # {trace_id=\"23456\"} 1000.0 2000") + end - xit "generates a metric with an exemplar" + it "generates a created metric point without exemplar and with labels" do + writer = Prometheus::Client::Formats::OpenMetrics::Writer.new(counter_with_exemplars) + + lines = writer.write.split("\n") + puts lines + + # this is hard to test + red_created = counter_with_exemplars.values(with_exemplars: true)[{:umlauts=>"Björn", :utf=>"佖佥", :code=>"red"}].created + expect(lines).to include("counter_with_exemplars_created{umlauts=\"Björn\",utf=\"佖佥\",code=\"red\"} #{red_created}") + blue_created = counter_with_exemplars.values(with_exemplars: true)[{:umlauts=>"Björn", :utf=>"佖佥", :code=>"blue"}].created + expect(lines).to include("counter_with_exemplars_created{umlauts=\"Björn\",utf=\"佖佥\",code=\"blue\"} #{blue_created}") + end + + it "A MetricPoint in a Metric's Counter's Total MAY reset to 0. If present, the corresponding Created time MUST also be set to the timestamp of the reset." end describe "gauge" do @@ -211,15 +225,15 @@ # describe "summary" do # let(:registry.summary(:summary)) do - # counter_without_ts = registry.counter(:counter_without_ts, + # counter_metric = registry.counter(:counter_metric, # docstring: 'foo description', # labels: [:umlauts, :utf, :code], # preset_labels: {umlauts: 'Björn', utf: '佖佥'}) - # counter_without_ts.increment(labels: { code: 'red'}, by: 42) - # counter_without_ts.increment(labels: { code: 'green'}, by: 3.14E42) - # counter_without_ts.increment(labels: { code: 'blue'}, by: 1.23e-45) + # counter_metric.increment(labels: { code: 'red'}, by: 42) + # counter_metric.increment(labels: { code: 'green'}, by: 3.14E42) + # counter_metric.increment(labels: { code: 'blue'}, by: 1.23e-45) # - # counter_without_ts + # counter_metric # end # it "generates a metric description" # it "generates a metric without a timestamp" From 52009d82282bd98850e75d59378512b781523106 Mon Sep 17 00:00:00 2001 From: Jeremiah Hemphill Date: Fri, 9 Jun 2023 11:05:56 -0400 Subject: [PATCH 11/21] Gauge exemplars --- lib/prometheus/client/exemplar.rb | 18 ++++++++++--- lib/prometheus/client/formats/open_metrics.rb | 2 +- lib/prometheus/client/gauge.rb | 6 ++--- .../client/formats/open_metrics_spec.rb | 26 +++++++------------ 4 files changed, 29 insertions(+), 23 deletions(-) diff --git a/lib/prometheus/client/exemplar.rb b/lib/prometheus/client/exemplar.rb index b939e127..c594c038 100644 --- a/lib/prometheus/client/exemplar.rb +++ b/lib/prometheus/client/exemplar.rb @@ -3,11 +3,23 @@ module Prometheus module Client - # essentially a wrapper for a hash and a timestamp - # maybe will hold validity checks eventually + # Store a snapshot of metric data including an extra set of kv pairs for a specific moment in + # time and specific moment of code execution. + # + # Value is expected to be set after the exemplar is initialized. class Exemplar - attr_reader :labels, :timestamp, :value + # The kv pairs that make up the unique information in the exemplar. + # + # We generally store trace ids here + attr_reader :labels + + # The time the exemplar was recorded + attr_reader :timestamp + + # The value of the metric at the time the exemplar was recorded attr_writer :value + attr_reader :value + def initialize(labels: {}, timestamp: nil) @labels = labels @timestamp = timestamp || Time.now.to_i diff --git a/lib/prometheus/client/formats/open_metrics.rb b/lib/prometheus/client/formats/open_metrics.rb index 24082725..b9b722c3 100644 --- a/lib/prometheus/client/formats/open_metrics.rb +++ b/lib/prometheus/client/formats/open_metrics.rb @@ -57,7 +57,7 @@ def metrics_to_a output << histogram elsif type == :counter output << counter - else + else # if [:gauge].include?(type) metric.values(with_exemplars: true).collect do |label_set, value_with_exemplars| output << metric_line(name, label_set, value_with_exemplars.value, value_with_exemplars.most_recent_exemplar) end diff --git a/lib/prometheus/client/gauge.rb b/lib/prometheus/client/gauge.rb index eaab57de..f7bde67d 100644 --- a/lib/prometheus/client/gauge.rb +++ b/lib/prometheus/client/gauge.rb @@ -17,21 +17,21 @@ def set(value, labels: {}, exemplar: nil) raise ArgumentError, 'value must be a number' end - @store.set(labels: label_set_for(labels), val: value, exemplar: nil) + @store.set(labels: label_set_for(labels), val: value, exemplar: exemplar) end # Increments Gauge value by 1 or adds the given value to the Gauge. # (The value can be negative, resulting in a decrease of the Gauge.) def increment(by: 1, labels: {}, exemplar: nil) label_set = label_set_for(labels) - @store.increment(labels: label_set, by: by, exemplar: nil) + @store.increment(labels: label_set, by: by, exemplar: exemplar) end # Decrements Gauge value by 1 or subtracts the given value from the Gauge. # (The value can be negative, resulting in a increase of the Gauge.) def decrement(by: 1, labels: {}, exemplar: nil) label_set = label_set_for(labels) - @store.increment(labels: label_set, by: -by, exemplar: nil) + @store.increment(labels: label_set, by: -by, exemplar: exemplar) end end end diff --git a/spec/prometheus/client/formats/open_metrics_spec.rb b/spec/prometheus/client/formats/open_metrics_spec.rb index 07e91225..490b8344 100644 --- a/spec/prometheus/client/formats/open_metrics_spec.rb +++ b/spec/prometheus/client/formats/open_metrics_spec.rb @@ -114,42 +114,36 @@ blue_created = counter_with_exemplars.values(with_exemplars: true)[{:umlauts=>"Björn", :utf=>"佖佥", :code=>"blue"}].created expect(lines).to include("counter_with_exemplars_created{umlauts=\"Björn\",utf=\"佖佥\",code=\"blue\"} #{blue_created}") end - - it "A MetricPoint in a Metric's Counter's Total MAY reset to 0. If present, the corresponding Created time MUST also be set to the timestamp of the reset." end describe "gauge" do - let :gauge_without_ts do - bar = registry.gauge(:gauge_without_ts, + let :gauge_with_exemplar do + bar = registry.gauge(:gauge_with_exemplar, docstring: "bar description\nwith newline", labels: [:status, :code]) - bar.set(15, labels: { status: 'success', code: 'pink'}) + bar.set(15, labels: { status: 'success', code: 'pink'}, exemplar: Prometheus::Client::Exemplar.new(labels: {trace_id: 23456}, timestamp: 2000)) + bar.set(17, labels: { status: 'success', code: 'pink'}) bar end it "generates a metric description" do - writer = Prometheus::Client::Formats::OpenMetrics::Writer.new(gauge_without_ts) + writer = Prometheus::Client::Formats::OpenMetrics::Writer.new(gauge_with_exemplar) lines = writer.write.split("\n") - expect(lines).to include("# TYPE gauge_without_ts gauge") - expect(lines).to include("# UNIT gauge_without_ts hotdogs") - expect(lines).to include("# HELP gauge_without_ts bar description\\nwith newline") + expect(lines).to include("# TYPE gauge_with_exemplar gauge") + expect(lines).to include("# UNIT gauge_with_exemplar hotdogs") + expect(lines).to include("# HELP gauge_with_exemplar bar description\\nwith newline") end it "generates a metric without a timestamp" do - writer = Prometheus::Client::Formats::OpenMetrics::Writer.new(gauge_without_ts) + writer = Prometheus::Client::Formats::OpenMetrics::Writer.new(gauge_with_exemplar) lines = writer.write.split("\n") - expect(lines).to include("gauge_without_ts{status=\"success\",code=\"pink\"} 15.0") + expect(lines).to include("gauge_with_exemplar{status=\"success\",code=\"pink\"} 17.0 # {trace_id=\"23456\"} 15.0 2000") end - - it "generates a metric with a timestamp" - it "generates a metric with an exemplar" - - it "A MetricPoint in a Metric with the type gauge MUST have a single value. I am pretty sure this means a single metric per label permutation per gauge but not 100% (JH)" end describe "histogram" do From 9194d4148e66b352cefc8b2ee4b2f49871992924 Mon Sep 17 00:00:00 2001 From: Jeremiah Hemphill Date: Fri, 9 Jun 2023 11:59:52 -0400 Subject: [PATCH 12/21] Basic tests --- lib/prometheus/client/exemplar_collection.rb | 11 +++- lib/prometheus/client/formats/open_metrics.rb | 1 + lib/prometheus/client/value_with_exemplars.rb | 4 +- .../client/exemplar_collection_spec.rb | 60 +++++++++++++++++++ spec/prometheus/client/exemplar_spec.rb | 17 ++++++ .../client/formats/open_metrics_spec.rb | 15 ----- .../client/value_with_exemplar_spec.rb | 52 ++++++++++++++++ 7 files changed, 140 insertions(+), 20 deletions(-) create mode 100644 spec/prometheus/client/exemplar_collection_spec.rb create mode 100644 spec/prometheus/client/exemplar_spec.rb create mode 100644 spec/prometheus/client/value_with_exemplar_spec.rb diff --git a/lib/prometheus/client/exemplar_collection.rb b/lib/prometheus/client/exemplar_collection.rb index f1456ae5..5df49250 100644 --- a/lib/prometheus/client/exemplar_collection.rb +++ b/lib/prometheus/client/exemplar_collection.rb @@ -3,7 +3,7 @@ module Prometheus module Client class ExemplarCollection - extend Enumerable + include Enumerable # store the exemplars attached to a metric + label set def initialize @@ -42,13 +42,18 @@ def each end end + def size + @exemplar_count + end + private def cleanup_if_necessary - @exemplar_count += 1 - if @exemplar_count > @max_exemplars || @collection.keys.size > @max_timestamps + if @exemplar_count >= @max_exemplars || @collection.keys.size >= @max_timestamps @exemplar_count -= @collection.delete(first_key)&.size end + + @exemplar_count += 1 end def first_key diff --git a/lib/prometheus/client/formats/open_metrics.rb b/lib/prometheus/client/formats/open_metrics.rb index b9b722c3..f17c8752 100644 --- a/lib/prometheus/client/formats/open_metrics.rb +++ b/lib/prometheus/client/formats/open_metrics.rb @@ -78,6 +78,7 @@ def histogram output << metric_line("#{name}_sum", label_set, value["sum"]) output << metric_line("#{name}_count", label_set, value["+Inf"]) + # output << metric_line("#{name}_created", label_set, created) end output diff --git a/lib/prometheus/client/value_with_exemplars.rb b/lib/prometheus/client/value_with_exemplars.rb index 33e40d39..f7f1077a 100644 --- a/lib/prometheus/client/value_with_exemplars.rb +++ b/lib/prometheus/client/value_with_exemplars.rb @@ -28,7 +28,7 @@ def most_recent_exemplar @exemplars.most_recent end - def set(value:, exemplar:) + def set(value:, exemplar: nil) @value = value.to_f if exemplar exemplar.value = @value # not convinced this line goes in this file @@ -38,7 +38,7 @@ def set(value:, exemplar:) @value end - def increment(by: 1, exemplar:) + def increment(by: 1, exemplar: nil) @value += by if exemplar exemplar.value = @value # not convinced this line goes in this file diff --git a/spec/prometheus/client/exemplar_collection_spec.rb b/spec/prometheus/client/exemplar_collection_spec.rb new file mode 100644 index 00000000..17b58c4f --- /dev/null +++ b/spec/prometheus/client/exemplar_collection_spec.rb @@ -0,0 +1,60 @@ +# encoding: UTF-8 + +require 'prometheus/client/exemplar' +require 'prometheus/client/exemplar_collection' + +describe Prometheus::Client::ExemplarCollection do + describe "add" do + it "adds an exemplar" do + collection = described_class.new + + exemplar = Prometheus::Client::Exemplar.new(labels: {hotdogs: "great"}, timestamp: 1) + collection.add(exemplar) + + expect(collection.first).to eq(exemplar) + end + + it "cleans up if necessary" do + collection = described_class.new + + 120.times do |index| + exemplar = Prometheus::Client::Exemplar.new(labels: {hotdogs: "great"}, timestamp: index) + collection.add(exemplar) + end + + expect(collection.size).to eq(120) + + exemplar = Prometheus::Client::Exemplar.new(labels: {hotdogs: "great"}, timestamp: 1000) + collection.add(exemplar) + + expect(collection.size).to eq(120) + expect(collection.most_recent.timestamp).to eq(1000) + end + end + + describe "most_recent" do + it "returns the most recent exemplar by timestamp" do + collection = described_class.new + + exemplar_1 = Prometheus::Client::Exemplar.new(labels: {hotdogs: "great"}, timestamp: 10) + collection.add(exemplar_1) + + exemplar_2 = Prometheus::Client::Exemplar.new(labels: {hotdogs: "bad"}, timestamp: 5) + collection.add(exemplar_2) + + expect(collection.most_recent).to eq(exemplar_1) + end + + it "returns the most recently written exemplar if there is a timestamp tie" do + collection = described_class.new + + exemplar_1 = Prometheus::Client::Exemplar.new(labels: {hotdogs: "great"}, timestamp: 1) + collection.add(exemplar_1) + + exemplar_2 = Prometheus::Client::Exemplar.new(labels: {hotdogs: "bad"}, timestamp: 1) + collection.add(exemplar_2) + + expect(collection.most_recent).to eq(exemplar_2) + end + end +end diff --git a/spec/prometheus/client/exemplar_spec.rb b/spec/prometheus/client/exemplar_spec.rb new file mode 100644 index 00000000..f69dd91e --- /dev/null +++ b/spec/prometheus/client/exemplar_spec.rb @@ -0,0 +1,17 @@ +# encoding: UTF-8 + +require 'prometheus/client/exemplar' + +describe Prometheus::Client::Exemplar do + it "sets default labels" do + e = described_class.new + + expect(e.labels).to eq({}) + end + + it "sets a default timestamp" do + e = described_class.new + + expect(e.timestamp).to be <= Time.now.to_i + end +end diff --git a/spec/prometheus/client/formats/open_metrics_spec.rb b/spec/prometheus/client/formats/open_metrics_spec.rb index 490b8344..1b8d20e6 100644 --- a/spec/prometheus/client/formats/open_metrics_spec.rb +++ b/spec/prometheus/client/formats/open_metrics_spec.rb @@ -15,21 +15,6 @@ let(:registry) { Prometheus::Client::Registry.new } - it "created should not have any labels" - it "If a unit is specified it MUST be provided in a UNIT metadata line. In addition, an underscore and the unit MUST be the suffix of the MetricFamily name." - it "If more than one MetricPoint is exposed for a Metric, the ordering should be by label permutation, then by oldest to newest timestamp" - # for example - # # TYPE foo_seconds summary - # # UNIT foo_seconds seconds - # foo_seconds_count{a="bb"} 0 123 - # foo_seconds_sum{a="bb"} 0 123 - # foo_seconds_count{a="bb"} 0 456 - # foo_seconds_sum{a="bb"} 0 456 - # foo_seconds_count{a="ccc"} 0 123 - # foo_seconds_sum{a="ccc"} 0 123 - # foo_seconds_count{a="ccc"} 0 456 - # foo_seconds_sum{a="ccc"} 0 456 - describe "metric writers" do describe "counter" do let(:counter_metric) do diff --git a/spec/prometheus/client/value_with_exemplar_spec.rb b/spec/prometheus/client/value_with_exemplar_spec.rb new file mode 100644 index 00000000..bcca8e6b --- /dev/null +++ b/spec/prometheus/client/value_with_exemplar_spec.rb @@ -0,0 +1,52 @@ +# encoding: UTF-8 + +require 'prometheus/client/exemplar' +require 'prometheus/client/exemplar_collection' + +describe Prometheus::Client::ValueWithExemplars do + describe "set" do + it "updates the value as a float" do + vwe = described_class.new + + vwe.set(value: 5) + + expect(vwe.value).to eq(5.0) + end + + it "stores an exemplar and updates its value" do + vwe = described_class.new + + exemplar = Prometheus::Client::Exemplar.new(labels: {hotdogs: "great"}) + vwe.set(value: 5, exemplar: exemplar) + + expect(vwe.most_recent_exemplar.value).to eq(5.0) + end + end + + describe "increment" do + it "updates the value as a float" do + vwe = described_class.new + + vwe.set(value: 5) + exemplar = Prometheus::Client::Exemplar.new(labels: {hotdogs: "great"}) + vwe.increment(by: 7, exemplar: exemplar) + + expect(vwe.value).to eq(12.0) + expect(vwe.most_recent_exemplar.value).to eq(12.0) + end + end + + describe "most_recent_exemplar" do + it "returns the most recent exemplar by timestamp" do + vwe = described_class.new + + vwe.set(value: 5) + exemplar = Prometheus::Client::Exemplar.new(labels: {hotdogs: "great"}) + vwe.increment(by: 7, exemplar: exemplar) + vwe.increment(by: 3) + + expect(vwe.value).to eq(15.0) + expect(vwe.most_recent_exemplar.value).to eq(12.0) + end + end +end From 14d11aa8f1d2be781221b38ee4948b228cfe38da Mon Sep 17 00:00:00 2001 From: Jeremiah Hemphill Date: Tue, 13 Jun 2023 19:55:42 -0400 Subject: [PATCH 13/21] Setup summaries without exemplars --- lib/prometheus/client/formats/open_metrics.rb | 29 ++++- lib/prometheus/client/metric.rb | 3 +- .../client/formats/open_metrics_spec.rb | 115 ++++++++++-------- 3 files changed, 91 insertions(+), 56 deletions(-) diff --git a/lib/prometheus/client/formats/open_metrics.rb b/lib/prometheus/client/formats/open_metrics.rb index f17c8752..eb12f522 100644 --- a/lib/prometheus/client/formats/open_metrics.rb +++ b/lib/prometheus/client/formats/open_metrics.rb @@ -36,7 +36,7 @@ def docstring end def unit - metric.unit rescue "hotdogs" + metric&.unit || nil end # the spec has a weird conversion with hard coded constants @@ -55,6 +55,8 @@ def metrics_to_a if type == :histogram output << histogram + elsif type == :summary + output << summary elsif type == :counter output << counter else # if [:gauge].include?(type) @@ -78,6 +80,20 @@ def histogram output << metric_line("#{name}_sum", label_set, value["sum"]) output << metric_line("#{name}_count", label_set, value["+Inf"]) + # created is dependent on the exemplar working + # output << metric_line("#{name}_created", label_set, created) + end + + output + end + + def summary + output = [] + + metric.values.collect do |label_set, value| + output << metric_line("#{name}_sum", label_set, value["sum"]) + output << metric_line("#{name}_count", label_set, value["count"]) + # created is dependent on the exemplar working # output << metric_line("#{name}_created", label_set, created) end @@ -140,11 +156,12 @@ def escape(string, format = :doc) end def description - [ - "# TYPE #{name} #{type}", - "# UNIT #{name} #{unit}", - "# HELP #{name} #{escape(docstring, :doc)}" - ] + output = [] + output << "# TYPE #{name} #{type}" + output << "# UNIT #{name} #{unit}" if unit + output << "# HELP #{name} #{escape(docstring, :doc)}" + + output end def write diff --git a/lib/prometheus/client/metric.rb b/lib/prometheus/client/metric.rb index 8f662def..bfb9fc9c 100644 --- a/lib/prometheus/client/metric.rb +++ b/lib/prometheus/client/metric.rb @@ -9,7 +9,7 @@ module Prometheus module Client # Metric class Metric - attr_reader :name, :docstring, :labels, :preset_labels + attr_reader :name, :docstring, :labels, :preset_labels, :unit def initialize(name, docstring:, @@ -30,6 +30,7 @@ def initialize(name, @name = name @docstring = docstring @preset_labels = stringify_values(preset_labels) + @unit = nil # not fully supported yet @all_labels_preset = false if preset_labels.keys.length == labels.length diff --git a/spec/prometheus/client/formats/open_metrics_spec.rb b/spec/prometheus/client/formats/open_metrics_spec.rb index 1b8d20e6..d4d6ece3 100644 --- a/spec/prometheus/client/formats/open_metrics_spec.rb +++ b/spec/prometheus/client/formats/open_metrics_spec.rb @@ -16,6 +16,8 @@ let(:registry) { Prometheus::Client::Registry.new } describe "metric writers" do + it "fully supports unit including comment string and forcing a metric name change based on the unit name" + describe "counter" do let(:counter_metric) do counter_metric = registry.counter(:counter_metric, @@ -35,7 +37,7 @@ lines = writer.write.split("\n") expect(lines).to include("# TYPE counter_metric counter") - expect(lines).to include("# UNIT counter_metric hotdogs") + # expect(lines).to include("# UNIT counter_metric hotdogs") expect(lines).to include("# HELP counter_metric foo description") end @@ -118,7 +120,7 @@ lines = writer.write.split("\n") expect(lines).to include("# TYPE gauge_with_exemplar gauge") - expect(lines).to include("# UNIT gauge_with_exemplar hotdogs") + # expect(lines).to include("# UNIT gauge_with_exemplar hotdogs") expect(lines).to include("# HELP gauge_with_exemplar bar description\\nwith newline") end @@ -150,7 +152,7 @@ lines = writer.write.split("\n") expect(lines).to include("# TYPE histogram_without_ts histogram") - expect(lines).to include("# UNIT histogram_without_ts hotdogs") + # expect(lines).to include("# UNIT histogram_without_ts hotdogs") expect(lines).to include("# HELP histogram_without_ts xuq description") end @@ -177,65 +179,80 @@ it "generates a sum that equals the sum of all measured event values" it "if there is a negative valued bucket, there should be no sum metric" it "is not clear if we should print the bucket multiple times with different timestamps to expose multiple exemplars" - - it "A Histogram's Metric's LabelSet MUST NOT have a 'le' label name." - it "Bucket values MAY have exemplars. Buckets are cumulative to allow monitoring systems to drop any non-+Inf bucket for performance/anti-denial-of-service reasons in a way that loses granularity but is still a valid Histogram." - it "Each bucket covers the values less and or equal to it, and the value of the exemplar MUST be within this range. Exemplars SHOULD be put into the bucket with the highest value. A bucket MUST NOT have more than one exemplar." + it "Bucket values MAY have exemplars. It is up to us on which bucket gets which exemplar other than the exemplar falling within the bucket range. Docs say exemplars SHOULD be put into the bucket with the highest value." end - describe "gaugehistogram" do - it "generates a metric description" - it "generates a metric without a timestamp" - it "generates a metric with a timestamp" - it "generates a metric with an exemplar" - end + describe "summary" do + let(:summary_metric) do + summary_metric = registry.summary(:summary_metric, + docstring: 'qux description', + labels: [:for, :code], + preset_labels: { for: 'sake', code: '1' }) + 92.times { summary_metric.observe(0) } + summary_metric.observe(1243.21) - describe "stateset" do - it "generates a metric description" - it "generates a metric without a timestamp" - it "generates a metric with a timestamp" - it "generates a metric with an exemplar" + summary_metric + end - it "A point of a StateSet metric MAY contain multiple states and MUST contain one boolean per State. States have a name which are Strings." - it "A StateSet Metric's LabelSet MUST NOT have a label name which is the same as the name of its MetricFamily." - it "If encoded as a StateSet, ENUMs MUST have exactly one Boolean which is true within a MetricPoint." - it "MetricFamilies of type StateSets MUST have an empty Unit string." + it "generates a metric description" do + writer = Prometheus::Client::Formats::OpenMetrics::Writer.new(summary_metric) + + lines = writer.write.split("\n") + + expect(lines).to include("# TYPE summary_metric summary") + expect(lines).to include("# HELP summary_metric qux description") + end + + it "generates a metric" do + writer = Prometheus::Client::Formats::OpenMetrics::Writer.new(summary_metric) + + lines = writer.write.split("\n") + + expect(lines).to include("summary_metric_sum{for=\"sake\",code=\"1\"} 1243.21") + expect(lines).to include("summary_metric_count{for=\"sake\",code=\"1\"} 93.0") + end + + it "generates a metric with an exemplar" + it "generates _created metric point" end - # describe "summary" do - # let(:registry.summary(:summary)) do - # counter_metric = registry.counter(:counter_metric, - # docstring: 'foo description', - # labels: [:umlauts, :utf, :code], - # preset_labels: {umlauts: 'Björn', utf: '佖佥'}) - # counter_metric.increment(labels: { code: 'red'}, by: 42) - # counter_metric.increment(labels: { code: 'green'}, by: 3.14E42) - # counter_metric.increment(labels: { code: 'blue'}, by: 1.23e-45) - # - # counter_metric - # end + # we don't support these types right now so I am punting for now + + # describe "gaugehistogram" do # it "generates a metric description" # it "generates a metric without a timestamp" # it "generates a metric with a timestamp" # it "generates a metric with an exemplar" # end - describe "info" do - it "generates a metric description" - it "generates a metric without a timestamp" - it "generates a metric with a timestamp" - it "generates a metric with an exemplar" + # describe "stateset" do + # it "generates a metric description" + # it "generates a metric without a timestamp" + # it "generates a metric with a timestamp" + # it "generates a metric with an exemplar" + # + # it "A point of a StateSet metric MAY contain multiple states and MUST contain one boolean per State. States have a name which are Strings." + # it "A StateSet Metric's LabelSet MUST NOT have a label name which is the same as the name of its MetricFamily." + # it "If encoded as a StateSet, ENUMs MUST have exactly one Boolean which is true within a MetricPoint." + # it "MetricFamilies of type StateSets MUST have an empty Unit string." + # end - it "A MetricPoint of an Info Metric contains a LabelSet. An Info MetricPoint's LabelSet MUST NOT have a label name which is the same as the name of a label of the LabelSet of its Metric." - it "Info MAY be used to encode ENUMs whose values do not change over time, such as the type of a network interface." - it "MetricFamilies of type Info MUST have an empty Unit string." - end + # describe "info" do + # it "generates a metric description" + # it "generates a metric without a timestamp" + # it "generates a metric with a timestamp" + # it "generates a metric with an exemplar" + # + # it "A MetricPoint of an Info Metric contains a LabelSet. An Info MetricPoint's LabelSet MUST NOT have a label name which is the same as the name of a label of the LabelSet of its Metric." + # it "Info MAY be used to encode ENUMs whose values do not change over time, such as the type of a network interface." + # it "MetricFamilies of type Info MUST have an empty Unit string." + # end - describe "unknown" do - it "generates a metric description" - it "generates a metric without a timestamp" - it "generates a metric with a timestamp" - it "generates a metric with an exemplar" - end + # describe "unknown" do + # it "generates a metric description" + # it "generates a metric without a timestamp" + # it "generates a metric with a timestamp" + # it "generates a metric with an exemplar" + # end end end From 2cb800603899a82e39c3bc5be70b53748dc1ab11 Mon Sep 17 00:00:00 2001 From: Jeremiah Hemphill Date: Tue, 13 Jun 2023 20:32:59 -0400 Subject: [PATCH 14/21] Added exemplar support to summaries but fought a losing battle --- lib/prometheus/client/formats/open_metrics.rb | 9 +++--- lib/prometheus/client/formats/text.rb | 4 +-- lib/prometheus/client/summary.rb | 31 ++++++++++++++++--- lib/prometheus/client/value_with_exemplars.rb | 3 +- .../client/formats/open_metrics_spec.rb | 22 ++++++++----- spec/prometheus/client/summary_spec.rb | 16 +++------- 6 files changed, 55 insertions(+), 30 deletions(-) diff --git a/lib/prometheus/client/formats/open_metrics.rb b/lib/prometheus/client/formats/open_metrics.rb index eb12f522..f98d6452 100644 --- a/lib/prometheus/client/formats/open_metrics.rb +++ b/lib/prometheus/client/formats/open_metrics.rb @@ -90,11 +90,10 @@ def histogram def summary output = [] - metric.values.collect do |label_set, value| - output << metric_line("#{name}_sum", label_set, value["sum"]) - output << metric_line("#{name}_count", label_set, value["count"]) - # created is dependent on the exemplar working - # output << metric_line("#{name}_created", label_set, created) + metric.values.collect do |label_set, vwe| + output << metric_line("#{name}_sum", label_set, vwe.value["sum"], vwe.most_recent_exemplar) + output << metric_line("#{name}_count", label_set, vwe.value["count"], vwe.most_recent_exemplar) + output << metric_line("#{name}_created", label_set, vwe.created) end output diff --git a/lib/prometheus/client/formats/text.rb b/lib/prometheus/client/formats/text.rb index b735389c..5cce700d 100644 --- a/lib/prometheus/client/formats/text.rb +++ b/lib/prometheus/client/formats/text.rb @@ -51,8 +51,8 @@ def representation(metric, label_set, value, &block) def summary(name, set, value) l = labels(set) - yield metric("#{name}_sum", l, value["sum"]) - yield metric("#{name}_count", l, value["count"]) + yield metric("#{name}_sum", l, value.value["sum"]) + yield metric("#{name}_count", l, value.value["count"]) end def histogram(name, set, value) diff --git a/lib/prometheus/client/summary.rb b/lib/prometheus/client/summary.rb index fbbed0a5..3b51b826 100644 --- a/lib/prometheus/client/summary.rb +++ b/lib/prometheus/client/summary.rb @@ -40,13 +40,36 @@ def get(labels: {}) end # Returns all label sets with their values expressed as hashes with their sum/count + # + # Converts from + # { + # {, :quantile=>"count"} => , + # {, :quantile=>"sum"} => + # } + # to + # { + # : {count: , sum: } + # } + # + # This isn't going to work long term because it isn't possible to differentiate between + # exemplars whose value is based on the count and exemplars whose value is based on the sum. + # For now, it is random/unusable. def values - values = @store.all_values + values = @store.all_values(with_exemplars: true) - values.each_with_object({}) do |(label_set, v), acc| + values.each_with_object({}) do |(label_set, value_with_exemplars), acc| actual_label_set = label_set.reject{|l| l == :quantile } - acc[actual_label_set] ||= { "count" => 0.0, "sum" => 0.0 } - acc[actual_label_set][label_set[:quantile]] = v + + if acc.has_key? actual_label_set + acc[actual_label_set].value[label_set[:quantile]] = value_with_exemplars.value + else + new_vwe = ValueWithExemplars.new + value = { "count" => 0.0, "sum" => 0.0 }.merge({ label_set[:quantile] => value_with_exemplars.value }) + new_vwe.value = value + new_vwe.exemplars = value_with_exemplars.exemplars # only copy over the exemplars once because both quantiles have the same exemplars + + acc[actual_label_set] = new_vwe + end end end diff --git a/lib/prometheus/client/value_with_exemplars.rb b/lib/prometheus/client/value_with_exemplars.rb index f7f1077a..ca7e6b3b 100644 --- a/lib/prometheus/client/value_with_exemplars.rb +++ b/lib/prometheus/client/value_with_exemplars.rb @@ -9,7 +9,8 @@ module Client # # no idea how this is going to work with histograms class ValueWithExemplars - attr_reader :value, :exemplar, :created + # changed from reader to accessor to make object setup easier for summaries + attr_accessor :value, :exemplars, :created def initialize @exemplars = ExemplarCollection.new diff --git a/spec/prometheus/client/formats/open_metrics_spec.rb b/spec/prometheus/client/formats/open_metrics_spec.rb index d4d6ece3..88e58a6f 100644 --- a/spec/prometheus/client/formats/open_metrics_spec.rb +++ b/spec/prometheus/client/formats/open_metrics_spec.rb @@ -93,7 +93,6 @@ writer = Prometheus::Client::Formats::OpenMetrics::Writer.new(counter_with_exemplars) lines = writer.write.split("\n") - puts lines # this is hard to test red_created = counter_with_exemplars.values(with_exemplars: true)[{:umlauts=>"Björn", :utf=>"佖佥", :code=>"red"}].created @@ -188,7 +187,10 @@ docstring: 'qux description', labels: [:for, :code], preset_labels: { for: 'sake', code: '1' }) - 92.times { summary_metric.observe(0) } + 80.times { summary_metric.observe(0) } + summary_metric.observe(0, exemplar: Prometheus::Client::Exemplar.new(labels: {trace_id: 12345}, timestamp: 1000)) + 10.times { summary_metric.observe(0) } + summary_metric.observe(0, exemplar: Prometheus::Client::Exemplar.new(labels: {trace_id: 23456}, timestamp: 2000)) summary_metric.observe(1243.21) summary_metric @@ -203,17 +205,23 @@ expect(lines).to include("# HELP summary_metric qux description") end - it "generates a metric" do + it "generates metrics with exemplars" do writer = Prometheus::Client::Formats::OpenMetrics::Writer.new(summary_metric) lines = writer.write.split("\n") - expect(lines).to include("summary_metric_sum{for=\"sake\",code=\"1\"} 1243.21") - expect(lines).to include("summary_metric_count{for=\"sake\",code=\"1\"} 93.0") + expect(lines).to include("summary_metric_sum{for=\"sake\",code=\"1\"} 1243.21 # {trace_id=\"23456\"} 0.0 2000") + expect(lines).to include("summary_metric_count{for=\"sake\",code=\"1\"} 93.0 # {trace_id=\"23456\"} 0.0 2000") end - it "generates a metric with an exemplar" - it "generates _created metric point" + it "generates _created metric point" do + writer = Prometheus::Client::Formats::OpenMetrics::Writer.new(summary_metric) + + lines = writer.write.split("\n") + + # can't test against the timestamp + expect(lines).to include(match /summary_metric_created{for="sake",code="1"}/) + end end # we don't support these types right now so I am punting for now diff --git a/spec/prometheus/client/summary_spec.rb b/spec/prometheus/client/summary_spec.rb index ba02ad36..fb3c7e26 100644 --- a/spec/prometheus/client/summary_spec.rb +++ b/spec/prometheus/client/summary_spec.rb @@ -79,10 +79,8 @@ summary.observe(3, labels: { status: 'bar' }) summary.observe(5, labels: { status: 'foo' }) - expect(summary.values).to eql( - { status: 'bar' } => { "count" => 1.0, "sum" => 3.0 }, - { status: 'foo' } => { "count" => 1.0, "sum" => 5.0 }, - ) + expect(summary.values[{ status: 'bar' }].value).to eql({ "count" => 1.0, "sum" => 3.0 }) + expect(summary.values[{ status: 'foo' }].value).to eql({ "count" => 1.0, "sum" => 5.0 }) end end @@ -96,18 +94,14 @@ 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[{ status: 'bar' }].value).to eql({ "count" => 0.0, "sum" => 0.0 }) + expect(summary.values[{ status: 'foo' }].value).to eql({ "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 }, - ) + expect(summary.values[{}].value).to eql({ "count" => 0.0, "sum" => 0.0 }) end end end From 3c134e6a405b090b06a610cbec540cfecca9f6f5 Mon Sep 17 00:00:00 2001 From: Jeremiah Hemphill Date: Wed, 14 Jun 2023 15:15:58 -0400 Subject: [PATCH 15/21] Fixing weird file load issues and require problems --- lib/prometheus/client/data_stores/single_threaded.rb | 2 +- lib/prometheus/client/data_stores/synchronized.rb | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/prometheus/client/data_stores/single_threaded.rb b/lib/prometheus/client/data_stores/single_threaded.rb index fc58703e..6fec1042 100644 --- a/lib/prometheus/client/data_stores/single_threaded.rb +++ b/lib/prometheus/client/data_stores/single_threaded.rb @@ -29,7 +29,7 @@ def validate_metric_settings(metric_settings:) class MetricStore def initialize - @internal_store = Hash.new { |hash, key| hash[key] = ValueWithExemplars.new } + @internal_store = Hash.new { |hash, key| hash[key] = Prometheus::Client::ValueWithExemplars.new } end def synchronize diff --git a/lib/prometheus/client/data_stores/synchronized.rb b/lib/prometheus/client/data_stores/synchronized.rb index 868a0ef3..450cb118 100644 --- a/lib/prometheus/client/data_stores/synchronized.rb +++ b/lib/prometheus/client/data_stores/synchronized.rb @@ -1,3 +1,8 @@ +# no idea why this is reuired but giving up for now +require 'prometheus/client/exemplar' +require 'prometheus/client/exemplar_collection' +require 'prometheus/client/value_with_exemplars' + module Prometheus module Client module DataStores @@ -24,7 +29,7 @@ def validate_metric_settings(metric_settings:) class MetricStore def initialize - @internal_store = Hash.new { |hash, key| hash[key] = ValueWithExemplars.new } + @internal_store = Hash.new { |hash, key| hash[key] = Prometheus::Client::ValueWithExemplars.new } @lock = Monitor.new end From 172ee636c98d27e24d84f2e607bd4f03a05f74a4 Mon Sep 17 00:00:00 2001 From: Jeremiah Hemphill Date: Thu, 15 Jun 2023 12:50:47 -0400 Subject: [PATCH 16/21] Turn on open metrics --- lib/prometheus/middleware/exporter.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/prometheus/middleware/exporter.rb b/lib/prometheus/middleware/exporter.rb index bad5189c..82051d60 100644 --- a/lib/prometheus/middleware/exporter.rb +++ b/lib/prometheus/middleware/exporter.rb @@ -15,8 +15,8 @@ module Middleware class Exporter attr_reader :app, :registry, :path - FORMATS = [Client::Formats::Text].freeze - FALLBACK = Client::Formats::Text + FORMATS = [Client::Formats::OpenMetrics].freeze + FALLBACK = Client::Formats::OpenMetrics def initialize(app, options = {}) @app = app From 31a388b9525efae27f000370aec418105f20331d Mon Sep 17 00:00:00 2001 From: Jeremiah Hemphill Date: Thu, 15 Jun 2023 12:53:44 -0400 Subject: [PATCH 17/21] Enabled open metrics and found some more broken stuff but I need to go to lunch --- lib/prometheus/client/formats/open_metrics.rb | 3 ++- lib/prometheus/client/formats/text.rb | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/prometheus/client/formats/open_metrics.rb b/lib/prometheus/client/formats/open_metrics.rb index f98d6452..31fc2f56 100644 --- a/lib/prometheus/client/formats/open_metrics.rb +++ b/lib/prometheus/client/formats/open_metrics.rb @@ -8,6 +8,7 @@ module OpenMetrics MEDIA_TYPE = 'text/plain'.freeze VERSION = '0.0.1'.freeze CONTENT_TYPE = "#{MEDIA_TYPE}; version=#{VERSION}".freeze + DELIMITER = "\n".freeze # public interface to generate out the /metrics payload def self.marshal(registry) @@ -15,7 +16,7 @@ def self.marshal(registry) registry.metrics.each do |metric| # generate metric and put it in lines - lines << Writer.new(metric).to_open_metrics + lines << Writer.new(metric).metrics_to_a end (lines << nil).join(DELIMITER) diff --git a/lib/prometheus/client/formats/text.rb b/lib/prometheus/client/formats/text.rb index 5cce700d..40ff092b 100644 --- a/lib/prometheus/client/formats/text.rb +++ b/lib/prometheus/client/formats/text.rb @@ -6,7 +6,7 @@ module Formats # Text format is human readable mainly used for manual inspection. module Text MEDIA_TYPE = 'text/plain'.freeze - VERSION = '0.0.4'.freeze + VERSION = '0.0.1'.freeze CONTENT_TYPE = "#{MEDIA_TYPE}; version=#{VERSION}".freeze METRIC_LINE = '%s%s %s'.freeze From 452dff68abfaf6a593af7f88780b940eeaa863fd Mon Sep 17 00:00:00 2001 From: Jeremiah Hemphill Date: Thu, 15 Jun 2023 14:38:49 -0400 Subject: [PATCH 18/21] unfarting the code --- lib/prometheus/client/formats/open_metrics.rb | 6 +++--- lib/prometheus/middleware/exporter.rb | 4 +++- spec/prometheus/middleware/exporter_spec.rb | 7 +++++-- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/lib/prometheus/client/formats/open_metrics.rb b/lib/prometheus/client/formats/open_metrics.rb index 31fc2f56..02e15557 100644 --- a/lib/prometheus/client/formats/open_metrics.rb +++ b/lib/prometheus/client/formats/open_metrics.rb @@ -16,10 +16,10 @@ def self.marshal(registry) registry.metrics.each do |metric| # generate metric and put it in lines - lines << Writer.new(metric).metrics_to_a + lines << Writer.new(metric).write end - (lines << nil).join(DELIMITER) + (lines.flatten.compact << nil).join(DELIMITER) end class Writer @@ -165,7 +165,7 @@ def description end def write - (description + metrics_to_a).join("\n") + (description + metrics_to_a).flatten.join(Prometheus::Client::Formats::OpenMetrics::DELIMITER) end end diff --git a/lib/prometheus/middleware/exporter.rb b/lib/prometheus/middleware/exporter.rb index 82051d60..a9c0bda9 100644 --- a/lib/prometheus/middleware/exporter.rb +++ b/lib/prometheus/middleware/exporter.rb @@ -15,6 +15,8 @@ module Middleware class Exporter attr_reader :app, :registry, :path + # this file does not support multiple formats + # I am officially giving up FORMATS = [Client::Formats::OpenMetrics].freeze FALLBACK = Client::Formats::OpenMetrics @@ -78,7 +80,7 @@ def not_acceptable(formats) [ 406, { 'content-type' => 'text/plain' }, - ["Supported media types: #{types.join(', ')}"], + ["Supported media types: #{types.uniq.join(', ')}"], ] end diff --git a/spec/prometheus/middleware/exporter_spec.rb b/spec/prometheus/middleware/exporter_spec.rb index e8232fc5..723ecb63 100644 --- a/spec/prometheus/middleware/exporter_spec.rb +++ b/spec/prometheus/middleware/exporter_spec.rb @@ -26,13 +26,16 @@ end context 'when requesting /metrics' do - text = Prometheus::Client::Formats::Text + # I am pretty annoyed at this hard coded format right now + # it should match the list of formats in the exporter and loop over the available ones + text = Prometheus::Client::Formats::OpenMetrics shared_examples 'ok' do |headers, fmt| it "responds with 200 OK and content-type #{fmt::CONTENT_TYPE}" do registry.counter(:foo, docstring: 'foo counter').increment(by: 9) get '/metrics', nil, headers + # require "pry"; binding.pry expect(last_response.status).to eql(200) expect(last_response.headers['content-type']).to eql(fmt::CONTENT_TYPE) @@ -69,7 +72,7 @@ end context 'when client uses different white spaces in Accept header' do - accept = 'text/plain;q=1.0 ; version=0.0.4' + accept = 'text/plain;q=1.0 ; version=0.0.1' # why is this version hard coded? include_examples 'ok', { 'HTTP_ACCEPT' => accept }, text end From 641d976100654b50b710478f2d6ace07c2890b9b Mon Sep 17 00:00:00 2001 From: Jeremiah Hemphill Date: Thu, 15 Jun 2023 14:56:20 -0400 Subject: [PATCH 19/21] added an exemplar empty method --- lib/prometheus/client/exemplar.rb | 7 ++++++- lib/prometheus/client/formats/open_metrics.rb | 2 +- spec/prometheus/client/exemplar_spec.rb | 9 +++++++++ spec/prometheus/client/formats/open_metrics_spec.rb | 2 ++ 4 files changed, 18 insertions(+), 2 deletions(-) diff --git a/lib/prometheus/client/exemplar.rb b/lib/prometheus/client/exemplar.rb index c594c038..0d6138ec 100644 --- a/lib/prometheus/client/exemplar.rb +++ b/lib/prometheus/client/exemplar.rb @@ -6,7 +6,8 @@ module Client # Store a snapshot of metric data including an extra set of kv pairs for a specific moment in # time and specific moment of code execution. # - # Value is expected to be set after the exemplar is initialized. + # Value is expected to be set after the exemplar is initialized. Exemplars without a value will + # not be exported. class Exemplar # The kv pairs that make up the unique information in the exemplar. # @@ -24,6 +25,10 @@ def initialize(labels: {}, timestamp: nil) @labels = labels @timestamp = timestamp || Time.now.to_i end + + def empty? + value.nil? + end end end end diff --git a/lib/prometheus/client/formats/open_metrics.rb b/lib/prometheus/client/formats/open_metrics.rb index 02e15557..147915b4 100644 --- a/lib/prometheus/client/formats/open_metrics.rb +++ b/lib/prometheus/client/formats/open_metrics.rb @@ -126,7 +126,7 @@ def counter def metric_line(name, label_set, value, exemplar = nil) output = "#{name}#{labels(label_set)} #{value}" output += " #{timestamp}" if timestamp - output += " # #{labels(exemplar.labels)} #{exemplar.value} #{exemplar.timestamp}" if exemplar + output += " # #{labels(exemplar.labels)} #{exemplar.value} #{exemplar.timestamp}" if exemplar && !exemplar.empty? output end diff --git a/spec/prometheus/client/exemplar_spec.rb b/spec/prometheus/client/exemplar_spec.rb index f69dd91e..b3c48e01 100644 --- a/spec/prometheus/client/exemplar_spec.rb +++ b/spec/prometheus/client/exemplar_spec.rb @@ -14,4 +14,13 @@ expect(e.timestamp).to be <= Time.now.to_i end + + it "is empty" do + e = described_class.new + + expect(e).to be_empty + + e.value = 5 + expect(e).not_to be_empty + end end diff --git a/spec/prometheus/client/formats/open_metrics_spec.rb b/spec/prometheus/client/formats/open_metrics_spec.rb index 88e58a6f..5431970d 100644 --- a/spec/prometheus/client/formats/open_metrics_spec.rb +++ b/spec/prometheus/client/formats/open_metrics_spec.rb @@ -100,6 +100,8 @@ blue_created = counter_with_exemplars.values(with_exemplars: true)[{:umlauts=>"Björn", :utf=>"佖佥", :code=>"blue"}].created expect(lines).to include("counter_with_exemplars_created{umlauts=\"Björn\",utf=\"佖佥\",code=\"blue\"} #{blue_created}") end + + it "does not write an exemplar with no value" end describe "gauge" do From 5865d9557e7ab06759d83e42000321b14a2c2772 Mon Sep 17 00:00:00 2001 From: Jeremiah Hemphill Date: Fri, 16 Jun 2023 10:30:47 -0400 Subject: [PATCH 20/21] Remove potential issue with counters ending in _total and turn on open metrics again --- lib/prometheus/client/formats/open_metrics.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/prometheus/client/formats/open_metrics.rb b/lib/prometheus/client/formats/open_metrics.rb index 147915b4..0b99f776 100644 --- a/lib/prometheus/client/formats/open_metrics.rb +++ b/lib/prometheus/client/formats/open_metrics.rb @@ -29,7 +29,7 @@ def initialize(metric) end def name - metric.name + metric.name.to_s end def docstring @@ -110,6 +110,10 @@ def counter exemplar = value_with_exemplars.most_recent_exemplar created = value_with_exemplars.created + # hack in support to remove _total from all of our counters because _total is now a + # reserved word + metric_name = name.gsub(/_total$/, "") + output << metric_line(name, label_set, value, exemplar) output << metric_line("#{name}_created", label_set, created) From 3121673cb7a5302c0866a34835cc4bc090cdbb8a Mon Sep 17 00:00:00 2001 From: Jeremiah Hemphill Date: Fri, 16 Jun 2023 10:39:12 -0400 Subject: [PATCH 21/21] Fixed content type, fixed eof, removed some hard coded text/plain tests --- lib/prometheus/client/formats/open_metrics.rb | 9 ++-- spec/prometheus/middleware/exporter_spec.rb | 47 ++++++++++--------- 2 files changed, 29 insertions(+), 27 deletions(-) diff --git a/lib/prometheus/client/formats/open_metrics.rb b/lib/prometheus/client/formats/open_metrics.rb index 0b99f776..29094e96 100644 --- a/lib/prometheus/client/formats/open_metrics.rb +++ b/lib/prometheus/client/formats/open_metrics.rb @@ -5,10 +5,11 @@ module Client module Formats module OpenMetrics # used by the middleware to determine if this format works for the request - MEDIA_TYPE = 'text/plain'.freeze - VERSION = '0.0.1'.freeze - CONTENT_TYPE = "#{MEDIA_TYPE}; version=#{VERSION}".freeze + MEDIA_TYPE = 'application/openmetrics-text'.freeze + VERSION = '1.0.0'.freeze + CONTENT_TYPE = "#{MEDIA_TYPE}; version=#{VERSION}; charset=utf-8".freeze DELIMITER = "\n".freeze + EOF = "# EOF\n" # public interface to generate out the /metrics payload def self.marshal(registry) @@ -19,7 +20,7 @@ def self.marshal(registry) lines << Writer.new(metric).write end - (lines.flatten.compact << nil).join(DELIMITER) + (lines.flatten.compact << EOF).join(DELIMITER) end class Writer diff --git a/spec/prometheus/middleware/exporter_spec.rb b/spec/prometheus/middleware/exporter_spec.rb index 723ecb63..0b5b5363 100644 --- a/spec/prometheus/middleware/exporter_spec.rb +++ b/spec/prometheus/middleware/exporter_spec.rb @@ -35,7 +35,6 @@ registry.counter(:foo, docstring: 'foo counter').increment(by: 9) get '/metrics', nil, headers - # require "pry"; binding.pry expect(last_response.status).to eql(200) expect(last_response.headers['content-type']).to eql(fmt::CONTENT_TYPE) @@ -45,7 +44,7 @@ shared_examples 'not acceptable' do |headers| it 'responds with 406 Not Acceptable' do - message = 'Supported media types: text/plain' + message = 'Supported media types: application/openmetrics-text' get '/metrics', nil, headers @@ -67,27 +66,29 @@ include_examples 'not acceptable', 'HTTP_ACCEPT' => 'application/json' end - context 'when client requests text/plain' do - include_examples 'ok', { 'HTTP_ACCEPT' => 'text/plain' }, text - end - - context 'when client uses different white spaces in Accept header' do - accept = 'text/plain;q=1.0 ; version=0.0.1' # why is this version hard coded? - - include_examples 'ok', { 'HTTP_ACCEPT' => accept }, text - end - - context 'when client does not include quality attribute' do - accept = 'application/json;q=0.5, text/plain' - - include_examples 'ok', { 'HTTP_ACCEPT' => accept }, text - end - - context 'when client accepts some unknown formats' do - accept = 'text/plain;q=0.3, proto/buf;q=0.7' - - include_examples 'ok', { 'HTTP_ACCEPT' => accept }, text - end + # the openmetrics type does not accept text plain + # not sure if this is good or bad + # context 'when client requests text/plain' do + # include_examples 'ok', { 'HTTP_ACCEPT' => 'text/plain' }, text + # end + # + # context 'when client uses different white spaces in Accept header' do + # accept = 'text/plain;q=1.0 ; version=0.0.1' # why is this version hard coded? + # + # include_examples 'ok', { 'HTTP_ACCEPT' => accept }, text + # end + # + # context 'when client does not include quality attribute' do + # accept = 'application/json;q=0.5, text/plain' + # + # include_examples 'ok', { 'HTTP_ACCEPT' => accept }, text + # end + # + # context 'when client accepts some unknown formats' do + # accept = 'text/plain;q=0.3, proto/buf;q=0.7' + # + # include_examples 'ok', { 'HTTP_ACCEPT' => accept }, text + # end context 'when client accepts only unknown formats' do accept = 'fancy/woo;q=0.3, proto/buf;q=0.7'