Skip to content

Conversation

@the-mikedavis
Copy link
Collaborator

Proposed Changes

This covers the optional part of #13782: updating GET /api/health/checks/protocol-listener/{protocol} to accept multiple protocols, comma-separated. The check only returns 200 OK when all of the request protocols have listeners available.

Connects #13782.

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 #NNNN)
  • 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

NOTE: the field in the error response missing is updated to a list. I'm not sure if we should consider this as a breaking change or not.

Checklist

Put an x in the boxes that apply.
You can also fill these out after creating the PR.
If you're unsure about any of them, don't hesitate to ask on the mailing list.
We're here to help!
This is simply a reminder of what we are going to look for before merging your code.

  • I have read the CONTRIBUTING.md document
  • I have signed the CA (see https://cla.pivotal.io/sign/rabbitmq)
  • 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

This PR also adds a commit which changes the helper used to look for active listeners from rabbit_networking:active_listeners/0 to rabbit_networking:node_listeners(node()) - this eliminates some unnecessary RPC calls. It's unrelated to the feature part of this change.

This is a minor change that avoids a cluster-wide query for active
listeners. The old code called `rabbit_networking:active_listeners/0`
and then filtered the results by ones available on the local node. This
caused an RPC and concatenation of all other cluster members' listeners
and then in the next line filtered down to local nodes. Equivalently we
can use `rabbit_networking:node_listeners(node())` which dumps a local
ETS table.

This is not a very impactful change but it's nice to keep the latency of
the health-check handlers low and reduce some unnecessary cluster noise.
This updates the health check for protocol listeners to accept a set of
protocols, comma-separated. The check only returns 200 OK when all
requested protocols have active listeners.
@the-mikedavis the-mikedavis self-assigned this May 8, 2025
@michaelklishin
Copy link
Collaborator

@the-mikedavis thank you, I'm on it.

FYI, the HTTP API reference was moved to the website. It's fine to update the bits in this repo, of course, but the plan is to eventually only have a website link.

@michaelklishin
Copy link
Collaborator

michaelklishin commented May 8, 2025

rabbitmqadmin should be covered by this enum variant, not sure if it's worth the time introducing another one for multiple protocols. Probably not.

@the-mikedavis
Copy link
Collaborator Author

the-mikedavis commented May 8, 2025

Ah I see that in the website repo now. I'll add the updates for this and #13872 (and the future health-check PRs) in some PRs to the website repo too 👍

@michaelklishin michaelklishin merged commit 98ba31a into main May 8, 2025
269 checks passed
@michaelklishin michaelklishin deleted the md/health-check-multi-protocol-listener branch May 8, 2025 16:22
michaelklishin added a commit that referenced this pull request May 8, 2025
Allow multiple protocols in protocol listener health check (backport #13871)
@michaelklishin michaelklishin added this to the 4.2.0 milestone Jun 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants