More http_server_request_duration_seconds buckets

Add more buckets to http_server_request_duration_seconds.
10 seconds is unfortunately not long enough.

The weirdness with the Prometheus::Middleware::Collector
initializing the registry is avoided by creating a
subclass that assumes a the registry is already
populated, and fetches the metrics object from it.

The exceptions were exporting malformed metrics.  Rather
than redefining the #trace method, the RegisteredCollector
defaults to an object that shares enough of the same interface
 as a metric to fool #trace and #record.  Because the FakeMetric
is not part of the registry, the exporter does not attempt to
export them.

config.ru

@@ -5,9 +5,8 @@ end
 
 ENV["APP_ENV"] ||= ENV["RAILS_ENV"]
 
-if ENV["PROMETHEUS_EXPORTER_URL"]
-  use Prometheus::Middleware::Collector
-end
+require "prometheus/middleware/registered_collector"
+use Prometheus::Middleware::RegisteredCollector if ENV["PROMETHEUS_EXPORTER_URL"]
 
 Bundler.require
 Spectrum::Json.configure(__dir__, ENV["RAILS_RELATIVE_URL_ROOT"])

config/puma.rb

@@ -49,19 +49,29 @@
   if ENV["PROMETHEUS_EXPORTER_URL"]
     Bundler.require :yabeda
 
+    Prometheus::Client.config.data_store =
+      Prometheus::Client::DataStores::DirectFileStore.new(dir: ENV["PROMETHEUS_MONITORING_DIR"])
+
+    # Set up the client registry here so everyone gets a copy
+    # We will use a RegisteredCollector so that we don't have to monkey with the registry
+    Prometheus::Client.registry.counter(
+      :http_server_requests_total,
+      docstring: "The total number of HTTP requests handled by the Rack application.",
+      labels: %i[code method path]
+    )
+    Prometheus::Client.registry.histogram(
+      :http_server_request_duration_seconds,
+      docstring: "The HTTP response duration of the Rack application.",
+      labels: %i[method path],
+      buckets: Prometheus::Client::Histogram::DEFAULT_BUCKETS + [15, 20, 30]
+    )
+    Yabeda.configure!
+
     plugin :yabeda
     plugin :yabeda_prometheus
     prometheus_exporter_url ENV["PROMETHEUS_EXPORTER_URL"]
 
     on_worker_boot do
-      # Prometheus::Middleware::Collector.new has side effects that registers
-      # metrics.  If they are already loaded it raises an exception.
-      if Prometheus::Client.registry.exist?(:http_server_requests_total)
-        Prometheus::Client.registry.unregister(:http_server_requests_total)
-        Prometheus::Client.registry.unregister(:http_server_request_duration_seconds)
-        Prometheus::Client.registry.unregister(:http_server_exceptions_total)
-      end
-
       uri = URI(ENV["PROMETHEUS_EXPORTER_URL"])
       ObjectSpace.each_object(TCPServer).each do |server|
         next if server.closed?
@@ -74,45 +84,10 @@
 
     if (monitoring_dir = ENV["PROMETHEUS_MONITORING_DIR"])
       on_prometheus_exporter_boot do
-        # http_server_exceptions_total was generating invalid metrics for us
-        if Prometheus::Client.registry.exist?(:http_server_exceptions_total)
-          Prometheus::Client.registry.unregister(:http_server_exceptions_total)
+        Dir[File.join(monitoring_dir, "*.bin")].each do |file_path|
+          File.unlink(file_path)
         end
-        # In clustered mode, the worker processes get these registered, but the coordinating process does not.
-        # Ending up in these stats being collected, but not reported.
-        unless Prometheus::Client.registry.exist?(:http_server_requests_total)
-          # These are copied from Prometheus::Middleware::Collector#init_request_metrics
-          # and  Prometheus::Middleware::Collector#init_exception_metrics
-          Prometheus::Client.registry.counter(
-            :http_server_requests_total,
-            docstring: "The total number of HTTP requests handled by the Rack application.",
-            labels: %i[code method path]
-          )
-          Prometheus::Client.registry.histogram(
-            :http_server_request_duration_seconds,
-            docstring: "The HTTP response duration of the Rack application.",
-            labels: %i[method path]
-          )
-          # http_server_exceptions_total was generating invalid metrics for us.
-          # Prometheus::Client.registry.histogram(
-          #   :http_server_exceptions_total,
-          #   docstring: "The total number of exceptions raised by the Rack application.",
-          #   labels: [:exception]
-          # )
-
-          Dir[File.join(monitoring_dir, "*.bin")].each do |file_path|
-            File.unlink(file_path)
-          end
-
-          Yabeda.configure!
-        end
-      end
-
-      before_fork do
-        Prometheus::Client.config.data_store =
-          Prometheus::Client::DataStores::DirectFileStore.new(dir: monitoring_dir)
       end
     end
-
   end
 end

lib/prometheus/middleware/registered_collector.rb

@@ -0,0 +1,27 @@
+module Prometheus
+  module Middleware
+    class RegisteredCollector < Collector
+      protected
+
+      def init_request_metrics
+        requests_name = :"#{@metrics_prefix}_requests_total"
+        durations_name = :"#{@metrics_prefix}_request_duration_seconds"
+        @requests = @registry.get(requests_name) || FakeMetrics
+        @durations = @registry.get(durations_name) || FakeMetrics
+      end
+
+      def init_exception_metrics
+        exceptions_name = :"#{@metrics_prefix}_exceptions_total"
+        @exceptions = @registry.get(exceptions_name) || FakeMetrics
+      end
+
+      class FakeMetrics
+        def self.increment(*)
+        end
+
+        def self.observe(*)
+        end
+      end
+    end
+  end
+end