Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Prometheus-style metrics #296

Closed
leonerd opened this issue Dec 2, 2016 · 8 comments
Closed

Add Prometheus-style metrics #296

leonerd opened this issue Dec 2, 2016 · 8 comments

Comments

@leonerd
Copy link
Contributor

leonerd commented Dec 2, 2016

(This is somewhat of a notes-to-self whiteboard to keep track of work on https://github.com/matrix-org/matrix-appservice-irc/tree/paul/metrics)

In particular, as well as adding whatever the bridge library has, it would be nice to preserve the values of the current statsd-based reporting. The four variables are:

ircas_connected_clients_gauge{}

ircas_membership_counter{protocol,state}

ircas_memory_gauge

ircas_requests_timer{direction,outcome}
ircas_requests_timer_count{...}
ircas_requests_timer_sum{...}
@leonerd
Copy link
Contributor Author

leonerd commented Dec 2, 2016

ircas_memory_gauge

Already handled by the prometheus client library, as process_memory_*

@leonerd
Copy link
Contributor Author

leonerd commented Dec 2, 2016

ircas_connected_clients_gauge

Well this is a fun one. The underlying statsd-exporting code in the IRC bridge exports this metric on a per-domain basis, but a bug in the statsd-to-prometheus bridge config (https://github.com/matrix-org/synapse-prometheus-config/blob/master/irc-as/statsd-prometheus-mappings.cfg#L14) currently means that only one of these metrics will turn up in prometheus, and not annotated by a domain name. This doesn't affect us in practice because we only run one domain per bridge instance anyway.

a75cb6a adds code to export this on a bridge-wide basis, counting all the users across all domains, as the bridge-standard metric of bridge_remote_ghosts

@leonerd
Copy link
Contributor Author

leonerd commented Dec 2, 2016

ircas_membership_counter{protocol,state}

Having IRL'ed this with @kegsay as to the intention of it, it seems it can be dropped.

The intention is to measure the rate of outbound join and part requests the bridge makes to IRC and Matrix. On the Matrix side we already have the bridge_matrix_api_calls counter which is automatically managed by the matrix SDK, and adding the usual complementary bridge_remote_api_calls counter to the IRC side in the bridge will cover it.

@leonerd
Copy link
Contributor Author

leonerd commented Dec 2, 2016

ircas_requests_timer{direction,outcome}
ircas_requests_timer_count{...}
ircas_requests_timer_sum{...}

These are supposed to track the total "end-to-end" lag of events within the bridge. The main bridge library doesn't have a direct equivalent for these yet, but they'd also be useful to add generally - gitter and slack can make equally-good use of it.

However...

This metric is a Histogram, and the prometheus library we're currently using (npm prometheus-client) doesn't actually support those. To get that we'd want to switch to prom-client; which I think would be good anyway as that generally seems to be a better library.

@leonerd
Copy link
Contributor Author

leonerd commented Dec 7, 2016

We now have prom-client in the bridge library as of

"matrix-appservice-bridge": "matrix-org/matrix-appservice-bridge#ca40ff5",

@leonerd
Copy link
Contributor Author

leonerd commented Dec 8, 2016

Commit 9dc1980 now adds request duration timer support, using prom-client's Histograms.

@kegsay - do you want a PR now then, for the work so far? Currently all I've done is export the same things that the statsd-related code does, in Prometheus style. I'd still like to get some of the other metrics (like the bridge gauges) working at some point.

@kegsay
Copy link
Member

kegsay commented Dec 8, 2016

Yes please. Send me a PR and then do another one for the extra stuff. Thanks!

@kegsay
Copy link
Member

kegsay commented Dec 20, 2016

We have feature parity now with statsd, any more is just bonus material. This issue is done.

@kegsay kegsay closed this as completed Dec 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants