Remove metrics listener type in favor of http listener with metrics resource

changelog.d/18584.removal

@@ -0,0 +1 @@
+Remove `metrics` listener (serves metrics at `/metrics`) type in favor of `http` listener with `metrics` resource (serves metrics at `/_synapse/metrics`).

docs/metrics-howto.md

@@ -60,8 +60,11 @@
 
       # beginning of the new metrics listener
       - port: 9000
-        type: metrics
+        type: http
         bind_addresses: ['::1', '127.0.0.1']
+        resources:
+          - names: [metrics]
+            compress: false
     ```
 
 1.  Restart Synapse.

docs/usage/configuration/config_documentation.md

@@ -494,7 +494,7 @@ Options for each entry include:
 
 * `bind_addresses` (array|null): A list of local addresses to listen on. The default is "all local interfaces".
 
-* `type` (string): The type of listener. Normally `http`, but other valid options are [`manhole`](../../manhole.md) and [`metrics`](../../metrics-howto.md).
+* `type` (string): The type of listener. Normally `http`, but other valid options are [`manhole`](../../manhole.md).
 
 * `tls` (boolean): Set to true to enable TLS for this listener. Will use the TLS key/cert specified in tls_private_key_path/tls_certificate_path.
 

schema/synapse-config.schema.yaml

@@ -451,8 +451,7 @@ properties:
           type: string
           description: >-
             The type of listener. Normally `http`, but other valid options are
-            [`manhole`](../../manhole.md) and
-            [`metrics`](../../metrics-howto.md).
+            [`manhole`](../../manhole.md).
           enum:
             - http
             - manhole

synapse/app/_base.py

@@ -283,42 +283,6 @@ async def wrapper() -> None:
     reactor.callWhenRunning(lambda: defer.ensureDeferred(wrapper()))
 
 
-def listen_metrics(bind_addresses: StrCollection, port: int) -> None:
-    """
-    Start Prometheus metrics server.
-    """
-    from prometheus_client import start_http_server as start_http_server_prometheus
-
-    from synapse.metrics import RegistryProxy
-
-    for host in bind_addresses:
-        logger.info("Starting metrics listener on %s:%d", host, port)
-        _set_prometheus_client_use_created_metrics(False)
-        start_http_server_prometheus(port, addr=host, registry=RegistryProxy)
-
-
-def _set_prometheus_client_use_created_metrics(new_value: bool) -> None:
-    """
-    Sets whether prometheus_client should expose `_created`-suffixed metrics for
-    all gauges, histograms and summaries.
-    There is no programmatic way to disable this without poking at internals;
-    the proper way is to use an environment variable which prometheus_client
-    loads at import time.
-
-    The motivation for disabling these `_created` metrics is that they're
-    a waste of space as they're not useful but they take up space in Prometheus.
-    """
-
-    import prometheus_client.metrics
-
-    if hasattr(prometheus_client.metrics, "_use_created"):
-        prometheus_client.metrics._use_created = new_value
-    else:
-        logger.error(
-            "Can't disable `_created` metrics in prometheus_client (brittle hack broken?)"
-        )
-
-
 def listen_manhole(
     bind_addresses: StrCollection,
     port: int,

synapse/app/generic_worker.py

@@ -283,23 +283,6 @@ def start_listening(self) -> None:
                     raise ConfigError(
                         "Can not using a unix socket for manhole at this time."
                     )
-
-            elif listener.type == "metrics":
-                if not self.config.metrics.enable_metrics:
-                    logger.warning(
-                        "Metrics listener configured, but enable_metrics is not True!"
-                    )
-                else:
-                    if isinstance(listener, TCPListenerConfig):
-                        _base.listen_metrics(
-                            listener.bind_addresses,
-                            listener.port,
-                        )
-                    else:
-                        raise ConfigError(
-                            "Can not use a unix socket for metrics at this time."
-                        )
-
             else:
                 logger.warning("Unsupported listener type: %s", listener.type)
 

synapse/app/homeserver.py

@@ -286,22 +286,6 @@ def start_listening(self) -> None:
                     raise ConfigError(
                         "Can not use a unix socket for manhole at this time."
                     )
-            elif listener.type == "metrics":
-                if not self.config.metrics.enable_metrics:
-                    logger.warning(
-                        "Metrics listener configured, but enable_metrics is not True!"
-                    )
-                else:
-                    if isinstance(listener, TCPListenerConfig):
-                        _base.listen_metrics(
-                            listener.bind_addresses,
-                            listener.port,
-                        )
-                    else:
-                        raise ConfigError(
-                            "Can not use a unix socket for metrics at this time."
-                        )
-
             else:
                 # this shouldn't happen, as the listener type should have been checked
                 # during parsing

synapse/metrics/_twisted_exposition.py

@@ -20,14 +20,49 @@
 #
 #
 
-from prometheus_client import REGISTRY, CollectorRegistry, generate_latest
+import logging
+
+from prometheus_client import (
+    REGISTRY,
+    CollectorRegistry,
+    generate_latest,
+)
 
 from twisted.web.resource import Resource
 from twisted.web.server import Request
 
+logger = logging.getLogger(__name__)
+
 CONTENT_TYPE_LATEST = "text/plain; version=0.0.4; charset=utf-8"
 
 
+def _set_prometheus_client_use_created_metrics(new_value: bool) -> None:
+    """
+    Sets whether prometheus_client should expose `_created`-suffixed metrics for
+    all gauges, histograms and summaries.
+
+    There is no programmatic way in the old versions of `prometheus_client` to disable
+    this without poking at internals; the proper way in the old `prometheus_client`
+    versions is to use an environment variable which prometheus_client loads at import
+    time OR to use the dedicated `disable_created_metrics()`/`enable_created_metrics()`
+    functions, but these were only added in `prometheus_client` `0.18.0`.
+
+    The motivation for disabling these `_created` metrics is that they're
+    a waste of space as they're not useful but they take up space in Prometheus.
+    """
+    import prometheus_client.metrics
+
+    if hasattr(prometheus_client.metrics, "_use_created"):
+        prometheus_client.metrics._use_created = new_value
+    else:
+        logger.error(
+            "Can't disable `_created` metrics in prometheus_client (brittle hack broken?)"
+        )
+
+
+_set_prometheus_client_use_created_metrics(False)
+
+
 class MetricsResource(Resource):
     """
     Twisted ``Resource`` that serves prometheus metrics.

tests/metrics/test_metrics.py

@@ -18,14 +18,10 @@
 # [This file includes modifications made by New Vector Limited]
 #
 #
-from importlib import metadata
 from typing import Dict, Protocol, Tuple
-from unittest.mock import patch
 
-from pkg_resources import parse_version
 from prometheus_client.core import Sample
 
-from synapse.app._base import _set_prometheus_client_use_created_metrics
 from synapse.metrics import REGISTRY, InFlightGauge, generate_latest
 from synapse.util.caches.deferred_cache import DeferredCache
 
@@ -184,30 +180,3 @@ def test_cache_metric(self) -> None:
 
         self.assertEqual(items["synapse_util_caches_cache_size"], "1.0")
         self.assertEqual(items["synapse_util_caches_cache_max_size"], "777.0")
-
-
-class PrometheusMetricsHackTestCase(unittest.HomeserverTestCase):
-    if parse_version(metadata.version("prometheus_client")) < parse_version("0.14.0"):
-        skip = "prometheus-client too old"
-
-    def test_created_metrics_disabled(self) -> None:
-        """
-        Tests that a brittle hack, to disable `_created` metrics, works.
-        This involves poking at the internals of prometheus-client.
-        It's not the end of the world if this doesn't work.
-
-        This test gives us a way to notice if prometheus-client changes
-        their internals.
-        """
-        import prometheus_client.metrics
-
-        PRIVATE_FLAG_NAME = "_use_created"
-
-        # By default, the pesky `_created` metrics are enabled.
-        # Check this assumption is still valid.
-        self.assertTrue(getattr(prometheus_client.metrics, PRIVATE_FLAG_NAME))
-
-        with patch("prometheus_client.metrics") as mock:
-            setattr(mock, PRIVATE_FLAG_NAME, True)
-            _set_prometheus_client_use_created_metrics(False)
-            self.assertFalse(getattr(mock, PRIVATE_FLAG_NAME, False))