Iterate on the prometheus logger with tests

These don't pass every time (depends on test ordering and I don't know
why)

plugins/routemaster-prometheus/routemaster_prometheus/__init__.py

@@ -9,6 +9,7 @@
 import pathlib
 import contextlib
 from timeit import default_timer as timer
+from werkzeug.routing import NotFound, RequestRedirect, MethodNotAllowed
 
 from prometheus_client import (
     CONTENT_TYPE_LATEST,
@@ -51,12 +52,6 @@
     ('method', 'endpoint', 'status'),
 )
 
-api_counter = Counter(
-    'routemaster_api_request_total',
-    'Total number of API requests',
-    ('method', 'status', 'endpoint'),
-)
-
 
 class PrometheusLogger(BaseLogger):
     """Instruments Routemaster with Prometheus."""
@@ -83,8 +78,13 @@ def __init__(
 
     def init_flask(self, flask_app):
         """Instrument Flask with Prometheus."""
+        self.url_adapter = flask_app.url_map.bind('localhost')
+        self.endpoint_lookup = {
+            rule.endpoint: rule.rule
+            for rule in flask_app.url_map.iter_rules()
+        }
 
-        @flask_app.route(self.path)
+        @flask_app.route(self.path, endpoint=self.path)
         def get_metrics():
             registry = CollectorRegistry()
             MultiProcessCollector(registry)
@@ -159,24 +159,31 @@ def process_request_finished(
         exc_info,
     ):
         """Log completed request metrics, given the timer we started."""
+        if exc_info:
+            exceptions.labels(type='api').inc()
+
         total_time = max(timer() - environ['_PROMETHEUS_REQUEST_TIMER'], 0)
-        path = environ.get('PATH_INFO', '')
+        path = environ['PATH_INFO']
+        method = environ['REQUEST_METHOD']
 
         if path == self.path:
             return
 
+        endpoint = ''
+
+        try:
+            match = self.url_adapter.match(path, method=method)
+            if match:
+                endpoint = self.endpoint_lookup[match[0]]
+        except (RequestRedirect, MethodNotAllowed, NotFound):
+            pass
+
         api_histogram.labels(
-            method=environ.get('REQUEST_METHOD'),
-            endpoint=path,
+            method=method,
             status=status,
+            endpoint=endpoint,
         ).observe(total_time)
 
-        api_counter.labels(
-            method=environ.get('REQUEST_METHOD'),
-            status=status,
-            endpoint=path,
-        ).inc()
-
 
 def _clear_directory(path: pathlib.Path):
     for root, dirs, files in os.walk(path):

plugins/tests/test_logging_plugins.py

@@ -7,7 +7,9 @@
 from flask import Flask
 from routemaster_sentry import SentryLogger
 from routemaster_prometheus import PrometheusLogger
+from prometheus_client.parser import text_string_to_metric_families
 
+from routemaster.config import LoggingPluginConfig
 from routemaster.logging import BaseLogger, SplitLogger
 
 SENTRY_KWARGS = {
@@ -106,3 +108,73 @@ def test_prometheus_logger_wipes_directory_on_startup(app):
 
     assert not filepath.exists()
     assert not dirpath.exists()
+
+
+def test_prometheus_logger_metrics(custom_app, custom_client):
+    test_metrics_path = '/test_prometheus_logger_metrics'
+    app = custom_app(
+        logging_plugins=[
+            LoggingPluginConfig(
+                dotted_path='routemaster_prometheus:PrometheusLogger',
+                kwargs={'path': test_metrics_path},
+            ),
+        ],
+    )
+    client = custom_client(app)
+
+    wsgi_environ = {
+        'REQUEST_METHOD': 'GET',
+        'PATH_INFO': '/',
+    }
+    app.logger.process_request_started(wsgi_environ)
+    app.logger.process_request_finished(
+        wsgi_environ,
+        status=200,
+        headers={},
+        exc_info=None,
+    )
+
+    metrics_response = client.get(test_metrics_path)
+    metrics_data = metrics_response.data.decode('utf-8')
+    metric_families = list(text_string_to_metric_families(metrics_data))
+    samples = [y for x in metric_families for y in x.samples]
+
+    assert (
+        'routemaster_api_request_duration_seconds_count',
+        {'method': 'GET', 'status': '200', 'endpoint': '/'},
+        1.0,
+    ) in samples
+
+
+def test_prometheus_logger_ignores_metrics_path(custom_app, custom_client):
+    test_metrics_path = '/test_prometheus_logger_ignores_metrics_path'
+    app = custom_app(
+        logging_plugins=[
+            LoggingPluginConfig(
+                dotted_path='routemaster_prometheus:PrometheusLogger',
+                kwargs={'path': test_metrics_path},
+            ),
+        ],
+    )
+    client = custom_client(app)
+
+    metrics_response = client.get(test_metrics_path)
+    metrics_data = metrics_response.data.decode('utf-8')
+    metric_families = list(text_string_to_metric_families(metrics_data))
+    samples = [y for x in metric_families for y in x.samples]
+
+    assert (
+        'routemaster_api_request_duration_seconds_count',
+        {'method': 'GET', 'status': '200', 'endpoint': test_metrics_path},
+        1.0,
+    ) not in samples
+
+
+def test_prometheus_logger_validates_metrics_path(app):
+    orig = os.environ['prometheus_multiproc_dir']
+    os.environ['prometheus_multiproc_dir'] = ''
+
+    with pytest.raises(ValueError):
+        PrometheusLogger(app.config)
+
+    os.environ['prometheus_multiproc_dir'] = orig