matrix-org / synapse

Synapse: Matrix homeserver written in Python/Twisted.
https://matrix-org.github.io/synapse
Apache License 2.0
11.79k stars 2.13k forks source link

Metrics work through http listener when enable_metrics false #16222

Open tcpipuk opened 11 months ago

tcpipuk commented 11 months ago

Description

Not a critical bug, but the "http" listener on generic_workers allows metrics when enable_metrics: false - this doesn't happen on the Synapse main process, which informs the user that metrics are disabled until enable_metrics is changed.

Steps to reproduce

Homeserver

tcpip.uk

Synapse Version

1.91.0

Installation Method

Docker (matrixdotorg/synapse)

Database

PostgreSQL 15.4

Workers

Multiple workers

Platform

Ubuntu 22.04 baremetal

Configuration

No response

Relevant log output

Not sure what to put here, metrics are served (through Twisted rather than sidecar) when the Synapse main process doesn't allow it.

Anything else that would be useful to know?

No response

DMRobertson commented 11 months ago

That's a surprise: according to https://github.com/matrix-org/synapse/blob/358896e1b835bf693ef40d4cf9f10077432e935b/synapse/app/generic_worker.py#L252-L267 you should see a warning and we shouldn't start serving metrics.

DMRobertson commented 11 months ago

Create worker_listener of "http" type with ["metrics"] resource

Oh, if this is the case then we wouldn't hit the path I highlighted above.

DMRobertson commented 11 months ago

Is the root problem here that we have an overlap between the concept of a metrics listener and a metrics resource?

https://matrix-org.github.io/synapse/latest/metrics-howto.html presents both options and suggests that the reason you'd want a metrics listener is because

[...] the metrics server on a different port, in a different thread to Synapse. This can make it more resilient to heavy load meaning metrics cannot be retrieved, and can be exposed to just internal networks easier.

DMRobertson commented 11 months ago

Homeserver:

https://github.com/matrix-org/synapse/blob/1c802de626de3293049206cb788af15cbc8ea17f/synapse/app/homeserver.py#L250-L253

Generic worker:

https://github.com/matrix-org/synapse/blob/358896e1b835bf693ef40d4cf9f10077432e935b/synapse/app/generic_worker.py#L237-L238

clokep commented 11 months ago

(I had a long-term plan to merge those start_listening methods which I never got to finishing.)