Merge branch 'rc/filter_health_endpoint_metrics' into 'master'

Filter health endpoint metrics

Closes #199434

See merge request gitlab-org/gitlab!27847

changelogs/unreleased/rc-filter_health_endpoint_metrics.yml

+---
+title: Filter health endpoint metrics
+merge_request: 27847
+author:
+type: added

lib/gitlab/metrics/requests_rack_middleware.rb

@@ -15,6 +15,8 @@ module Gitlab
         "report" =>  %w(404)
       }.freeze
 
+      HEALTH_ENDPOINT = /^\/-\/(liveness|readiness|health|metrics)\/?$/.freeze
+
       def initialize(app)
         @app = app
       end
@@ -32,6 +34,10 @@ module Gitlab
                                                            {}, [0.05, 0.1, 0.25, 0.5, 0.7, 1, 2.5, 5, 10, 25])
       end
 
+      def self.http_health_requests_total
+        @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, statuses|
           statuses.each do |status|
@@ -43,8 +49,13 @@ module Gitlab
       def call(env)
         method = env['REQUEST_METHOD'].downcase
         started = Time.now.to_f
+
         begin
+          if health_endpoint?(env['PATH_INFO'])
+            RequestsRackMiddleware.http_health_requests_total.increment(method: method)
+          else
             RequestsRackMiddleware.http_request_total.increment(method: method)
+          end
 
           status, headers, body = @app.call(env)
 
@@ -57,6 +68,12 @@ module Gitlab
           raise
         end
       end
+
+      def health_endpoint?(path)
+        return false if path.blank?
+
+        HEALTH_ENDPOINT.match?(CGI.unescape(path))
+      end
     end
   end
 end

spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb

@@ -36,6 +36,74 @@ describe Gitlab::Metrics::RequestsRackMiddleware do
 
         Timecop.scale(3600) { subject.call(env) }
       end
+
+      context 'request is a health check endpoint' do
+        it 'increments health endpoint counter' do
+          env['PATH_INFO'] = '/-/liveness'
+
+          expect(described_class).to receive_message_chain(:http_health_requests_total, :increment).with(method: 'get')
+
+          subject.call(env)
+        end
+
+        context 'with trailing slash' do
+          before do
+            env['PATH_INFO'] = '/-/liveness/'
+          end
+
+          it 'increments health endpoint counter' do
+            expect(described_class).to receive_message_chain(:http_health_requests_total, :increment).with(method: 'get')
+
+            subject.call(env)
+          end
+        end
+
+        context 'with percent encoded values' do
+          before do
+            env['PATH_INFO'] = '/-/%6D%65%74%72%69%63%73' # /-/metrics
+          end
+
+          it 'increments health endpoint counter' do
+            expect(described_class).to receive_message_chain(:http_health_requests_total, :increment).with(method: 'get')
+
+            subject.call(env)
+          end
+        end
+      end
+
+      context 'request is not a health check endpoint' do
+        it 'does not increment health endpoint counter' do
+          env['PATH_INFO'] = '/-/ordinary-requests'
+
+          expect(described_class).not_to receive(:http_health_requests_total)
+
+          subject.call(env)
+        end
+
+        context 'path info is a root path' do
+          before do
+            env['PATH_INFO'] = '/-/'
+          end
+
+          it 'does not increment health endpoint counter' do
+            expect(described_class).not_to receive(:http_health_requests_total)
+
+            subject.call(env)
+          end
+        end
+
+        context 'path info is a subpath' do
+          before do
+            env['PATH_INFO'] = '/-/health/subpath'
+          end
+
+          it 'does not increment health endpoint counter' do
+            expect(described_class).not_to receive(:http_health_requests_total)
+
+            subject.call(env)
+          end
+        end
+      end
     end
 
     context '@app.call throws exception' do