diff options
| -rw-r--r-- | config/initializers/8_metrics.rb | 2 | ||||
| -rw-r--r-- | lib/gitlab/metrics/connection_rack_middleware.rb | 40 | ||||
| -rw-r--r-- | lib/gitlab/metrics/requests_rack_middleware.rb | 40 | ||||
| -rw-r--r-- | spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb (renamed from spec/lib/gitlab/metrics/connection_rack_middleware_spec.rb) | 32 |
4 files changed, 54 insertions, 60 deletions
diff --git a/config/initializers/8_metrics.rb b/config/initializers/8_metrics.rb index 5e34aac6527..2a911297376 100644 --- a/config/initializers/8_metrics.rb +++ b/config/initializers/8_metrics.rb @@ -123,7 +123,7 @@ Gitlab::Metrics::UnicornSampler.initialize_instance(Settings.monitoring.unicorn_ Gitlab::Application.configure do |config| # 0 should be Sentry to catch errors in this middleware - config.middleware.insert(1, Gitlab::Metrics::ConnectionRackMiddleware) + config.middleware.insert(1, Gitlab::Metrics::RequestsRackMiddleware) end if Gitlab::Metrics.enabled? diff --git a/lib/gitlab/metrics/connection_rack_middleware.rb b/lib/gitlab/metrics/connection_rack_middleware.rb deleted file mode 100644 index ac87e481dbd..00000000000 --- a/lib/gitlab/metrics/connection_rack_middleware.rb +++ /dev/null @@ -1,40 +0,0 @@ -module Gitlab - module Metrics - class ConnectionRackMiddleware - def initialize(app) - @app = app - end - - def self.rack_request_count - @rack_request_count ||= Gitlab::Metrics.counter(:http_requests_total, 'Rack request count') - end - - def self.rack_uncaught_errors_count - @rack_uncaught_errors_count ||= Gitlab::Metrics.counter(:rack_uncaught_errors_total, 'Rack connections handling uncaught errors count') - end - - def self.rack_execution_time - @rack_execution_time ||= Gitlab::Metrics.histogram(:http_request_duration_seconds, 'Rack connection handling execution time', - {}, [0.05, 0.1, 0.25, 0.5, 0.7, 1, 1.5, 2, 2.5, 3, 5, 7, 10]) - end - - def call(env) - method = env['REQUEST_METHOD'].downcase - started = Time.now.to_f - begin - ConnectionRackMiddleware.rack_request_count.increment(method: method) - - status, headers, body = @app.call(env) - - [status, headers, body] - rescue - ConnectionRackMiddleware.rack_uncaught_errors_count.increment - raise - ensure - elapsed = Time.now.to_f - started - ConnectionRackMiddleware.rack_execution_time.observe({method: method, status: status}, elapsed) - end - end - end - end -end diff --git a/lib/gitlab/metrics/requests_rack_middleware.rb b/lib/gitlab/metrics/requests_rack_middleware.rb new file mode 100644 index 00000000000..0dc19f31d03 --- /dev/null +++ b/lib/gitlab/metrics/requests_rack_middleware.rb @@ -0,0 +1,40 @@ +module Gitlab + module Metrics + class RequestsRackMiddleware + def initialize(app) + @app = app + end + + def self.http_request_total + @http_request_total ||= Gitlab::Metrics.counter(:http_requests_total, 'Request count') + end + + def self.rack_uncaught_errors_count + @rack_uncaught_errors_count ||= Gitlab::Metrics.counter(:rack_uncaught_errors_total, 'Request handling uncaught errors count') + end + + def self.http_request_duration_seconds + @http_request_duration_seconds ||= Gitlab::Metrics.histogram(:http_request_duration_seconds, 'Request handling execution time', + {}, [0.05, 0.1, 0.25, 0.5, 0.7, 1, 2.5, 5, 10, 25]) + end + + def call(env) + method = env['REQUEST_METHOD'].downcase + started = Time.now.to_f + begin + RequestsRackMiddleware.http_request_total.increment(method: method) + + status, headers, body = @app.call(env) + + elapsed = Time.now.to_f - started + RequestsRackMiddleware.http_request_duration_seconds.observe({ method: method, status: status }, elapsed) + + [status, headers, body] + rescue + RequestsRackMiddleware.rack_uncaught_errors_count.increment + raise + end + end + end + end +end diff --git a/spec/lib/gitlab/metrics/connection_rack_middleware_spec.rb b/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb index 12392b8ddda..461b1e4182a 100644 --- a/spec/lib/gitlab/metrics/connection_rack_middleware_spec.rb +++ b/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::Metrics::ConnectionRackMiddleware do +describe Gitlab::Metrics::RequestsRackMiddleware do let(:app) { double('app') } subject { described_class.new(app) } @@ -23,7 +23,7 @@ describe Gitlab::Metrics::ConnectionRackMiddleware do end it 'increments requests count' do - expect(described_class).to receive_message_chain(:rack_request_count, :increment).with(method: 'get') + expect(described_class).to receive_message_chain(:http_request_total, :increment).with(method: 'get') subject.call(env) end @@ -32,16 +32,22 @@ describe Gitlab::Metrics::ConnectionRackMiddleware do execution_time = 10 allow(app).to receive(:call) do |*args| Timecop.freeze(execution_time.seconds) + [200, nil, nil] end - expect(described_class).to receive_message_chain(:rack_execution_time, :observe).with({status: 200, method: 'get'}, execution_time) + expect(described_class).to receive_message_chain(:http_request_duration_seconds, :observe).with({ status: 200, method: 'get' }, execution_time) subject.call(env) end end context '@app.call throws exception' do - let(:rack_response_count) { double('rack_response_count') } + let(:http_request_duration_seconds) { double('http_request_duration_seconds') } + + before do + allow(app).to receive(:call).and_raise(StandardError) + allow(described_class).to receive(:http_request_duration_seconds).and_return(http_request_duration_seconds) + end it 'increments exceptions count' do expect(described_class).to receive_message_chain(:rack_uncaught_errors_count, :increment) @@ -50,25 +56,13 @@ describe Gitlab::Metrics::ConnectionRackMiddleware do end it 'increments requests count' do - expect(described_class).to receive_message_chain(:rack_request_count, :increment).with(method: 'get') - - expect { subject.call(env) }.to raise_error(StandardError) - end - - it "does't increment response count" do - expect(described_class.rack_response_count).not_to receive(:increment) + expect(described_class).to receive_message_chain(:http_request_total, :increment).with(method: 'get') expect { subject.call(env) }.to raise_error(StandardError) end - it 'measures execution time' do - execution_time = 10 - allow(app).to receive(:call) do |*args| - Timecop.freeze(execution_time.seconds) - raise StandardError - end - - expect(described_class).to receive_message_chain(:rack_execution_time, :observe).with({}, execution_time) + it "does't measure request execution time" do + expect(described_class.http_request_duration_seconds).not_to receive(:increment) expect { subject.call(env) }.to raise_error(StandardError) end |