Forbid varying label sets

lib/prometheus/client/label_set_validator.rb

@@ -30,13 +30,23 @@ def valid?(labels)
       end
 
       def validate(labels)
-        @validated[labels.hash] ||= valid?(labels)
+        return labels if @validated.key?(labels.hash)
 
-        labels
+        valid?(labels)
+
+        unless @validated.empty? || match?(labels, @validated.first.last)
+          fail InvalidLabelSetError, 'labels must have the same signature'
+        end
+
+        @validated[labels.hash] = labels
       end
 
       private
 
+      def match?(a, b)
+        a.keys.sort == b.keys.sort
+      end
+
       def validate_symbol(key)
         return true if key.is_a?(Symbol)
 

lib/prometheus/client/metric.rb

@@ -29,7 +29,9 @@ def type
 
       # Returns the value for the given label set
       def get(labels = {})
-        @values[label_set_for(labels)]
+        @validator.valid?(labels)
+
+        @values[labels]
       end
 
       # Returns all label sets with their values

lib/prometheus/client/rack/collector.rb

@@ -20,7 +20,8 @@ def initialize(app, options = {}, &label_builder)
             }
           end
 
-          init
+          init_request_metrics
+          init_exception_metrics
         end
 
         def call(env) # :nodoc:
@@ -29,10 +30,10 @@ def call(env) # :nodoc:
 
         protected
 
-        def init
+        def init_request_metrics
           @requests = @registry.counter(
             :http_requests_total,
-            'A counter of the total number of HTTP requests made',)
+            'A counter of the total number of HTTP requests made.',)
           @requests_duration = @registry.counter(
             :http_request_durations_total_microseconds,
             'The total amount of time spent answering HTTP requests ' \
@@ -42,28 +43,27 @@ def init
             'A histogram of the response latency (microseconds).',)
         end
 
+        def init_exception_metrics
+          @exceptions = @registry.counter(
+            :http_exceptions_total,
+            'A counter of the total number of exceptions raised.',)
+        end
+
         def trace(env)
           start = Time.now
-          response = yield
+          yield.tap do |response|
+            duration = ((Time.now - start) * 1_000_000).to_i
+            record(labels(env, response), duration)
+          end
         rescue => exception
+          @exceptions.increment(exception: exception.class.name)
           raise
-        ensure
-          duration = ((Time.now - start) * 1_000_000).to_i
-          record(labels(env, response, exception), duration)
         end
 
-        def labels(env, response, exception)
-          labels = @label_builder.call(env)
-
-          if response
+        def labels(env, response)
+          @label_builder.call(env).tap do |labels|
             labels[:code] = response.first.to_s
-          else
-            labels[:exception] = exception.class.name
           end
-
-          labels
-        rescue
-          nil
         end
 
         def record(labels, duration)

lib/prometheus/client/summary.rb

@@ -35,8 +35,10 @@ def add(labels, value)
 
       # Returns the value for the given label set
       def get(labels = {})
+        @validator.valid?(labels)
+
         synchronize do
-          Value.new(@values[label_set_for(labels)])
+          Value.new(@values[labels])
         end
       end
 

spec/prometheus/client/label_set_validator_spec.rb

@@ -57,5 +57,13 @@
 
       expect { validator.validate(input) }.to raise_exception(invalid)
     end
+
+    it 'raises InvalidLabelSetError for varying label sets' do
+      validator.validate(method: 'get', code: '200')
+
+      expect do
+        validator.validate(method: 'get', exception: 'NoMethodError')
+      end.to raise_exception(invalid)
+    end
   end
 end

spec/prometheus/client/rack/collector_spec.rb

@@ -51,15 +51,25 @@
 
   context 'when the app raises an exception' do
     let(:original_app) do
-      ->(_) { fail NoMethodError }
+      lambda do |env|
+        if env['PATH_INFO'] == '/broken'
+          fail NoMethodError
+        else
+          [200, { 'Content-Type' => 'text/html' }, ['OK']]
+        end
+      end
+    end
+
+    before do
+      get '/foo'
     end
 
     it 'traces exceptions' do
-      labels = { method: 'get', path: '/foo', exception: 'NoMethodError' }
+      labels = { exception: 'NoMethodError' }
 
-      expect { get '/foo' }.to raise_error NoMethodError
+      expect { get '/broken' }.to raise_error NoMethodError
 
-      expect(registry.get(:http_requests_total).get(labels)).to eql(1)
+      expect(registry.get(:http_exceptions_total).get(labels)).to eql(1)
     end
   end