Refactor label set validation

The previous pseudo class LabelSet didn't communicate the behavior very
well. This change introduces a real validator class. This does not only
make the validation character clear, but will also allow it to validate
label sets per metric object, instead of using the previous global
lookup table.

lib/prometheus/client/label_set_validator.rb

@@ -0,0 +1,59 @@
+# encoding: UTF-8
+
+module Prometheus
+  module Client
+    # LabelSetValidator ensures that all used label sets comply with the
+    # Prometheus specification.
+    class LabelSetValidator
+      # TODO: we might allow setting :instance in the future
+      RESERVED_LABELS = [:job, :instance]
+
+      class LabelSetError        < StandardError; end
+      class InvalidLabelSetError < LabelSetError; end
+      class InvalidLabelError    < LabelSetError; end
+      class ReservedLabelError   < LabelSetError; end
+
+      def initialize
+        @validated = {}
+      end
+
+      def valid?(labels)
+        unless labels.respond_to?(:all?)
+          fail InvalidLabelSetError, "#{labels} is not a valid label set"
+        end
+
+        labels.all? do |key, _|
+          validate_symbol(key)
+          validate_name(key)
+          validate_reserved_key(key)
+        end
+      end
+
+      def validate(labels)
+        @validated[labels.hash] ||= valid?(labels)
+
+        labels
+      end
+
+      private
+
+      def validate_symbol(key)
+        return true if key.is_a?(Symbol)
+
+        fail InvalidLabelError, "label #{key} is not a symbol"
+      end
+
+      def validate_name(key)
+        return true unless key.to_s.start_with?('__')
+
+        fail ReservedLabelError, "label #{key} must not start with __"
+      end
+
+      def validate_reserved_key(key)
+        return true unless RESERVED_LABELS.include?(key)
+
+        fail ReservedLabelError, "#{key} is reserved"
+      end
+    end
+  end
+end

lib/prometheus/client/metric.rb

@@ -1,7 +1,7 @@
 # encoding: UTF-8
 
 require 'thread'
-require 'prometheus/client/label_set'
+require 'prometheus/client/label_set_validator'
 
 module Prometheus
   module Client
@@ -10,18 +10,16 @@ class Metric
       attr_reader :name, :docstring, :base_labels
 
       def initialize(name, docstring, base_labels = {})
-        unless name.is_a?(Symbol)
-          fail ArgumentError, 'given name must be a symbol'
-        end
+        @mutex = Mutex.new
+        @validator = LabelSetValidator.new
+        @values = Hash.new { |hash, key| hash[key] = default }
 
-        if !docstring.respond_to?(:empty?) || docstring.empty?
-          fail ArgumentError, 'docstring must be given'
-        end
+        validate_name(name)
+        validate_docstring(docstring)
+        @validator.valid?(base_labels)
 
         @name, @docstring = name, docstring
-        @base_labels = LabelSet.new(base_labels)
-        @mutex = Mutex.new
-        @values = Hash.new { |hash, key| hash[key] = default }
+        @base_labels = base_labels
       end
 
       # Returns the metric type
@@ -34,6 +32,7 @@ def get(labels = {})
         @values[label_set_for(labels)]
       end
 
+      # Returns all label sets with their values
       def values
         synchronize do
           @values.each_with_object({}) do |(labels, value), memo|
@@ -48,8 +47,20 @@ def default
         nil
       end
 
+      def validate_name(name)
+        return true if name.is_a?(Symbol)
+
+        fail ArgumentError, 'given name must be a symbol'
+      end
+
+      def validate_docstring(docstring)
+        return true if docstring.respond_to?(:empty?) && !docstring.empty?
+
+        fail ArgumentError, 'docstring must be given'
+      end
+
       def label_set_for(labels)
-        LabelSet.new(labels)
+        @validator.validate(labels)
       end
 
       def synchronize(&block)

spec/examples/metric_example.rb

@@ -9,9 +9,11 @@
     end
 
     it 'raises an exception if a reserved base label is used' do
+      exception = Prometheus::Client::LabelSetValidator::ReservedLabelError
+
       expect do
         described_class.new(:foo, 'foo docstring',  __name__: 'reserved')
-      end.to raise_exception Prometheus::Client::LabelSet::ReservedLabelError
+      end.to raise_exception exception
     end
 
     it 'raises an exception if the given name is blank' do

spec/prometheus/client/label_set_validator_spec.rb

@@ -0,0 +1,61 @@
+# encoding: UTF-8
+
+require 'prometheus/client/label_set_validator'
+
+describe Prometheus::Client::LabelSetValidator do
+  let(:validator) { Prometheus::Client::LabelSetValidator.new }
+  let(:invalid) { Prometheus::Client::LabelSetValidator::InvalidLabelSetError }
+
+  describe '.new' do
+    it 'returns an instance of a LabelSetValidator' do
+      expect(validator).to be_a(Prometheus::Client::LabelSetValidator)
+    end
+  end
+
+  describe '#valid?' do
+    it 'returns true for a valid label check' do
+      expect(validator.valid?(version: 'alpha')).to eql(true)
+    end
+
+    it 'raises Invaliddescribed_classError if a label set is not a hash' do
+      expect do
+        validator.valid?('invalid')
+      end.to raise_exception invalid
+    end
+
+    it 'raises InvalidLabelError if a label key is not a symbol' do
+      expect do
+        validator.valid?('key' => 'value')
+      end.to raise_exception(described_class::InvalidLabelError)
+    end
+
+    it 'raises InvalidLabelError if a label key starts with __' do
+      expect do
+        validator.valid?(__reserved__: 'key')
+      end.to raise_exception(described_class::ReservedLabelError)
+    end
+
+    it 'raises ReservedLabelError if a label key is reserved' do
+      [:job, :instance].each do |label|
+        expect do
+          validator.valid?(label => 'value')
+        end.to raise_exception(described_class::ReservedLabelError)
+      end
+    end
+  end
+
+  describe '#validate' do
+    it 'returns a given valid label set' do
+      hash = { version: 'alpha' }
+
+      expect(validator.validate(hash)).to eql(hash)
+    end
+
+    it 'raises an exception if a given label set is not valid' do
+      input = 'broken'
+      validator.should_receive(:valid?).with(input).and_raise(invalid)
+
+      expect { validator.validate(input) }.to raise_exception(invalid)
+    end
+  end
+end