From 2d0741e562dfcaca777b2d97f0cf11b8ac973f0a Mon Sep 17 00:00:00 2001 From: Pawel Chojnacki Date: Thu, 13 Jul 2017 00:46:17 +0200 Subject: Rename ConnectionRackMiddleware to RequestsRackMiddleware. + fix tests after metrics rename --- config/initializers/8_metrics.rb | 2 +- lib/gitlab/metrics/connection_rack_middleware.rb | 40 ----------- lib/gitlab/metrics/requests_rack_middleware.rb | 40 +++++++++++ .../metrics/connection_rack_middleware_spec.rb | 77 ---------------------- .../metrics/requests_rack_middleware_spec.rb | 71 ++++++++++++++++++++ 5 files changed, 112 insertions(+), 118 deletions(-) delete mode 100644 lib/gitlab/metrics/connection_rack_middleware.rb create mode 100644 lib/gitlab/metrics/requests_rack_middleware.rb delete mode 100644 spec/lib/gitlab/metrics/connection_rack_middleware_spec.rb create mode 100644 spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb 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/connection_rack_middleware_spec.rb deleted file mode 100644 index 12392b8ddda..00000000000 --- a/spec/lib/gitlab/metrics/connection_rack_middleware_spec.rb +++ /dev/null @@ -1,77 +0,0 @@ -require 'spec_helper' - -describe Gitlab::Metrics::ConnectionRackMiddleware do - let(:app) { double('app') } - subject { described_class.new(app) } - - around do |example| - Timecop.freeze { example.run } - end - - describe '#call' do - let(:status) { 100 } - let(:env) { { 'REQUEST_METHOD' => 'GET' } } - let(:stack_result) { [status, {}, 'body'] } - - before do - allow(app).to receive(:call).and_return(stack_result) - end - - context '@app.call succeeds with 200' do - before do - allow(app).to receive(:call).and_return([200, nil, nil]) - end - - it 'increments requests count' do - expect(described_class).to receive_message_chain(:rack_request_count, :increment).with(method: 'get') - - subject.call(env) - end - - it 'measures execution time' do - execution_time = 10 - allow(app).to receive(:call) do |*args| - Timecop.freeze(execution_time.seconds) - end - - expect(described_class).to receive_message_chain(:rack_execution_time, :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') } - - it 'increments exceptions count' do - expect(described_class).to receive_message_chain(:rack_uncaught_errors_count, :increment) - - expect { subject.call(env) }.to raise_error(StandardError) - 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 { 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) - - expect { subject.call(env) }.to raise_error(StandardError) - end - end - end -end diff --git a/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb b/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb new file mode 100644 index 00000000000..461b1e4182a --- /dev/null +++ b/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb @@ -0,0 +1,71 @@ +require 'spec_helper' + +describe Gitlab::Metrics::RequestsRackMiddleware do + let(:app) { double('app') } + subject { described_class.new(app) } + + around do |example| + Timecop.freeze { example.run } + end + + describe '#call' do + let(:status) { 100 } + let(:env) { { 'REQUEST_METHOD' => 'GET' } } + let(:stack_result) { [status, {}, 'body'] } + + before do + allow(app).to receive(:call).and_return(stack_result) + end + + context '@app.call succeeds with 200' do + before do + allow(app).to receive(:call).and_return([200, nil, nil]) + end + + it 'increments requests count' do + expect(described_class).to receive_message_chain(:http_request_total, :increment).with(method: 'get') + + subject.call(env) + end + + it 'measures execution time' 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(:http_request_duration_seconds, :observe).with({ status: 200, method: 'get' }, execution_time) + + subject.call(env) + end + end + + context '@app.call throws exception' do + 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) + + expect { subject.call(env) }.to raise_error(StandardError) + end + + it 'increments requests count' do + expect(described_class).to receive_message_chain(:http_request_total, :increment).with(method: 'get') + + expect { subject.call(env) }.to raise_error(StandardError) + end + + 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 + end + end +end -- cgit v1.2.1