Commit 6aa1f23c authored by Ben Kochie's avatar Ben Kochie Committed by Mark Chao

Initialize http_requests_total label values

Add http method/status initilaization to the `http_requests_total`
counter. This is required to track error rates and error ratios.

For example, the ratio of 5xx requests to all requests.

    sum(rate(http_requests_total{status=~"5.."}[5m])
    /
    sum(rate(http_requests_total[5m])

Without the inialization, there would be no values for 5xx on small or
low traffic servers. This would lead to blank graphs.

https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/585Signed-off-by: default avatarBen Kochie <bjk@gitlab.com>
parent 66bb7420
---
title: Make sure the http_requests_total and http_request_duration_seconds metrics are not empty on application start
merge_request: 45755
author:
type: fixed
......@@ -70,7 +70,7 @@ if !Rails.env.test? && Gitlab::Metrics.prometheus_metrics_enabled?
Gitlab::Metrics.gauge(:deployments, 'GitLab Version', {}, :max).set({ version: Gitlab::VERSION }, 1)
unless Gitlab::Runtime.sidekiq?
Gitlab::Metrics::RequestsRackMiddleware.initialize_http_request_duration_seconds
Gitlab::Metrics::RequestsRackMiddleware.initialize_metrics
end
rescue IOError => e
Gitlab::ErrorTracking.track_exception(e)
......
......@@ -3,7 +3,15 @@
module Gitlab
module Metrics
class RequestsRackMiddleware
HTTP_METHODS = %w(delete get head options patch post put).to_set.freeze
HTTP_METHODS = {
"delete" => %w(200 202 204 303 400 401 403 404 500 503),
"get" => %w(200 204 301 302 303 304 307 400 401 403 404 410 422 429 500 503),
"head" => %w(200 204 301 302 303 401 403 404 410 500),
"options" => %w(200 404),
"patch" => %w(200 202 204 400 403 404 409 416 500),
"post" => %w(200 201 202 204 301 302 303 304 400 401 403 404 406 409 410 412 422 429 500 503),
"put" => %w(200 202 204 400 401 403 404 405 406 409 410 422 500)
}.freeze
HEALTH_ENDPOINT = /^\/-\/(liveness|readiness|health|metrics)\/?$/.freeze
......@@ -14,8 +22,8 @@ module Gitlab
@app = app
end
def self.http_request_total
@http_request_total ||= ::Gitlab::Metrics.counter(:http_requests_total, 'Request count')
def self.http_requests_total
@http_requests_total ||= ::Gitlab::Metrics.counter(:http_requests_total, 'Request count')
end
def self.rack_uncaught_errors_count
......@@ -31,15 +39,31 @@ module Gitlab
@http_health_requests_total ||= ::Gitlab::Metrics.counter(:http_health_requests_total, 'Health endpoint request count')
end
def self.initialize_http_request_duration_seconds
HTTP_METHODS.each do |method|
def self.initialize_metrics
# This initialization is done to avoid gaps in scraped metrics after
# restarts. It makes sure all counters/histograms are available at
# process start.
#
# For example `rate(http_requests_total{status="500"}[1m])` would return
# no data until the first 500 error would occur.
# The list of feature categories is currently not needed by the application
# anywhere else. So no need to keep these in memory forever.
# Doing it here, means we're only reading the file on boot.
feature_categories = YAML.load_file(Rails.root.join('config', 'feature_categories.yml')).map(&:strip).uniq << FEATURE_CATEGORY_DEFAULT
HTTP_METHODS.each do |method, statuses|
http_request_duration_seconds.get({ method: method })
statuses.product(feature_categories) do |status, feature_category|
http_requests_total.get({ method: method, status: status, feature_category: feature_category })
end
end
end
def call(env)
method = env['REQUEST_METHOD'].downcase
method = 'INVALID' unless HTTP_METHODS.include?(method)
method = 'INVALID' unless HTTP_METHODS.key?(method)
started = Time.now.to_f
health_endpoint = health_endpoint?(env['PATH_INFO'])
status = 'undefined'
......@@ -61,9 +85,13 @@ module Gitlab
raise
ensure
if health_endpoint
RequestsRackMiddleware.http_health_requests_total.increment(status: status, method: method)
RequestsRackMiddleware.http_health_requests_total.increment(status: status.to_s, method: method)
else
RequestsRackMiddleware.http_request_total.increment(status: status, method: method, feature_category: feature_category.presence || FEATURE_CATEGORY_DEFAULT)
RequestsRackMiddleware.http_requests_total.increment(
status: status.to_s,
method: method,
feature_category: feature_category.presence || FEATURE_CATEGORY_DEFAULT
)
end
end
end
......
......@@ -2,7 +2,7 @@
require 'spec_helper'
RSpec.describe Gitlab::Metrics::RequestsRackMiddleware do
RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do
let(:app) { double('app') }
subject { described_class.new(app) }
......@@ -21,20 +21,15 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware 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', status: 200, feature_category: 'unknown')
subject.call(env)
end
RSpec::Matchers.define :a_positive_execution_time do
match { |actual| actual > 0 }
end
it 'measures execution time' do
it 'tracks request count and duration' do
expect(described_class).to receive_message_chain(:http_requests_total, :increment).with(method: 'get', status: '200', feature_category: 'unknown')
expect(described_class).to receive_message_chain(:http_request_duration_seconds, :observe).with({ method: 'get' }, a_positive_execution_time)
Timecop.scale(3600) { subject.call(env) }
subject.call(env)
end
context 'request is a health check endpoint' do
......@@ -44,15 +39,10 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware do
env['PATH_INFO'] = path
end
it 'increments health endpoint counter rather than overall counter' do
expect(described_class).to receive_message_chain(:http_health_requests_total, :increment).with(method: 'get', status: 200)
expect(described_class).not_to receive(:http_request_total)
subject.call(env)
end
it 'does not record the request duration' do
it 'increments health endpoint counter rather than overall counter and does not record duration' do
expect(described_class).not_to receive(:http_request_duration_seconds)
expect(described_class).not_to receive(:http_requests_total)
expect(described_class).to receive_message_chain(:http_health_requests_total, :increment).with(method: 'get', status: '200')
subject.call(env)
end
......@@ -67,14 +57,9 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware do
env['PATH_INFO'] = path
end
it 'increments overall counter rather than health endpoint counter' do
expect(described_class).to receive_message_chain(:http_request_total, :increment).with(method: 'get', status: 200, feature_category: 'unknown')
it 'increments regular counters and tracks duration' do
expect(described_class).to receive_message_chain(:http_requests_total, :increment).with(method: 'get', status: '200', feature_category: 'unknown')
expect(described_class).not_to receive(:http_health_requests_total)
subject.call(env)
end
it 'records the request duration' do
expect(described_class)
.to receive_message_chain(:http_request_duration_seconds, :observe)
.with({ method: 'get' }, a_positive_execution_time)
......@@ -88,26 +73,18 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware do
context '@app.call throws exception' do
let(:http_request_duration_seconds) { double('http_request_duration_seconds') }
let(:http_requests_total) { double('http_requests_total') }
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)
allow(described_class).to receive(:http_requests_total).and_return(http_requests_total)
end
it 'increments exceptions count' do
it 'tracks the correct metrics' 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', status: 'undefined', feature_category: 'unknown')
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(described_class).to receive_message_chain(:http_requests_total, :increment).with(method: 'get', status: 'undefined', feature_category: 'unknown')
expect(described_class.http_request_duration_seconds).not_to receive(:observe)
expect { subject.call(env) }.to raise_error(StandardError)
end
......@@ -119,8 +96,9 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware do
allow(app).to receive(:call).and_return([200, { described_class::FEATURE_CATEGORY_HEADER => 'issue_tracking' }, nil])
end
it 'adds the feature category to the labels for http_request_total' do
expect(described_class).to receive_message_chain(:http_request_total, :increment).with(method: 'get', status: 200, feature_category: 'issue_tracking')
it 'adds the feature category to the labels for http_requests_total' do
expect(described_class).to receive_message_chain(:http_requests_total, :increment).with(method: 'get', status: '200', feature_category: 'issue_tracking')
expect(described_class).not_to receive(:http_health_requests_total)
subject.call(env)
end
......@@ -128,8 +106,8 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware do
it 'does not record a feature category for health check endpoints' do
env['PATH_INFO'] = '/-/liveness'
expect(described_class).to receive_message_chain(:http_health_requests_total, :increment).with(method: 'get', status: 200)
expect(described_class).not_to receive(:http_request_total)
expect(described_class).to receive_message_chain(:http_health_requests_total, :increment).with(method: 'get', status: '200')
expect(described_class).not_to receive(:http_requests_total)
subject.call(env)
end
......@@ -141,21 +119,37 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware do
end
it 'sets the feature category to unknown' do
expect(described_class).to receive_message_chain(:http_request_total, :increment).with(method: 'get', status: 200, feature_category: 'unknown')
expect(described_class).to receive_message_chain(:http_requests_total, :increment).with(method: 'get', status: '200', feature_category: 'unknown')
expect(described_class).not_to receive(:http_health_requests_total)
subject.call(env)
end
end
end
describe '.initialize_http_request_duration_seconds' do
it "sets labels" do
describe '.initialize_metrics', :prometheus do
it "sets labels for http_requests_total" do
feature_categories = YAML.load_file(Rails.root.join('config', 'feature_categories.yml')).map(&:strip) << described_class::FEATURE_CATEGORY_DEFAULT
expected_labels = []
described_class::HTTP_METHODS.each do |method|
expected_labels << { method: method }
described_class::HTTP_METHODS.each do |method, statuses|
statuses.each do |status|
feature_categories.each do |feature_category|
expected_labels << { method: method.to_s, status: status.to_s, feature_category: feature_category.to_s }
end
end
end
described_class.initialize_metrics
expect(described_class.http_requests_total.values.keys).to contain_exactly(*expected_labels)
end
it 'sets labels for http_request_duration_seconds' do
expected_labels = described_class::HTTP_METHODS.keys.map { |method| { method: method } }
described_class.initialize_metrics
described_class.initialize_http_request_duration_seconds
expect(described_class.http_request_duration_seconds.values.keys).to include(*expected_labels)
end
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment