FEATURE: Add content_type label to web performance metrics (#65)

For now, `content_type` label can either be `json`, `html` or `other` to limit the cardinality of the metrics.
discourse · Apr 11, 2023 · efcee66 · efcee66
1 parent 6275c8a
commit efcee66
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 3 deletions.
diff --git a/lib/collector.rb b/lib/collector.rb
@@ -259,7 +259,12 @@ def process_web(metric)
       # STDERR.puts metric.to_h.inspect
       # STDERR.puts metric.controller.to_s + " " + metric.action.to_s
 
-      labels = { cache: !!metric.cache, success: (200..299).include?(metric.status_code) }
+      labels = {
+        cache: !!metric.cache,
+        success: (200..299).include?(metric.status_code),
+        content_type: web_metric_content_type(metric),
+        logged_in: metric.logged_in,
+      }
 
       if observe_timings?(metric)
         labels[:controller] = metric.controller
@@ -269,8 +274,6 @@ def process_web(metric)
         labels[:action] = "other"
       end
 
-      labels[:logged_in] = metric.logged_in
-
       @http_duration_seconds.observe(metric.duration, labels)
       @http_sql_duration_seconds.observe(metric.sql_duration, labels)
       @http_redis_duration_seconds.observe(metric.redis_duration, labels)
@@ -399,5 +402,15 @@ def observe_timings?(metric)
         (metric.controller == "users" && metric.action == "show") ||
         (metric.controller == "categories" && metric.action == "categories_and_latest")
     end
+
+    def web_metric_content_type(metric)
+      if metric.json
+        "json"
+      elsif metric.html
+        "html"
+      else
+        "other"
+      end
+    end
   end
 end
diff --git a/lib/internal_metric/web.rb b/lib/internal_metric/web.rb
@@ -14,6 +14,7 @@ class Web < Base
       mobile
       tracked
       json
+      html
       admin_api
       user_api
       forced_anon
@@ -97,6 +98,9 @@ def self.from_env_data(env, data, host)
         env["PATH_INFO"].to_s.ends_with?(".json") ||
           env["HTTP_ACCEPT"].to_s.include?("application/json")
 
+      metric.html =
+        env["PATH_INFO"].to_s.ends_with?(".html") || env["HTTP_ACCEPT"].to_s.include?("text/html")
+
       metric.ajax = env["HTTP_X_REQUESTED_WITH"] == "XMLHttpRequest"
       metric.forced_anon = !!env["DISCOURSE_FORCE_ANON"]
 

diff --git a/spec/lib/collector_spec.rb b/spec/lib/collector_spec.rb
@@ -257,6 +257,7 @@ module DiscoursePrometheus
         sql_duration: 1,
         redis_duration: 3,
         net_duration: 5,
+        json: true,
         controller: "list",
         action: "latest",
         logged_in: true,
@@ -272,6 +273,7 @@ module DiscoursePrometheus
         action: "latest",
         cache: true,
         logged_in: false,
+        html: true,
       )
 
       collector = Collector.new
@@ -289,6 +291,7 @@ module DiscoursePrometheus
             success: true,
             cache: false,
             logged_in: true,
+            content_type: "json",
           } => {
             "count" => 1,
             "sum" => sum,
@@ -299,6 +302,7 @@ module DiscoursePrometheus
             success: false,
             cache: true,
             logged_in: false,
+            content_type: "html",
           } => {
             "count" => 1,
             "sum" => sum,