bug-1906108: rework pageview counters to timers

This reworks all the pageview counting metrics to a timing metric single key `socorro.collector.pageview_duration_ms` with the method and path as tags. This makes it easier to do a dashboard for.
mozilla-services · Aug 12, 2024 · 5e99958 · 5e99958
1 parent 033e154
commit 5e99958
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 22 deletions.
diff --git a/antenna/breakpad_resource.py b/antenna/breakpad_resource.py
@@ -302,7 +302,9 @@ def cleanup_crash_report(self, raw_crash):
                 del raw_crash[bad_field]
                 notes.append("Removed %s from raw crash." % bad_field)
 
-    @METRICS.timer_decorator("collector.breakpad_resource.on_post.time")
+    @METRICS.timer_decorator(
+        "collector.pageview_duration_ms", tags=["method:post", "path:/submit"]
+    )
     def on_post(self, req, resp):
         """Handle incoming HTTP POSTs.
 

diff --git a/antenna/health_resource.py b/antenna/health_resource.py
@@ -14,9 +14,11 @@
 class BrokenResource:
     """Handle ``/__broken__`` endpoint."""
 
+    @METRICS.timer_decorator(
+        "collector.pageview_duration_ms", tags=["method:get", "path:/__broken__"]
+    )
     def on_get(self, req, resp):
         """Implement GET HTTP request."""
-        METRICS.incr("collector.health.broken.count")
         # This is intentional breakage
         raise Exception("intentional exception")
 
@@ -27,9 +29,11 @@ class VersionResource:
     def __init__(self, basedir):
         self.basedir = basedir
 
+    @METRICS.timer_decorator(
+        "collector.pageview_duration_ms", tags=["method:get", "path:/__version__"]
+    )
     def on_get(self, req, resp):
         """Implement GET HTTP request."""
-        METRICS.incr("collector.health.version.count")
         version_info = get_version_info(self.basedir)
 
         resp.content_type = "application/json; charset=utf-8"
@@ -40,9 +44,11 @@ def on_get(self, req, resp):
 class LBHeartbeatResource:
     """Handle ``/__lbheartbeat__`` to let the load balancing know application health."""
 
+    @METRICS.timer_decorator(
+        "collector.pageview_duration_ms", tags=["method:get", "path:/__lbheartbeat__"]
+    )
     def on_get(self, req, resp):
         """Implement GET HTTP request."""
-        METRICS.incr("collector.health.lbheartbeat.count")
         resp.content_type = "application/json; charset=utf-8"
         resp.status = falcon.HTTP_200
 
@@ -75,9 +81,11 @@ class HeartbeatResource:
     def __init__(self, app):
         self.antenna_app = app
 
+    @METRICS.timer_decorator(
+        "collector.pageview_duration_ms", tags=["method:get", "path:/__heartbeat__"]
+    )
     def on_get(self, req, resp):
         """Implement GET HTTP request."""
-        METRICS.incr("collector.health.heartbeat.count")
         state = HealthState()
 
         # So we're going to think of Antenna like a big object graph and

diff --git a/antenna/statsd_metrics.yaml b/antenna/statsd_metrics.yaml
@@ -118,22 +118,12 @@ socorro.collector.crashmover.crash_publish.time:
   description: |
     Timer for how long it takes to publish a crash report for processing.
 
-socorro.collector.health.broken.count:
-  type: "incr"
-  description: |
-    Counter for ``/__broken__`` view.
-
-socorro.collector.health.version.count:
-  type: "incr"
+socorro.collector.pageview_duration_ms:
+  type: "timing"
   description: |
-    Counter for ``/__version__`` view.
+    Timer for how long it took to handle an HTTP request.
 
-socorro.collector.health.lbheartbeat.count:
-  type: "incr"
-  description: |
-    Counter for ``/__lbheartbeat__`` view.
+    Tags:
 
-socorro.collector.health.heartbeat.count:
-  type: "incr"
-  description: |
-    Counter for ``/__heartbeat__`` view.
+    * ``path``: the request path
+    * ``method``: the request method
diff --git a/tests/test_breakpad_resource.py b/tests/test_breakpad_resource.py
@@ -474,7 +474,11 @@ def test_submit_crash_report(self, client, metricsmock):
         mm.assert_timing("socorro.collector.crashmover.crash_publish.time")
         mm.assert_incr("socorro.collector.crashmover.save_crash.count")
         mm.assert_timing("socorro.collector.crashmover.crash_handling.time")
-        mm.assert_timing("socorro.collector.breakpad_resource.on_post.time")
+
+        mm.assert_timing(
+            "socorro.collector.pageview_duration_ms",
+            tags=["method:post", "path:/submit", AnyTagValue("host")],
+        )
 
     def test_existing_uuid(self, client):
         """Verify if the crash report has a uuid already, it's reused."""

diff --git a/tests/test_sentry.py b/tests/test_sentry.py
@@ -47,6 +47,17 @@
                             "post_context": ANY,
                             "pre_context": ANY,
                         },
+                        {
+                            "abs_path": "/usr/local/lib/python3.11/site-packages/markus/main.py",
+                            "context_line": ANY,
+                            "filename": "markus/main.py",
+                            "function": "_timer_decorator",
+                            "in_app": False,
+                            "lineno": ANY,
+                            "module": "markus.main",
+                            "post_context": ANY,
+                            "pre_context": ANY,
+                        },
                         {
                             "abs_path": "/app/antenna/health_resource.py",
                             "context_line": ANY,