diff --git a/lib/prometheus/client/histogram.rb b/lib/prometheus/client/histogram.rb index 12d1ed78..97504543 100644 --- a/lib/prometheus/client/histogram.rb +++ b/lib/prometheus/client/histogram.rb @@ -42,12 +42,18 @@ def self.exponential_buckets(start:, factor: 2, count:) end def with_labels(labels) - self.class.new(name, - docstring: docstring, - labels: @labels, - preset_labels: preset_labels.merge(labels), - buckets: @buckets, - store_settings: @store_settings) + new_metric = self.class.new(name, + docstring: docstring, + labels: @labels, + preset_labels: preset_labels.merge(labels), + buckets: @buckets, + store_settings: @store_settings) + + # The new metric needs to use the same store as the "main" declared one, otherwise + # any observations on that copy with the pre-set labels won't actually be exported. + new_metric.replace_internal_store(@store) + + new_metric end def type diff --git a/lib/prometheus/client/metric.rb b/lib/prometheus/client/metric.rb index 100e455b..899926e6 100644 --- a/lib/prometheus/client/metric.rb +++ b/lib/prometheus/client/metric.rb @@ -40,8 +40,16 @@ def initialize(name, metric_type: type, metric_settings: store_settings ) + + # 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 + end + + protected def replace_internal_store(new_store) + @store = new_store end + # Returns the value for the given label set def get(labels: {}) label_set = label_set_for(labels) @@ -49,11 +57,17 @@ def get(labels: {}) end def with_labels(labels) - self.class.new(name, - docstring: docstring, - labels: @labels, - preset_labels: preset_labels.merge(labels), - store_settings: @store_settings) + new_metric = self.class.new(name, + docstring: docstring, + labels: @labels, + preset_labels: preset_labels.merge(labels), + store_settings: @store_settings) + + # The new metric needs to use the same store as the "main" declared one, otherwise + # any observations on that copy with the pre-set labels won't actually be exported. + new_metric.replace_internal_store(@store) + + new_metric end def init_label_set(labels) diff --git a/lib/prometheus/client/push.rb b/lib/prometheus/client/push.rb index 03efb9f7..3078811a 100644 --- a/lib/prometheus/client/push.rb +++ b/lib/prometheus/client/push.rb @@ -3,6 +3,7 @@ require 'thread' require 'net/http' require 'uri' +require 'cgi' require 'prometheus/client' require 'prometheus/client/formats/text' @@ -20,7 +21,7 @@ class Push attr_reader :job, :instance, :gateway, :path - def initialize(job, instance = nil, gateway = nil) + def initialize(job, instance = nil, gateway = nil, **kwargs) @mutex = Mutex.new @job = job @instance = instance @@ -30,6 +31,8 @@ def initialize(job, instance = nil, gateway = nil) @http = Net::HTTP.new(@uri.host, @uri.port) @http.use_ssl = (@uri.scheme == 'https') + @http.open_timeout = kwargs[:open_timeout] if kwargs[:open_timeout] + @http.read_timeout = kwargs[:read_timeout] if kwargs[:read_timeout] end def add(registry) diff --git a/lib/prometheus/client/registry.rb b/lib/prometheus/client/registry.rb index 4bf63aa4..0b2f6e9a 100644 --- a/lib/prometheus/client/registry.rb +++ b/lib/prometheus/client/registry.rb @@ -22,7 +22,7 @@ def register(metric) name = metric.name @mutex.synchronize do - if exist?(name.to_sym) + if @metrics.key?(name.to_sym) raise AlreadyRegisteredError, "#{name} has already been registered" end @metrics[name.to_sym] = metric @@ -73,15 +73,15 @@ def histogram(name, docstring:, labels: [], preset_labels: {}, end def exist?(name) - @metrics.key?(name) + @mutex.synchronize { @metrics.key?(name) } end def get(name) - @metrics[name.to_sym] + @mutex.synchronize { @metrics[name.to_sym] } end def metrics - @metrics.values + @mutex.synchronize { @metrics.values } end end end diff --git a/lib/prometheus/client/version.rb b/lib/prometheus/client/version.rb index ec33bff5..0e0f5c54 100644 --- a/lib/prometheus/client/version.rb +++ b/lib/prometheus/client/version.rb @@ -2,6 +2,6 @@ module Prometheus module Client - VERSION = '2.1.0' + VERSION = '2.2.0' end end diff --git a/lib/prometheus/middleware/exporter.rb b/lib/prometheus/middleware/exporter.rb index 5a74d8e9..640a3985 100644 --- a/lib/prometheus/middleware/exporter.rb +++ b/lib/prometheus/middleware/exporter.rb @@ -21,11 +21,12 @@ def initialize(app, options = {}) @app = app @registry = options[:registry] || Client.registry @path = options[:path] || '/metrics' + @port = options[:port] @acceptable = build_dictionary(FORMATS, FALLBACK) end def call(env) - if env['PATH_INFO'] == @path + if metrics_port?(env['SERVER_PORT']) && env['PATH_INFO'] == @path format = negotiate(env, @acceptable) format ? respond_with(format) : not_acceptable(FORMATS) else @@ -86,6 +87,10 @@ def build_dictionary(formats, fallback) memo[format::MEDIA_TYPE] = format end end + + def metrics_port?(request_port) + @port.nil? || @port.to_s == request_port + end end end end diff --git a/prometheus-client.gemspec b/prometheus-client.gemspec index 20ad18bf..21690fab 100644 --- a/prometheus-client.gemspec +++ b/prometheus-client.gemspec @@ -8,7 +8,7 @@ Gem::Specification.new do |s| s.summary = 'A suite of instrumentation metric primitives' \ 'that can be exposed through a web services interface.' s.authors = ['Ben Kochie', 'Chris Sinjakli', 'Daniel Magliola'] - s.email = ['superq@gmail.com', 'chris@gocardless.com', 'dmagliola@crystalgears.com'] + s.email = ['superq@gmail.com', 'chris@sinjakli.co.uk', 'dmagliola@crystalgears.com'] s.homepage = 'https://github.com/prometheus/client_ruby' s.license = 'Apache 2.0' diff --git a/spec/prometheus/client/registry_spec.rb b/spec/prometheus/client/registry_spec.rb index 32727d00..3c4da190 100644 --- a/spec/prometheus/client/registry_spec.rb +++ b/spec/prometheus/client/registry_spec.rb @@ -33,10 +33,6 @@ mutex = Mutex.new containers = [] - def registry.exist?(*args) - super.tap { sleep(0.01) } - end - Array.new(5) do Thread.new do result = begin diff --git a/spec/prometheus/middleware/exporter_spec.rb b/spec/prometheus/middleware/exporter_spec.rb index 8916513b..5299fc1e 100644 --- a/spec/prometheus/middleware/exporter_spec.rb +++ b/spec/prometheus/middleware/exporter_spec.rb @@ -6,13 +6,14 @@ describe Prometheus::Middleware::Exporter do include Rack::Test::Methods + let(:options) { { registry: registry } } let(:registry) do Prometheus::Client::Registry.new end let(:app) do app = ->(_) { [200, { 'Content-Type' => 'text/html' }, ['OK']] } - described_class.new(app, registry: registry) + described_class.new(app, **options) end context 'when requesting app endpoints' do @@ -96,5 +97,30 @@ include_examples 'ok', { 'HTTP_ACCEPT' => accept }, text end + + context 'when a port is specified' do + let(:options) { { registry: registry, port: 9999 } } + + context 'when a request is on the specified port' do + it 'responds with 200 OK' do + registry.counter(:foo, docstring: 'foo counter').increment(by: 9) + + get 'http://example.org:9999/metrics', nil, {} + + expect(last_response.status).to eql(200) + expect(last_response.header['Content-Type']).to eql(text::CONTENT_TYPE) + expect(last_response.body).to eql(text.marshal(registry)) + end + end + + context 'when a request is not on the specified port' do + it 'returns the app response' do + get 'http://example.org:8888/metrics', nil, {} + + expect(last_response).to be_ok + expect(last_response.body).to eql('OK') + end + end + end end end