Merge branch 'add-dimension-etag-caching-metrics' into 'master'

Include endpoint in metrics for ETag caching middleware

See merge request !10495

3 files changed, 40 insertions, 24 deletions

diff --git a/changelogs/unreleased/add-dimension-etag-caching-metrics.yml b/changelogs/unreleased/add-dimension-etag-caching-metrics.yml
new file mode 100644
index 00000000000..f2a13eb7c61
--- /dev/null
+++ b/changelogs/unreleased/add-dimension-etag-caching-metrics.yml
@@ -0,0 +1,4 @@
+---
+title: Include endpoint in metrics for ETag caching middleware
+merge_request: 10495
+author:
diff --git a/lib/gitlab/etag_caching/middleware.rb b/lib/gitlab/etag_caching/middleware.rb
index cd4e318033d..630fe4fa849 100644
--- a/lib/gitlab/etag_caching/middleware.rb
+++ b/lib/gitlab/etag_caching/middleware.rb
@@ -2,26 +2,34 @@ module Gitlab
   module EtagCaching
     class Middleware
       RESERVED_WORDS = NamespaceValidator::WILDCARD_ROUTES.map { |word| "/#{word}/" }.join('|')
-      ROUTE_REGEXP = Regexp.union(
-        %r(^(?!.*(#{RESERVED_WORDS})).*/noteable/issue/\d+/notes\z),
-        %r(^(?!.*(#{RESERVED_WORDS})).*/issues/\d+/rendered_title\z)
-      )
+      ROUTES = [
+        {
+          regexp: %r(^(?!.*(#{RESERVED_WORDS})).*/noteable/issue/\d+/notes\z),
+          name: 'issue_notes'
+        },
+        {
+          regexp: %r(^(?!.*(#{RESERVED_WORDS})).*/issues/\d+/rendered_title\z),
+          name: 'issue_title'
+        }
+      ].freeze
 
       def initialize(app)
         @app = app
       end
 
       def call(env)
-        return @app.call(env) unless enabled_for_current_route?(env)
-        Gitlab::Metrics.add_event(:etag_caching_middleware_used)
+        route = match_current_route(env)
+        return @app.call(env) unless route
+
+        track_event(:etag_caching_middleware_used, route)
 
         etag, cached_value_present = get_etag(env)
         if_none_match = env['HTTP_IF_NONE_MATCH']
 
         if if_none_match == etag
-          handle_cache_hit(etag)
+          handle_cache_hit(etag, route)
         else
-          track_cache_miss(if_none_match, cached_value_present)
+          track_cache_miss(if_none_match, cached_value_present, route)
 
           status, headers, body = @app.call(env)
           headers['ETag'] = etag
@@ -31,8 +39,8 @@ module Gitlab
 
       private
 
-      def enabled_for_current_route?(env)
-        ROUTE_REGEXP.match(env['PATH_INFO'])
+      def match_current_route(env)
+        ROUTES.find { |route| route[:regexp].match(env['PATH_INFO']) }
       end
 
       def get_etag(env)
@@ -52,23 +60,27 @@ module Gitlab
         %Q{W/"#{value}"}
       end
 
-      def handle_cache_hit(etag)
-        Gitlab::Metrics.add_event(:etag_caching_cache_hit)
+      def handle_cache_hit(etag, route)
+        track_event(:etag_caching_cache_hit, route)
 
         status_code = Gitlab::PollingInterval.polling_enabled? ? 304 : 429
 
         [status_code, { 'ETag' => etag }, ['']]
       end
 
-      def track_cache_miss(if_none_match, cached_value_present)
+      def track_cache_miss(if_none_match, cached_value_present, route)
         if if_none_match.blank?
-          Gitlab::Metrics.add_event(:etag_caching_header_missing)
+          track_event(:etag_caching_header_missing, route)
         elsif !cached_value_present
-          Gitlab::Metrics.add_event(:etag_caching_key_not_found)
+          track_event(:etag_caching_key_not_found, route)
         else
-          Gitlab::Metrics.add_event(:etag_caching_resource_changed)
+          track_event(:etag_caching_resource_changed, route)
         end
       end
+
+      def track_event(name, route)
+        Gitlab::Metrics.add_event(name, endpoint: route[:name])
+      end
     end
   end
 end
diff --git a/spec/lib/gitlab/etag_caching/middleware_spec.rb b/spec/lib/gitlab/etag_caching/middleware_spec.rb
index 6ec4360adc2..c872d8232b0 100644
--- a/spec/lib/gitlab/etag_caching/middleware_spec.rb
+++ b/spec/lib/gitlab/etag_caching/middleware_spec.rb
@@ -47,9 +47,9 @@ describe Gitlab::EtagCaching::Middleware do
 
       it 'tracks "etag_caching_key_not_found" event' do
         expect(Gitlab::Metrics).to receive(:add_event)
-          .with(:etag_caching_middleware_used)
+          .with(:etag_caching_middleware_used, endpoint: 'issue_notes')
         expect(Gitlab::Metrics).to receive(:add_event)
-          .with(:etag_caching_key_not_found)
+          .with(:etag_caching_key_not_found, endpoint: 'issue_notes')
 
         middleware.call(build_env(path, if_none_match))
       end
@@ -93,9 +93,9 @@ describe Gitlab::EtagCaching::Middleware do
 
     it 'tracks "etag_caching_cache_hit" event' do
       expect(Gitlab::Metrics).to receive(:add_event)
-        .with(:etag_caching_middleware_used)
+        .with(:etag_caching_middleware_used, endpoint: 'issue_notes')
       expect(Gitlab::Metrics).to receive(:add_event)
-        .with(:etag_caching_cache_hit)
+        .with(:etag_caching_cache_hit, endpoint: 'issue_notes')
 
       middleware.call(build_env(path, if_none_match))
     end
@@ -132,9 +132,9 @@ describe Gitlab::EtagCaching::Middleware do
       mock_app_response
 
       expect(Gitlab::Metrics).to receive(:add_event)
-        .with(:etag_caching_middleware_used)
+        .with(:etag_caching_middleware_used, endpoint: 'issue_notes')
       expect(Gitlab::Metrics).to receive(:add_event)
-        .with(:etag_caching_resource_changed)
+        .with(:etag_caching_resource_changed, endpoint: 'issue_notes')
 
       middleware.call(build_env(path, if_none_match))
     end
@@ -150,9 +150,9 @@ describe Gitlab::EtagCaching::Middleware do
 
     it 'tracks "etag_caching_header_missing" event' do
       expect(Gitlab::Metrics).to receive(:add_event)
-        .with(:etag_caching_middleware_used)
+        .with(:etag_caching_middleware_used, endpoint: 'issue_notes')
       expect(Gitlab::Metrics).to receive(:add_event)
-        .with(:etag_caching_header_missing)
+        .with(:etag_caching_header_missing, endpoint: 'issue_notes')
 
       middleware.call(build_env(path, if_none_match))
     end