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.
 

pyproject.toml

@@ -1,38 +1,38 @@
 [tool.towncrier]
-    package = "synapse"
-    filename = "CHANGES.md"
-    directory = "changelog.d"
-    issue_format = "[\\#{issue}](https://github.com/element-hq/synapse/issues/{issue})"
-
-    [[tool.towncrier.type]]
-        directory = "feature"
-        name = "Features"
-        showcontent = true
-
-    [[tool.towncrier.type]]
-        directory = "bugfix"
-        name = "Bugfixes"
-        showcontent = true
-
-    [[tool.towncrier.type]]
-        directory = "docker"
-        name = "Updates to the Docker image"
-        showcontent = true
-
-    [[tool.towncrier.type]]
-        directory = "doc"
-        name = "Improved Documentation"
-        showcontent = true
-
-    [[tool.towncrier.type]]
-        directory = "removal"
-        name = "Deprecations and Removals"
-        showcontent = true
-
-    [[tool.towncrier.type]]
-        directory = "misc"
-        name = "Internal Changes"
-        showcontent = true
+package = "synapse"
+filename = "CHANGES.md"
+directory = "changelog.d"
+issue_format = "[\\#{issue}](https://github.com/element-hq/synapse/issues/{issue})"
+
+[[tool.towncrier.type]]
+directory = "feature"
+name = "Features"
+showcontent = true
+
+[[tool.towncrier.type]]
+directory = "bugfix"
+name = "Bugfixes"
+showcontent = true
+
+[[tool.towncrier.type]]
+directory = "docker"
+name = "Updates to the Docker image"
+showcontent = true
+
+[[tool.towncrier.type]]
+directory = "doc"
+name = "Improved Documentation"
+showcontent = true
+
+[[tool.towncrier.type]]
+directory = "removal"
+name = "Deprecations and Removals"
+showcontent = true
+
+[[tool.towncrier.type]]
+directory = "misc"
+name = "Internal Changes"
+showcontent = true
 
 [tool.ruff]
 line-length = 88
@@ -47,11 +47,7 @@ target-version = "py39"
 # flake8-bugbear compatible checks. Its error codes are described at
 # https://beta.ruff.rs/docs/rules/#flake8-bugbear-b
 #  B023: Functions defined inside a loop must not use variables redefined in the loop
-ignore = [
-    "B023",
-    "E501",
-    "E731",
-]
+ignore = ["B023", "E501", "E731"]
 select = [
     # pycodestyle
     "E",
@@ -82,7 +78,15 @@ select = [
 
 [tool.ruff.lint.isort]
 combine-as-imports = true
-section-order = ["future", "standard-library", "third-party", "twisted", "first-party", "testing", "local-folder"]
+section-order = [
+    "future",
+    "standard-library",
+    "third-party",
+    "twisted",
+    "first-party",
+    "testing",
+    "local-folder",
+]
 known-first-party = ["synapse"]
 
 [tool.ruff.lint.isort.sections]
@@ -107,9 +111,7 @@ authors = ["Matrix.org Team and Contributors <[email protected]>"]
 license = "AGPL-3.0-or-later"
 readme = "README.rst"
 repository = "https://github.com/element-hq/synapse"
-packages = [
-    { include = "synapse" },
-]
+packages = [{ include = "synapse" }]
 classifiers = [
     "Development Status :: 5 - Production/Stable",
     "Topic :: Communications :: Chat",
@@ -125,7 +127,7 @@ include = [
     { path = "INSTALL.md", format = "sdist" },
     { path = "mypy.ini", format = "sdist" },
     { path = "scripts-dev", format = "sdist" },
-    { path = "synmark", format="sdist" },
+    { path = "synmark", format = "sdist" },
     { path = "sytest-blacklist", format = "sdist" },
     { path = "tests", format = "sdist" },
     { path = "UPGRADE.rst", format = "sdist" },
@@ -135,9 +137,7 @@ include = [
     { path = "rust/build.rs", format = "sdist" },
     { path = "rust/src/**", format = "sdist" },
 ]
-exclude = [
-    { path = "synapse/*.so", format = "sdist"}
-]
+exclude = [{ path = "synapse/*.so", format = "sdist" }]
 
 [tool.poetry.build]
 script = "build_rust.py"
@@ -178,7 +178,7 @@ signedjson = "^1.1.0"
 service-identity = ">=18.1.0"
 # Twisted 18.9 introduces some logger improvements that the structured
 # logger utilises
-Twisted = {extras = ["tls"], version = ">=18.9.0"}
+Twisted = { extras = ["tls"], version = ">=18.9.0" }
 treq = ">=15.1"
 # Twisted has required pyopenssl 16.0 since about Twisted 16.6.
 pyOpenSSL = ">=16.0.0"
@@ -195,7 +195,9 @@ pymacaroons = ">=0.13.0"
 msgpack = ">=0.5.2"
 phonenumbers = ">=8.2.0"
 # we use GaugeHistogramMetric, which was added in prom-client 0.4.0.
-prometheus-client = ">=0.4.0"
+# `prometheus_client.metrics` was added in 0.5.0, so we require that too.
+# We chose 0.6.0 as that is the current version in Debian Buster (oldstable).
+prometheus-client = ">=0.6.0"
 # we use `order`, which arrived in attrs 19.2.0.
 # Note: 21.1.0 broke `/sync`, see https://github.com/matrix-org/synapse/issues/9936
 attrs = ">=19.2.0,!=21.1.0"
@@ -297,7 +299,9 @@ all = [
     # matrix-synapse-ldap3
     "matrix-synapse-ldap3",
     # postgres
-    "psycopg2", "psycopg2cffi", "psycopg2cffi-compat",
+    "psycopg2",
+    "psycopg2cffi",
+    "psycopg2cffi-compat",
     # saml2
     "pysaml2",
     # oidc and jwt
@@ -307,9 +311,11 @@ all = [
     # sentry
     "sentry-sdk",
     # opentracing
-    "jaeger-client", "opentracing",
+    "jaeger-client",
+    "opentracing",
     # redis
-    "txredisapi", "hiredis",
+    "txredisapi",
+    "hiredis",
     # cache-memory
     "pympler",
     # improved user search
@@ -397,8 +403,8 @@ enable = "pypy"
 #
 # We temporarily pin Rust to 1.82.0 to work around
 # https://github.com/element-hq/synapse/issues/17988
-before-all =  "sh .ci/before_build_wheel.sh"
-environment= { PATH = "$PATH:$HOME/.cargo/bin" }
+before-all = "sh .ci/before_build_wheel.sh"
+environment = { PATH = "$PATH:$HOME/.cargo/bin" }
 
 # For some reason if we don't manually clean the build directory we
 # can end up polluting the next build with a .so that is for the wrong

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