Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Remove legacy Prometheus metrics names. They were deprecated in Synapse v1.69.0 and disabled by default in Synapse v1.71.0. #14538

Merged
merged 6 commits into from
Nov 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/14538.removal
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Remove legacy Prometheus metrics names. They were deprecated in Synapse v1.69.0 and disabled by default in Synapse v1.71.0.
22 changes: 22 additions & 0 deletions docs/upgrade.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,28 @@ process, for example:
dpkg -i matrix-synapse-py3_1.3.0+stretch1_amd64.deb
```

# Upgrading to v1.73.0

## Legacy Prometheus metric names have now been removed

Synapse v1.69.0 included the deprecation of legacy Prometheus metric names
and offered an option to disable them.
Synapse v1.71.0 disabled legacy Prometheus metric names by default.

This version, v1.73.0, removes those legacy Prometheus metric names entirely.
This also means that the `enable_legacy_metrics` configuration option has been
removed; it will no longer be possible to re-enable the legacy metric names.

If you use metrics and have not yet updated your Grafana dashboard(s),
Prometheus console(s) or alerting rule(s), please consider doing so when upgrading
to this version.
Note that the included Grafana dashboard was updated in v1.72.0 to correct some
metric names which were missed when legacy metrics were disabled by default.

See [v1.69.0: Deprecation of legacy Prometheus metric names](#deprecation-of-legacy-prometheus-metric-names)
for more context.


# Upgrading to v1.72.0

## Dropping support for PostgreSQL 10
Expand Down
25 changes: 0 additions & 25 deletions docs/usage/configuration/config_documentation.md
Original file line number Diff line number Diff line change
Expand Up @@ -2437,31 +2437,6 @@ Example configuration:
enable_metrics: true
```
---
### `enable_legacy_metrics`

Set to `true` to publish both legacy and non-legacy Prometheus metric names,
or to `false` to only publish non-legacy Prometheus metric names.
Defaults to `false`. Has no effect if `enable_metrics` is `false`.
**In Synapse v1.67.0 up to and including Synapse v1.70.1, this defaulted to `true`.**

Legacy metric names include:
- metrics containing colons in the name, such as `synapse_util_caches_response_cache:hits`, because colons are supposed to be reserved for user-defined recording rules;
- counters that don't end with the `_total` suffix, such as `synapse_federation_client_sent_edus`, therefore not adhering to the OpenMetrics standard.

These legacy metric names are unconventional and not compliant with OpenMetrics standards.
They are included for backwards compatibility.

Example configuration:
```yaml
enable_legacy_metrics: false
```

See https://github.com/matrix-org/synapse/issues/11106 for context.

*Since v1.67.0.*

**Will be removed in v1.73.0.**
---
### `sentry`

Use this option to enable sentry integration. Provide the DSN assigned to you by sentry
Expand Down
16 changes: 4 additions & 12 deletions synapse/app/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -266,26 +266,18 @@ async def wrapper() -> None:
reactor.callWhenRunning(lambda: defer.ensureDeferred(wrapper()))


def listen_metrics(
bind_addresses: Iterable[str], port: int, enable_legacy_metric_names: bool
) -> None:
def listen_metrics(bind_addresses: Iterable[str], port: int) -> None:
"""
Start Prometheus metrics server.
"""
from prometheus_client import start_http_server as start_http_server_prometheus

from synapse.metrics import (
RegistryProxy,
start_http_server as start_http_server_legacy,
)
from synapse.metrics import RegistryProxy

for host in bind_addresses:
logger.info("Starting metrics listener on %s:%d", host, port)
if enable_legacy_metric_names:
start_http_server_legacy(port, addr=host, registry=RegistryProxy)
else:
_set_prometheus_client_use_created_metrics(False)
start_http_server_prometheus(port, addr=host, registry=RegistryProxy)
_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:
Expand Down
1 change: 0 additions & 1 deletion synapse/app/generic_worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,6 @@ def start_listening(self) -> None:
_base.listen_metrics(
listener.bind_addresses,
listener.port,
enable_legacy_metric_names=self.config.metrics.enable_legacy_metrics,
)
else:
logger.warning("Unsupported listener type: %s", listener.type)
Expand Down
1 change: 0 additions & 1 deletion synapse/app/homeserver.py
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,6 @@ def start_listening(self) -> None:
_base.listen_metrics(
listener.bind_addresses,
listener.port,
enable_legacy_metric_names=self.config.metrics.enable_legacy_metrics,
)
else:
# this shouldn't happen, as the listener type should have been checked
Expand Down
2 changes: 0 additions & 2 deletions synapse/config/metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@ class MetricsConfig(Config):
def read_config(self, config: JsonDict, **kwargs: Any) -> None:
self.enable_metrics = config.get("enable_metrics", False)

self.enable_legacy_metrics = config.get("enable_legacy_metrics", False)

Comment on lines -46 to -47
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we raise a warning telling admins that this setting has been removed and is ignored?

(Probably not? If we're going to rip this out, we should rip it out)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I don't see a big need to. Nobody will be harmed if they leave this in and the option only existed for 4 versions. I wouldn't even be sure that anyone used the options.

self.report_stats = config.get("report_stats", None)
self.report_stats_endpoint = config.get(
"report_stats_endpoint", "https://matrix.org/report-usage-stats/push"
Expand Down
7 changes: 1 addition & 6 deletions synapse/metrics/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,7 @@
# This module is imported for its side effects; flake8 needn't warn that it's unused.
import synapse.metrics._reactor_metrics # noqa: F401
from synapse.metrics._gc import MIN_TIME_BETWEEN_GCS, install_gc_manager
from synapse.metrics._legacy_exposition import (
MetricsResource,
generate_latest,
start_http_server,
)
from synapse.metrics._twisted_exposition import MetricsResource, generate_latest
from synapse.metrics._types import Collector
from synapse.util import SYNAPSE_VERSION

Expand Down Expand Up @@ -474,7 +470,6 @@ def register_threadpool(name: str, threadpool: ThreadPool) -> None:
"Collector",
"MetricsResource",
"generate_latest",
"start_http_server",
"LaterGauge",
"InFlightGauge",
"GaugeBucketCollector",
Expand Down
Loading