feat: add room count metric

[itsoyou]

Expose room count as well as locally joined room count in metrics #3242

This is revised version of the previous pr to align more consistent coding style.

• synapse_rooms_total: Rooms currently stored in the database, for example like the query select count(*) from rooms;, but there should already be a helper function available in Synapse.
• synapse_locally_joined_rooms_total: Rooms with at least one local member. Should be similar to this query: select count(*) from room_stats_current where local_users_in_room > 0;

If you have any other better approach please feel free to share it here 🥰

[jason-famedly]

All in all, this looks really cool. I think I will pull the patch and apply it to my server to see what it looks like. I think we get database transaction metrics for free too(maybe, will check if I do patch my server with it)

Could you work up a sample query of what the prometheus query would look like? Would then be helpful for infra when they need to add it to the grafana metrics(and I can cheat and use it too, because I'm lazy 😉 )

[jason-famedly]

Also looks like the one test fails on trial: https://github.com/famedly/synapse/actions/runs/16568291444

the other complement and sytest failures are bogus, ignore them

[itsoyou]

All in all, this looks really cool. I think I will pull the patch and apply it to my server to see what it looks like. I think we get database transaction metrics for free too(maybe, will check if I do patch my server with it)

Could you work up a sample query of what the prometheus query would look like? Would then be helpful for infra when they need to add it to the grafana metrics(and I can cheat and use it too, because I'm lazy 😉 )

you can find metrics with names synapse_known_rooms_total, synapse_locally_joined_rooms_total
I think you can also try synapse_known_rooms_total[24h] and for graph, try avg_over_time(synapse_known_rooms_total[1h]) something like this

[itsoyou]

Check how often room_stats_current this table gets updated!
Maybe update the metric at the spot where the table get updated...

[itsoyou]

When I met Nico on Friday, we talked about it and came up with this:

• Check how often room_stats_current this table gets updated!
• Maybe update the metric at the spot where the table get updated.
• count(*) on room/room_stats_current table wouldn’t be so expensive, unless it has millions of rows. But it would be nice if we have index on local_users_in_room

So I looked into where the room_stats_current table being updated. Mainly 2 places,

1. Room purging https://github.com/famedly/synapse/blob/master/synapse/storage/databases/main/purge_events.py#L334
2. update stats (background update) https://github.com/famedly/synapse/blob/master/synapse/storage/databases/main/stats.py#L135-L137

But I am still on the side that it’s better to have periodic update, since it is simpler to manage, and this is not a time-critical metrics.

And since the room count queries are not super expensive, I'd like to have it updated more often than an hour. around every 15 minutes.

This is another good place to manage all this metrics. Here they make metrics shared between the phone home stats and the prometheus exporter.
https://github.com/famedly/synapse/blob/master/synapse/metrics/common_usage_metrics.py

[nico-famedly]

synapse/synapse/handlers/stats.py

Line 140 in 691f7a2

await self.store.bulk_update_stats_delta(

is called on every new event. So I think we could hook into that update function to update our metrics, at least the room count. The other one is a bit more tricky, but should work as well.

[nico-famedly]

synapse/synapse/handlers/stats.py

Line 148 in 691f7a2

event_processing_positions.labels("stats").set(max_pos)

already does that for another metric, imo.

[jason-famedly]

Here's the query plan

It may be that in the future we'll need an index on local_users_in_room. There is already one on room_id, but that isn't used here. The COUNT(*) is more efficient without it, so don't change that

[nico-famedly]

Looks good to me, although we should likely validate, if this causes any amount of significant CPU increase when receiving or sending events.

[jason-famedly]

Although I usually question the need for metrics tests, perhaps just having some that check what the database calls do instead would be ok. That said, unless there ends up being something ridiculous around performance degradation let's go with this.

Nice job, and thanks for dealing with us being difficult! 😄