Refactor GaugeBucketCollector metrics to be homeserver-scoped

[MadLittleMods]

Refactor GaugeBucketCollector metrics to be homeserver-scoped

Part of #18592

Testing strategy

1. Add the metrics listener in your homeserver.yaml

   listeners:
     # This is just showing how to configure metrics either way
     #
     # `http` `metrics` resource
     - port: 9322
       type: http
       bind_addresses: ['127.0.0.1']
       resources:
         - names: [metrics]
           compress: false
     # `metrics` listener
     - port: 9323
       type: metrics
       bind_addresses: ['127.0.0.1']

2. Start the homeserver: poetry run synapse_homeserver --config-path homeserver.yaml
3. Fetch http://localhost:9322/_synapse/metrics and/or http://localhost:9323/metrics
4. Adjust the number of msecs in the looping_call so that _read_forward_extremities runs immediately instead of after an hour.
5. Observe response includes the synapse_forward_extremities and synapse_excess_extremity_events metrics with the server_name label

Pull Request Checklist

• Pull request is based on the develop branch
• Pull request includes a changelog file. The entry should:
  • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
  • Use markdown where necessary, mostly for code blocks.
  • End with either a period (.) or an exclamation mark (!).
  • Start with a capital letter.
  • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
• Code style is correct (run the linters)

[devonh]

What if people add metrics in the future that use the upstream GaugeHistogram type and we don't catch that it doesn't have the server_name label?

[devonh]

Is this function exactly the same as from the upstream code?
It looks like it is.

If so, could we not just inherit from GaugeHistogramMetricFamily directly (instead of Metric) to not need to duplicate this code?
It's already not ideal to need to almost duplicate the __init__ function.

[MadLittleMods]

I've updated to inherit from GaugeHistogramMetricFamily. It seems a bit brittle as we're just relying on setting self._labelnames = tuple(labelnames) and hoping the super implementation does something with it.

I think I'd rather just have our own full-custom version but will go forward with your preference.

[devonh]

I see your point. Both cases have a level of fragility. Since if the upstream version changes something else about how it is adding metrics, then we have no way of knowing we should update our copied code snippet.

But either way, I think this PR is good to go. If we see this being too fragile the way it is, we can always revisit. Since it's just a metric label we aren't dealing with a potential Synapse functionality breakage so the stakes are low.

[MadLittleMods]

What if people add metrics in the future that use the upstream GaugeHistogram type and we don't catch that it doesn't have the server_name label?

-- @devonh, #18715 (review)

The linting for this is coming as part of #18733

It doesn't really matter whether lints are introduced along with the changes as when we introduce the lints, it will catch whatever is leftover. To be clear, I think this PR is exhaustive but I'm just pointing out that it isn't a hard requirement.

[devonh]

Small request to comment the risk of setting labelname.
Otherwise this looks good! Thanks for bearing with me :)

[devonh]

Since we are relying on the inheritance now to apply the labels, we should add a comment here to describe that risk / fragility.

[MadLittleMods]

As an alternative, I've updated to use the super() constructor to set the internal label names field. This feels a bit better to use their stable API to get everything done.

[devonh]

I see your point. Both cases have a level of fragility. Since if the upstream version changes something else about how it is adding metrics, then we have no way of knowing we should update our copied code snippet.

But either way, I think this PR is good to go. If we see this being too fragile the way it is, we can always revisit. Since it's just a metric label we aren't dealing with a potential Synapse functionality breakage so the stakes are low.

[MadLittleMods]

Thanks for the review @devonh 🦚