Add metrics_labels to make_mutable_config to introduce the value in 3.13 migrated queues, in line with 4.2 queues

[elo-magnier-7s]

Proposed Changes

The mutable ra config is re-injected at every ra_server:start (I think it's start at least!), this will inject the missing metrics_labels that will let 3.13 queues emit raft metrics in 4.2/4.3.
While it is aggressively re-injecting it, I have seen no place where it is overwritten, and the config should not get re-written to disk if it doesn't change (ra_log:write_config is gated behind #{has_changed := true}).

This has to be paired with a ra PR (incoming) that makes the metrics_labels mutable.
Paired ra change: rabbitmq/ra#651

As far as tests go, it seems hard to test, as it requires a 3.13 era queue.
I have made local tests pass on 4.2.9, and did a "live" check by introducing a patched 4.2.9, then going to an unpatched 4.3.4, and the queues still behaved, so by all means, this solution is overkill - it is a one-time mutate then write, once it is in there, the queue is similar to a 4.2 era queue.

Types of Changes

What types of changes does your code introduce to this project?
Put an x in the boxes that apply

• Bug fix (non-breaking change which fixes issue Quorum queues declared on 3.13 do not emit Raft metrics for Prometheus after upgrading to 4.2 #17183)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause an observable behavior change in existing systems)
• Documentation improvements (corrections, new content, etc)
• Cosmetic change (whitespace, formatting, etc)
• Build system and/or CI

Checklist

Put an x in the boxes that apply.
You can also fill these out after creating the PR.
This is simply a reminder of what we are going to look for before merging your code.

• Mandatory: I (or my employer/client) have have signed the CA (see https://github.com/rabbitmq/cla)
• I have read the CONTRIBUTING.md document
• I have added tests that prove my fix is effective or that my feature works
• All tests pass locally with my changes
• If relevant, I have added necessary documentation to https://github.com/rabbitmq/rabbitmq-website
• If relevant, I have added this change to the first version(s) in release-notes that I expect to introduce it

Further Comments

I am not sure of the status of our CA/CLA, we have contributed code before, but my knowledgable people are on leave.

[mergify]

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

• Queue this pull request

[michaelklishin]

Thanks.

Let's start with signing the CLA.

[michaelklishin]

@elo-magnier-7s below is a comment I have posted to the Ra PR but it belongs here just as much.

I'd like to clarify that we are not adding anything back at the ra level, even if from the system monitoring perspective you might consider that metric "lost".

RabbitMQ 3.13-era quorum queues did not persist metrics_labels, so there is nothing to recover from disk on restarts.

This approach seems to be the most viable from a few points of view:

1. We are updating the current code only
2. The change is very localized
3. The data structure we want to store into the metrics state is a map with two scalar values (so there's no nesting complexity, for example)

The only alternative is indeed forcing the Ra members to be re-created, which can mean a non-trivial amount of effort for every operator migrating from 3.13 to 4.2 and onwards.

[michaelklishin]

After considering tweaking prometheus_rabbitmq_raft_metrics_collector:collect_per_object_metrics/2 to inject that key when it is missing as an alternative that requires no ra changes, I conclude that it would require seshat:format/2 changes so that it doesn't skip unlabelled values.

So it's either a tiny ra change or a not-entirely-trivial seshat change plus a small RabbitMQ change in each case.

[elo-magnier-7s]

I've poked my team in regards to the CLA, that's going to take some people having a word internally, hopefully not too much time, I'll follow-up Monday, it's Friday near midnight right now 😅

Would you maybe like to have a change of title/commit text? Because reading your comment, it feels like you object to the wording a bit.
Maybe "Add metrics_labels to make_mutable_config to introduce the value in 3.13 migrated queues, in line with 4.2 queues".

If that works for you I'll change those Monday, I can't be trusted with a computer right now (and we're waiting for the CLA anyway!)

---

(Oh no, I just saw your edit after writing all that, that you ended at the same conclusion as me! Leaving it for posterity, if that gets an idea flowing for making the change in seshat instead!)

Alternatively we can tweak prometheus_rabbitmq_raft_metrics_collector:collect_per_object_metrics/2 to inject that key when it is missing.

Then we wouldn't need any ra changes at all.

The way I see it, outside of altering the ETS table before calling seshat:format/2, which feels more costly, unless it's a run-once bit of code (which it can't be, because that bit is not aware of when the table will have reloaded), there's not really a way that is rabbitmq-server only, we'd need to make a change in seshat to accommodate the concept of overriding the LabelsAsMap, as it's pulled directly from ETS. There's that Options, but I don't see how to currently use it for the purpose, and it feels more hacky, as, at this point, we're not even caring about individual queues in prometheus_rabbitmq_raft_metrics_collector:collect_per_object_metrics/2, that's actually seshat that deals with the iterating...

Ok, I see a solution from that that doesn't seem as bad:
What if sestet:format took an additional closure in the options that would let you alter the record on each iteration.
That way, the prom calling code can retain "ownership" of the record, and do any last second fixes.

[elo-magnier-7s]

I've changed the commit & PR titles, with no change in code

We've made a request for the CLA, so that's on the way too!

[michaelklishin]

@elo-magnier-7s JFYI, there was some progress on building team consensus around this change.

[michaelklishin]

Now that rabbitmq/ra#651 is in, we need a new ra release to go along with this PR.

[michaelklishin]

Now that https://github.com/rabbitmq/ra/releases/tag/v3.2.0 is out, this PR just needs to wait for a version bump.

[michaelklishin]

Doh #17285.

[michaelklishin]

Given that this bug fix depends on a new ra minor, it will not be backported unless an older ra series backports the change.

But even shipping this fix in 4.4.0 sounds acceptable to me.