[Suggestion] Modify rabbit_plugins to honor optional_applications, enabling companion plugins to be merged into their parents
#15979
Unanswered
lukebakken
asked this question in
Ideas
Replies: 2 comments 1 reply
-
|
@lukebakken thank you. The Broadcom side of the core team has some grievances with the current plugin system as well. We need to try to get together on video to design something that'd hopefully give everyone what they find missing. |
Beta Was this translation helpful? Give feedback.
1 reply
-
|
Makes sense. I don't know how well |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
-
RabbitMQ series
main (4.4.x)
Operating system (distribution) used
N/A
How is RabbitMQ deployed?
N/A
What would you like to suggest for a future version of RabbitMQ?
Background
The RabbitMQ plugin system currently has no concept of optional dependencies. When a plugin lists another plugin in its
applicationskey (viaDEPSin erlang.mk), the plugin system implicitly enables that dependency whenever the declaring plugin is enabled. This forces the split-plugin pattern:rabbitmq_federation_management,rabbitmq_shovel_management,rabbitmq_stream_management,rabbitmq_federation_prometheus, andrabbitmq_shovel_prometheusall exist as separate plugins solely because there is no way for their parent plugin to depend onrabbitmq_managementorrabbitmq_prometheuswithout unconditionally enabling them.The gap
OTP 24 introduced
optional_applicationsin.appfiles. erlang.mk already supports this viaOPTIONAL_DEPS, which places apps in bothapplicationsandoptional_applications. OTP'sapplication_controllerhonorsoptional_applicationsat startup - it does not throw{not_started, App}for apps listed there.However,
rabbit_plugins:mkplugin/4reads onlyapplicationswhen building#plugin.dependencies:It ignores
optional_applicationsentirely, so the plugin system still treats optional deps as mandatory for implicit enablement.Proposed change
optional_dependenciesfield to#plugin{}inrabbit_common/include/rabbit.hrlrabbit_plugins:mkplugin/4, readoptional_applicationsfrom the.appfile and subtract those apps fromdependencies:This is sufficient -
dependencies/3builds its graph from#plugin.dependenciesonly, and therabbitmq-pluginsCLI callsdependencies/3directly, so both implicit enablement and the CLI's enable/disable/list commands automatically reflect the correct behavior with no further changes.The interaction with
extra_dependenciesandis_strictly_plugin/1is safe.ensure_dependencies/1moves any app for whichis_loadable/1returnstruefromdependenciesintoextra_dependencies.is_loadable/1succeeds for any app whose ebin is on the code path - this includes OTP/stdlib apps, RabbitMQ core apps likerabbitandrabbit_common, and any plugin present in the plugins directory.is_strictly_plugin/1checks whetherrabbitis inextra_dependencies, which it always will be for a real plugin.The proposed change subtracts
optional_applicationsfromdependenciesinmkplugin/4, beforeensure_dependencies/1runs. So optional deps never reachensure_dependencies/1at all - the interaction is fully safe regardless of whetheris_loadable/1would returntrueorfalsefor them.Opt-out configuration
Merging companion plugins into their parents raises a question: what if a user wants
rabbitmq_shovelandrabbitmq_managementrunning, but does not want the shovel management UI extension? Today that is possible by simply not enablingrabbitmq_shovel_management.To preserve this control, each plugin would expose a cuttlefish setting that defaults to
true(opt-out model). For example:The guard in each plugin's app module would check both whether the optional application is running and whether the setting is enabled:
However, the management dispatcher also re-scans all running plugins at runtime when
rabbitmq-plugins enable/disableis called on a live broker. It does this viarabbit_mgmt_reset_handler, which responds toplugins_changedevents and callsrabbit_mgmt_app:reset_dispatcher/1. The dispatcher discovers extensions by finding all modules implementing therabbit_mgmt_extensionbehaviour across all loaded RabbitMQ-related apps, then calling theirdispatcher/0callback. This means the opt-out guard must also live indispatcher/0, not only at startup:Note:
rabbit_mgmt_reset_handleronly triggersreset_dispatcherwhen a management extension changes (not whenrabbitmq_managementitself starts or stops - in that case the dispatcher is freshly created or destroyed). Whenrabbitmq_managementstarts, it callsstart_configured_listeners([], true)which scans all already-loaded apps includingrabbitmq_shovel, so thedispatcher/0guard is respected in that path too.This ensures the opt-out is respected both at startup and when plugins are enabled or disabled at runtime.
The default of
truemeans existing users who upgrade get the same behaviour as today (management and/or prometheus extension active when both plugins are enabled), with no configuration change required. Users who want to opt out set the flag tofalse.Impact: companion plugins that could be merged
With this change, the following thin plugins could be merged into their parents:
rabbitmq_federation_prometheusrabbitmq_federationrabbitmq_shovel_prometheusrabbitmq_shovelrabbitmq_federation_managementrabbitmq_federationrabbitmq_shovel_managementrabbitmq_shovelrabbitmq_stream_managementrabbitmq_streamEach merged plugin would guard its optional functionality on whether the relevant application (
prometheusorrabbitmq_management) is running.Backwards compatibility
When a companion plugin is merged and its name disappears, users with the old name in
enabled_pluginswill not experience a startup failure.prepare_plugins/1builds its graph only from plugins present in the plugins directory - unknown names are silently skipped. Release notes should document the change so users can clean up theirenabled_pluginsfiles.Test coverage
Neither existing plugin test suite (
unit_plugin_directories_SUITE,unit_plugin_versioning_SUITE) coversdependencies/3,mkplugin/4, orensure_dependencies/1. New unit tests would be required, covering: optional deps excluded from implicit enablement, missing optional deps not causing startup errors, mandatory deps still enforced, and the upgrade scenario where a stale companion plugin name is present inenabled_plugins.If this sounds like a useful contribution I'll whip up a PR, thanks.
Beta Was this translation helpful? Give feedback.
All reactions