Merge branch '214279-es-instrumentation-web-metrics' into 'master'

Add Elasticsearch request metrics to rack middleware

See merge request gitlab-org/gitlab!33233

config/initializers/zz_metrics.rb

@@ -148,6 +148,7 @@ if Gitlab::Metrics.enabled? && !Rails.env.test? && !(Rails.env.development? && d
     config.middleware.use(Gitlab::Metrics::RackMiddleware)
     config.middleware.use(Gitlab::Middleware::RailsQueueDuration)
     config.middleware.use(Gitlab::Metrics::RedisRackMiddleware)
+    config.middleware.use(Gitlab::Metrics::ElasticsearchRackMiddleware)
   end
 
   Sidekiq.configure_server do |config|

doc/administration/monitoring/prometheus/gitlab_metrics.md

@@ -94,6 +94,8 @@ The following metrics are available:
 | `http_request_duration_seconds`                                | Histogram |                    9.4 | HTTP response time from rack middleware                                                             | `method`, `status`                                  |
 | `http_redis_requests_duration_seconds`                         | Histogram |                   13.1 | Redis requests duration during web transactions                                                     | `controller`, `action`                              |
 | `http_redis_requests_total`                                    | Counter   |                   13.1 | Redis requests count during web transactions                                                        | `controller`, `action`                              |
+| `http_elasticsearch_requests_duration_seconds` **(STARTER)**   | Histogram |                   13.1 | Elasticsearch requests duration during web transactions                                             | `controller`, `action`                              |
+| `http_elasticsearch_requests_total` **(STARTER)**              | Counter   |                   13.1 | Elasticsearch requests count during web transactions                                                | `controller`, `action`                              |
 | `pipelines_created_total`                                      | Counter   |                    9.4 | Counter of pipelines created                                                                        |                                                     |
 | `rack_uncaught_errors_total`                                   | Counter   |                    9.4 | Rack connections handling uncaught errors count                                                     |                                                     |
 | `user_session_logins_total`                                    | Counter   |                    9.4 | Counter of how many users have logged in                                                            |                                                     |

ee/changelogs/unreleased/214279-es-instrumentation-web-metrics.yml

+---
+title: Add Elasticsearch metrics in Rack middleware
+merge_request: 33233
+author:
+type: added

lib/gitlab/metrics/elasticsearch_rack_middleware.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Metrics
+    # Rack middleware for tracking Elasticsearch metrics from Grape and Web requests.
+    class ElasticsearchRackMiddleware
+      HISTOGRAM_BUCKETS = [0.1, 0.25, 0.5, 1, 2.5, 5, 10, 60].freeze
+
+      def initialize(app)
+        @app = app
+
+        @requests_total_counter = Gitlab::Metrics.counter(:http_elasticsearch_requests_total,
+                                                         'Amount of calls to Elasticsearch servers during web requests',
+                                                         Gitlab::Metrics::Transaction::BASE_LABELS)
+        @requests_duration_histogram = Gitlab::Metrics.histogram(:http_elasticsearch_requests_duration_seconds,
+                                                                 'Query time for Elasticsearch servers during web requests',
+                                                                 Gitlab::Metrics::Transaction::BASE_LABELS,
+                                                                 HISTOGRAM_BUCKETS)
+      end
+
+      def call(env)
+        transaction = Gitlab::Metrics.current_transaction
+
+        @app.call(env)
+      ensure
+        record_metrics(transaction)
+      end
+
+      private
+
+      def record_metrics(transaction)
+        labels = transaction.labels
+        query_time = ::Gitlab::Instrumentation::ElasticsearchTransport.query_time
+        request_count = ::Gitlab::Instrumentation::ElasticsearchTransport.get_request_count
+
+        @requests_total_counter.increment(labels, request_count)
+        @requests_duration_histogram.observe(labels, query_time)
+      end
+    end
+  end
+end

lib/gitlab/metrics/redis_rack_middleware.rb

@@ -6,6 +6,14 @@ module Gitlab
     class RedisRackMiddleware
       def initialize(app)
         @app = app
+
+        @requests_total_counter = Gitlab::Metrics.counter(:http_redis_requests_total,
+                                                          'Amount of calls to Redis servers during web requests',
+                                                          Gitlab::Metrics::Transaction::BASE_LABELS)
+        @requests_duration_histogram = Gitlab::Metrics.histogram(:http_redis_requests_duration_seconds,
+                                                                 'Query time for Redis servers during web requests',
+                                                                 Gitlab::Metrics::Transaction::BASE_LABELS,
+                                                                 Gitlab::Instrumentation::Redis::QUERY_TIME_BUCKETS)
       end
 
       def call(env)
@@ -13,7 +21,7 @@ module Gitlab
 
         @app.call(env)
       ensure
-        record_metrics(transaction) if transaction
+        record_metrics(transaction)
       end
 
       private
@@ -23,14 +31,8 @@ module Gitlab
         query_time = Gitlab::Instrumentation::Redis.query_time
         request_count = Gitlab::Instrumentation::Redis.get_request_count
 
-        Gitlab::Metrics.counter(:http_redis_requests_total,
-                                'Amount of calls to Redis servers during web requests',
-                                Gitlab::Metrics::Transaction::BASE_LABELS).increment(labels, request_count)
-
-        Gitlab::Metrics.histogram(:http_redis_requests_duration_seconds,
-                                  'Query time for Redis servers during web requests',
-                                  Gitlab::Metrics::Transaction::BASE_LABELS,
-                                  Gitlab::Instrumentation::Redis::QUERY_TIME_BUCKETS).observe(labels, query_time)
+        @requests_total_counter.increment(labels, request_count)
+        @requests_duration_histogram.observe(labels, query_time)
       end
     end
   end

spec/lib/gitlab/metrics/elasticsearch_rack_middleware_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Metrics::ElasticsearchRackMiddleware do
+  let(:app) { double(:app, call: 'app call result') }
+  let(:middleware) { described_class.new(app) }
+  let(:env) { {} }
+  let(:transaction) { Gitlab::Metrics::WebTransaction.new(env) }
+
+  describe '#call' do
+    let(:counter) { instance_double(Prometheus::Client::Counter, increment: nil) }
+    let(:histogram) { instance_double(Prometheus::Client::Histogram, observe: nil) }
+    let(:elasticsearch_query_time) { 0.1 }
+    let(:elasticsearch_requests_count) { 2 }
+
+    before do
+      allow(Gitlab::Instrumentation::ElasticsearchTransport).to receive(:query_time) { elasticsearch_query_time }
+      allow(Gitlab::Instrumentation::ElasticsearchTransport).to receive(:get_request_count) { elasticsearch_requests_count }
+
+      allow(Gitlab::Metrics).to receive(:counter)
+        .with(:http_elasticsearch_requests_total,
+              an_instance_of(String),
+              Gitlab::Metrics::Transaction::BASE_LABELS)
+        .and_return(counter)
+
+      allow(Gitlab::Metrics).to receive(:histogram)
+        .with(:http_elasticsearch_requests_duration_seconds,
+              an_instance_of(String),
+              Gitlab::Metrics::Transaction::BASE_LABELS,
+              described_class::HISTOGRAM_BUCKETS)
+        .and_return(histogram)
+
+      allow(Gitlab::Metrics).to receive(:current_transaction).and_return(transaction)
+    end
+
+    it 'calls the app' do
+      expect(middleware.call(env)).to eq('app call result')
+    end
+
+    it 'records elasticsearch metrics' do
+      expect(counter).to receive(:increment).with(transaction.labels, elasticsearch_requests_count)
+      expect(histogram).to receive(:observe).with(transaction.labels, elasticsearch_query_time)
+
+      middleware.call(env)
+    end
+
+    it 'records elasticsearch metrics if an error is raised' do
+      expect(counter).to receive(:increment).with(transaction.labels, elasticsearch_requests_count)
+      expect(histogram).to receive(:observe).with(transaction.labels, elasticsearch_query_time)
+
+      allow(app).to receive(:call).with(env).and_raise(StandardError)
+
+      expect { middleware.call(env) }.to raise_error(StandardError)
+    end
+  end
+end

spec/lib/gitlab/metrics/redis_rack_middleware_spec.rb

@@ -13,68 +13,49 @@ describe Gitlab::Metrics::RedisRackMiddleware do
   end
 
   describe '#call' do
-    context 'when metrics are disabled' do
-      before do
-        allow(Gitlab::Metrics).to receive(:current_transaction).and_return(nil)
-      end
-
-      it 'calls the app' do
-        expect(middleware.call(env)).to eq('wub wub')
-      end
-
-      it 'does not record metrics' do
-        expect(Gitlab::Metrics).not_to receive(:counter)
-        expect(Gitlab::Metrics).not_to receive(:histogram)
-
-        middleware.call(env)
-      end
+    let(:counter) { double(Prometheus::Client::Counter, increment: nil) }
+    let(:histogram) { double(Prometheus::Client::Histogram, observe: nil) }
+    let(:redis_query_time) { 0.1 }
+    let(:redis_requests_count) { 2 }
+
+    before do
+      allow(Gitlab::Instrumentation::Redis).to receive(:query_time) { redis_query_time }
+      allow(Gitlab::Instrumentation::Redis).to receive(:get_request_count) { redis_requests_count }
+
+      allow(Gitlab::Metrics).to receive(:counter)
+        .with(:http_redis_requests_total,
+              an_instance_of(String),
+              Gitlab::Metrics::Transaction::BASE_LABELS)
+        .and_return(counter)
+
+      allow(Gitlab::Metrics).to receive(:histogram)
+        .with(:http_redis_requests_duration_seconds,
+              an_instance_of(String),
+              Gitlab::Metrics::Transaction::BASE_LABELS,
+              Gitlab::Instrumentation::Redis::QUERY_TIME_BUCKETS)
+        .and_return(histogram)
+
+      allow(Gitlab::Metrics).to receive(:current_transaction).and_return(transaction)
     end
 
-    context 'when metrics are enabled' do
-      let(:counter) { double(Prometheus::Client::Counter, increment: nil) }
-      let(:histogram) { double(Prometheus::Client::Histogram, observe: nil) }
-      let(:redis_query_time) { 0.1 }
-      let(:redis_requests_count) { 2 }
-
-      before do
-        allow(Gitlab::Instrumentation::Redis).to receive(:query_time) { redis_query_time }
-        allow(Gitlab::Instrumentation::Redis).to receive(:get_request_count) { redis_requests_count }
-
-        allow(Gitlab::Metrics).to receive(:counter)
-          .with(:http_redis_requests_total,
-                an_instance_of(String),
-                Gitlab::Metrics::Transaction::BASE_LABELS)
-          .and_return(counter)
-
-        allow(Gitlab::Metrics).to receive(:histogram)
-          .with(:http_redis_requests_duration_seconds,
-                an_instance_of(String),
-                Gitlab::Metrics::Transaction::BASE_LABELS,
-                Gitlab::Instrumentation::Redis::QUERY_TIME_BUCKETS)
-          .and_return(histogram)
-
-        allow(Gitlab::Metrics).to receive(:current_transaction).and_return(transaction)
-      end
-
-      it 'calls the app' do
-        expect(middleware.call(env)).to eq('wub wub')
-      end
+    it 'calls the app' do
+      expect(middleware.call(env)).to eq('wub wub')
+    end
 
-      it 'records redis metrics' do
-        expect(counter).to receive(:increment).with(transaction.labels, redis_requests_count)
-        expect(histogram).to receive(:observe).with(transaction.labels, redis_query_time)
+    it 'records redis metrics' do
+      expect(counter).to receive(:increment).with(transaction.labels, redis_requests_count)
+      expect(histogram).to receive(:observe).with(transaction.labels, redis_query_time)
 
-        middleware.call(env)
-      end
+      middleware.call(env)
+    end
 
-      it 'records redis metrics if an error is raised' do
-        expect(counter).to receive(:increment).with(transaction.labels, redis_requests_count)
-        expect(histogram).to receive(:observe).with(transaction.labels, redis_query_time)
+    it 'records redis metrics if an error is raised' do
+      expect(counter).to receive(:increment).with(transaction.labels, redis_requests_count)
+      expect(histogram).to receive(:observe).with(transaction.labels, redis_query_time)
 
-        allow(app).to receive(:call).with(env).and_raise(StandardError)
+      allow(app).to receive(:call).with(env).and_raise(StandardError)
 
-        expect { middleware.call(env) }.to raise_error(StandardError)
-      end
+      expect { middleware.call(env) }.to raise_error(StandardError)
     end
   end
 end