diff --git a/lib/gitlab/metrics/exporter/base_exporter.rb b/lib/gitlab/metrics/exporter/base_exporter.rb index 47c862c0232e3a03d220521297b4bf3957c0e3e2..f26b1ab7b4551d4f007da8a07d1c2c975c071546 100644 --- a/lib/gitlab/metrics/exporter/base_exporter.rb +++ b/lib/gitlab/metrics/exporter/base_exporter.rb @@ -39,13 +39,8 @@ def start_working @server = ::WEBrick::HTTPServer.new( Port: settings.port, BindAddress: settings.address, - Logger: logger, AccessLog: access_log) - server.mount_proc '/readiness' do |req, res| - render_probe(readiness_probe, req, res) - end - server.mount_proc '/liveness' do |req, res| - render_probe(liveness_probe, req, res) - end + Logger: logger, AccessLog: access_log + ) server.mount '/', Rack::Handler::WEBrick, rack_app true @@ -72,8 +67,14 @@ def stop_working end def rack_app + readiness = readiness_probe + liveness = liveness_probe + pid = thread_name + Rack::Builder.app do use Rack::Deflater + use Gitlab::Metrics::Exporter::MetricsMiddleware, pid + use Gitlab::Metrics::Exporter::HealthChecksMiddleware, readiness, liveness use ::Prometheus::Client::Rack::Exporter if ::Gitlab::Metrics.metrics_folder_present? run -> (env) { [404, {}, ['']] } end @@ -86,14 +87,6 @@ def readiness_probe def liveness_probe ::Gitlab::HealthChecks::Probes::Collection.new end - - def render_probe(probe, req, res) - result = probe.execute - - res.status = result.http_status - res.content_type = 'application/json; charset=utf-8' - res.body = result.json.to_json - end end end end diff --git a/lib/gitlab/metrics/exporter/health_checks_middleware.rb b/lib/gitlab/metrics/exporter/health_checks_middleware.rb new file mode 100644 index 0000000000000000000000000000000000000000..c43b8004b72a911bcd1460598d85742556b4394f --- /dev/null +++ b/lib/gitlab/metrics/exporter/health_checks_middleware.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +module Gitlab + module Metrics + module Exporter + class HealthChecksMiddleware + def initialize(app, readiness_probe, liveness_probe) + @app = app + @readiness_probe = readiness_probe + @liveness_probe = liveness_probe + end + + def call(env) + case env['PATH_INFO'] + when '/readiness' then render_probe(@readiness_probe) + when '/liveness' then render_probe(@liveness_probe) + else @app.call(env) + end + end + + private + + def render_probe(probe) + result = probe.execute + + [ + result.http_status, + { 'Content-Type' => 'application/json; charset=utf-8' }, + [result.json.to_json] + ] + end + end + end + end +end diff --git a/lib/gitlab/metrics/exporter/metrics_middleware.rb b/lib/gitlab/metrics/exporter/metrics_middleware.rb new file mode 100644 index 0000000000000000000000000000000000000000..e17f1c13cf0e772506f3a987634034092f7e4771 --- /dev/null +++ b/lib/gitlab/metrics/exporter/metrics_middleware.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +module Gitlab + module Metrics + module Exporter + class MetricsMiddleware + def initialize(app, pid) + @app = app + default_labels = { + pid: pid + } + @requests_total = Gitlab::Metrics.counter( + :exporter_http_requests_total, 'Total number of HTTP requests', default_labels + ) + @request_durations = Gitlab::Metrics.histogram( + :exporter_http_request_duration_seconds, + 'HTTP request duration histogram (seconds)', + default_labels, + [0.025, 0.05, 0.1, 0.25, 0.5, 1, 2.5, 5, 10] + ) + end + + def call(env) + start = Gitlab::Metrics::System.monotonic_time + @app.call(env).tap do |response| + duration = Gitlab::Metrics::System.monotonic_time - start + + labels = { + method: env['REQUEST_METHOD'].downcase, + path: env['PATH_INFO'].to_s, + code: response.first.to_s + } + + @requests_total.increment(labels) + @request_durations.observe(labels, duration) + end + end + end + end + end +end diff --git a/metrics_server/dependencies.rb b/metrics_server/dependencies.rb index 494b795b2923613f9a418cbe7b74c48dbe0f8418..36d61a633598198d88c061238ab573041ea9b870 100644 --- a/metrics_server/dependencies.rb +++ b/metrics_server/dependencies.rb @@ -24,6 +24,8 @@ require_relative '../lib/gitlab/metrics/samplers/ruby_sampler' require_relative '../lib/gitlab/metrics/exporter/base_exporter' require_relative '../lib/gitlab/metrics/exporter/sidekiq_exporter' +require_relative '../lib/gitlab/metrics/exporter/metrics_middleware' +require_relative '../lib/gitlab/metrics/exporter/health_checks_middleware' require_relative '../lib/gitlab/health_checks/probes/collection' require_relative '../lib/gitlab/health_checks/probes/status' require_relative '../lib/gitlab/process_management' diff --git a/spec/commands/metrics_server/metrics_server_spec.rb b/spec/commands/metrics_server/metrics_server_spec.rb index f3936e6b3468c4c003051a58316768f66f74de90..b755801bb6511879bc7f2161a5269844bdbe4709 100644 --- a/spec/commands/metrics_server/metrics_server_spec.rb +++ b/spec/commands/metrics_server/metrics_server_spec.rb @@ -23,6 +23,8 @@ end context 'with a running server' do + let(:metrics_dir) { Dir.mktmpdir } + before do # We need to send a request to localhost WebMock.allow_net_connect! @@ -33,7 +35,8 @@ env = { 'GITLAB_CONFIG' => config_file.path, 'METRICS_SERVER_TARGET' => 'sidekiq', - 'WIPE_METRICS_DIR' => '1' + 'WIPE_METRICS_DIR' => '1', + 'prometheus_multiproc_dir' => metrics_dir } @pid = Process.spawn(env, 'bin/metrics-server', pgroup: true) end @@ -55,6 +58,7 @@ # 'No such process' means the process died before ensure config_file.unlink + FileUtils.rm_rf(metrics_dir, secure: true) end it 'serves /metrics endpoint' do diff --git a/spec/lib/gitlab/metrics/exporter/base_exporter_spec.rb b/spec/lib/gitlab/metrics/exporter/base_exporter_spec.rb index 9cd1ef4094e21a24c01716e7d11614dd50e6cd35..204a5dab9c098165df7b479d35a908ece11f677b 100644 --- a/spec/lib/gitlab/metrics/exporter/base_exporter_spec.rb +++ b/spec/lib/gitlab/metrics/exporter/base_exporter_spec.rb @@ -111,6 +111,18 @@ describe 'request handling' do using RSpec::Parameterized::TableSyntax + let(:fake_collector) do + Class.new do + def initialize(app, ...) + @app = app + end + + def call(env) + @app.call(env) + end + end + end + where(:method_class, :path, :http_status) do Net::HTTP::Get | '/metrics' | 200 Net::HTTP::Get | '/liveness' | 200 @@ -123,6 +135,8 @@ allow(settings).to receive(:port).and_return(0) allow(settings).to receive(:address).and_return('127.0.0.1') + stub_const('Gitlab::Metrics::Exporter::MetricsMiddleware', fake_collector) + # We want to wrap original method # and run handling of requests # in separate thread @@ -134,8 +148,6 @@ # is raised as we close listeners end end - - exporter.start.join end after do @@ -146,12 +158,25 @@ let(:config) { exporter.server.config } let(:request) { method_class.new(path) } - it 'responds with proper http_status' do + subject(:response) do http = Net::HTTP.new(config[:BindAddress], config[:Port]) - response = http.request(request) + http.request(request) + end + + it 'responds with proper http_status' do + exporter.start.join expect(response.code).to eq(http_status.to_s) end + + it 'collects request metrics' do + expect_next_instance_of(fake_collector) do |instance| + expect(instance).to receive(:call).and_call_original + end + + exporter.start.join + response + end end end diff --git a/spec/lib/gitlab/metrics/exporter/health_checks_middleware_spec.rb b/spec/lib/gitlab/metrics/exporter/health_checks_middleware_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..9ee46a45e7ab3aad2dd6fdbccac55d0ae7dc5fca --- /dev/null +++ b/spec/lib/gitlab/metrics/exporter/health_checks_middleware_spec.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' + +RSpec.describe Gitlab::Metrics::Exporter::HealthChecksMiddleware do + let(:app) { double(:app) } + let(:env) { { 'PATH_INFO' => path } } + + let(:readiness_probe) { double(:readiness_probe) } + let(:liveness_probe) { double(:liveness_probe) } + let(:probe_result) { Gitlab::HealthChecks::Probes::Status.new(200, { status: 'ok' }) } + + subject(:middleware) { described_class.new(app, readiness_probe, liveness_probe) } + + describe '#call' do + context 'handling /readiness requests' do + let(:path) { '/readiness' } + + it 'handles the request' do + expect(readiness_probe).to receive(:execute).and_return(probe_result) + + response = middleware.call(env) + + expect(response).to eq([200, { 'Content-Type' => 'application/json; charset=utf-8' }, ['{"status":"ok"}']]) + end + end + + context 'handling /liveness requests' do + let(:path) { '/liveness' } + + it 'handles the request' do + expect(liveness_probe).to receive(:execute).and_return(probe_result) + + response = middleware.call(env) + + expect(response).to eq([200, { 'Content-Type' => 'application/json; charset=utf-8' }, ['{"status":"ok"}']]) + end + end + + context 'handling other requests' do + let(:path) { '/other_path' } + + it 'forwards them to the next middleware' do + expect(app).to receive(:call).with(env).and_return([201, {}, []]) + + response = middleware.call(env) + + expect(response).to eq([201, {}, []]) + end + end + end +end diff --git a/spec/lib/gitlab/metrics/exporter/metrics_middleware_spec.rb b/spec/lib/gitlab/metrics/exporter/metrics_middleware_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..ac5721f5974599d28f7767f0685e68c149ba6479 --- /dev/null +++ b/spec/lib/gitlab/metrics/exporter/metrics_middleware_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Metrics::Exporter::MetricsMiddleware do + let(:app) { double(:app) } + let(:pid) { 'fake_exporter' } + let(:env) { { 'PATH_INFO' => '/path', 'REQUEST_METHOD' => 'GET' } } + + subject(:middleware) { described_class.new(app, pid) } + + def metric(name, method, path, status) + metric = ::Prometheus::Client.registry.get(name) + return unless metric + + values = metric.values.transform_keys { |k| k.slice(:method, :path, :pid, :code) } + values[{ method: method, path: path, pid: pid, code: status.to_s }]&.get + end + + before do + expect(app).to receive(:call).with(env).and_return([200, {}, []]) + end + + describe '#call', :prometheus do + it 'records a total requests metric' do + response = middleware.call(env) + + expect(response).to eq([200, {}, []]) + expect(metric(:exporter_http_requests_total, 'get', '/path', 200)).to eq(1.0) + end + + it 'records a request duration histogram' do + response = middleware.call(env) + + expect(response).to eq([200, {}, []]) + expect(metric(:exporter_http_request_duration_seconds, 'get', '/path', 200)).to be_a(Hash) + end + end +end diff --git a/spec/lib/gitlab/metrics/exporter/web_exporter_spec.rb b/spec/lib/gitlab/metrics/exporter/web_exporter_spec.rb index 9deaecbf41b7853c24ea494fc735790135054b88..0531bccf4b44aef1b89d5a1369462893700d6ccc 100644 --- a/spec/lib/gitlab/metrics/exporter/web_exporter_spec.rb +++ b/spec/lib/gitlab/metrics/exporter/web_exporter_spec.rb @@ -24,14 +24,14 @@ exporter.stop end - context 'when running server' do + context 'when running server', :prometheus do it 'readiness probe returns succesful status' do expect(readiness_probe.http_status).to eq(200) expect(readiness_probe.json).to include(status: 'ok') expect(readiness_probe.json).to include('web_exporter' => [{ 'status': 'ok' }]) end - it 'initializes request metrics', :prometheus do + it 'initializes request metrics' do expect(Gitlab::Metrics::RailsSlis).to receive(:initialize_request_slis_if_needed!).and_call_original http = Net::HTTP.new(exporter.server.config[:BindAddress], exporter.server.config[:Port]) @@ -42,7 +42,7 @@ end describe '#mark_as_not_running!' do - it 'readiness probe returns a failure status' do + it 'readiness probe returns a failure status', :prometheus do exporter.mark_as_not_running! expect(readiness_probe.http_status).to eq(503)