Merge 11a2813 into ff8a62b

lib/prometheus/middleware/collector.rb

@@ -32,8 +32,6 @@ def initialize(app, options = {})
         @app = app
         @registry = options[:registry] || Client.registry
         @metrics_prefix = options[:metrics_prefix] || 'http_server'
-        @counter_lb = options[:counter_label_builder] || COUNTER_LB
-        @duration_lb = options[:duration_label_builder] || DURATION_LB
 
         init_request_metrics
         init_exception_metrics
@@ -45,42 +43,17 @@ def call(env) # :nodoc:
 
       protected
 
-      aggregation = lambda do |str|
-        str
-          .gsub(%r{/[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}(/|$)}, '/:uuid\\1')
-          .gsub(%r{/\d+(/|$)}, '/:id\\1')
-      end
-
-      COUNTER_LB = proc do |env, code|
-        next { code: nil, method: nil, path: nil } if env.empty?
-
-        {
-          code:   code,
-          method: env['REQUEST_METHOD'].downcase,
-          path:   aggregation.call(env['PATH_INFO']),
-        }
-      end
-
-      DURATION_LB = proc do |env, _|
-        next { method: nil, path: nil } if env.empty?
-
-        {
-          method: env['REQUEST_METHOD'].downcase,
-          path:   aggregation.call(env['PATH_INFO']),
-        }
-      end
-
       def init_request_metrics
         @requests = @registry.counter(
           :"#{@metrics_prefix}_requests_total",
           docstring:
             'The total number of HTTP requests handled by the Rack application.',
-          labels: @counter_lb.call({}, "").keys
+          labels: %i[code method path]
         )
         @durations = @registry.histogram(
           :"#{@metrics_prefix}_request_duration_seconds",
           docstring: 'The HTTP response duration of the Rack application.',
-          labels: @duration_lb.call({}, "").keys
+          labels: %i[method path]
         )
       end
 
@@ -103,12 +76,29 @@ def trace(env)
       end
 
       def record(env, code, duration)
-        @requests.increment(labels: @counter_lb.call(env, code))
-        @durations.observe(duration, labels: @duration_lb.call(env, code))
+        counter_labels = {
+          code:   code,
+          method: env['REQUEST_METHOD'].downcase,
+          path:   strip_ids_from_path(env['PATH_INFO']),
+        }
+
+        duration_labels = {
+          method: env['REQUEST_METHOD'].downcase,
+          path:   strip_ids_from_path(env['PATH_INFO']),
+        }
+
+        @requests.increment(labels: counter_labels)
+        @durations.observe(duration, labels: duration_labels)
       rescue
         # TODO: log unexpected exception during request recording
         nil
       end
+
+      def strip_ids_from_path(path)
+        path
+          .gsub(%r{/[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}(/|$)}, '/:uuid\\1')
+          .gsub(%r{/\d+(/|$)}, '/:id\\1')
+      end
     end
   end
 end

spec/prometheus/middleware/collector_spec.rb

@@ -23,6 +23,8 @@
     described_class.new(original_app, registry: registry)
   end
 
+  let(:dummy_error) { RuntimeError.new("Dummy error from tests") }
+
   it 'returns the app response' do
     get '/foo'
 
@@ -32,7 +34,7 @@
 
   it 'handles errors in the registry gracefully' do
     counter = registry.get(:http_server_requests_total)
-    expect(counter).to receive(:increment).and_raise(NoMethodError)
+    expect(counter).to receive(:increment).and_raise(dummy_error)
 
     get '/foo'
 
@@ -84,7 +86,7 @@
   context 'when the app raises an exception' do
     let(:original_app) do
       lambda do |env|
-        raise NoMethodError if env['PATH_INFO'] == '/broken'
+        raise dummy_error if env['PATH_INFO'] == '/broken'
 
         [200, { 'Content-Type' => 'text/html' }, ['OK']]
       end
@@ -95,35 +97,10 @@
     end
 
     it 'traces exceptions' do
-      expect { get '/broken' }.to raise_error NoMethodError
+      expect { get '/broken' }.to raise_error RuntimeError
 
       metric = :http_server_exceptions_total
-      labels = { exception: 'NoMethodError' }
-      expect(registry.get(metric).get(labels: labels)).to eql(1.0)
-    end
-  end
-
-  context 'when using a custom counter label builder' do
-    let(:app) do
-      described_class.new(
-        original_app,
-        registry: registry,
-        counter_label_builder: lambda do |env, code|
-          next { code: nil, method: nil } if env.empty?
-
-          {
-            code:   code,
-            method: env['REQUEST_METHOD'].downcase,
-          }
-        end,
-      )
-    end
-
-    it 'allows labels configuration' do
-      get '/foo/bar'
-
-      metric = :http_server_requests_total
-      labels = { method: 'get', code: '200' }
+      labels = { exception: 'RuntimeError' }
       expect(registry.get(metric).get(labels: labels)).to eql(1.0)
     end
   end