Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider providing a Prometheus variant #419

Closed
michaelklishin opened this issue Jun 22, 2020 · 4 comments · Fixed by #422
Closed

Consider providing a Prometheus variant #419

michaelklishin opened this issue Jun 22, 2020 · 4 comments · Fixed by #422
Labels
Request Request for image modification or feature

Comments

@michaelklishin
Copy link
Contributor

This plugin has a variant/tag where it pre-enables the management plugin. Should we add another one that enables RabbitMQ Prometheus plugin instead? Or would it be too much? (we definitely don't want to have a tag per plugin but these two monitoring-related ones are essential)

Enabling both makes sense in some cases and not so much in others. We can assume that Prometheus users would not want to use rabbitmq-management for monitoring.

@gerhard
Copy link
Contributor

gerhard commented Jun 22, 2020

I would like to see rabbitmq-prometheus be enabled by default. Because it currently depends on rabbitmq-management, only the management variant can have it enabled by default.

rabbitmq-prometheus overhead is minimal - think /proc subsystem - and metrics are aggregated by default, so no sensitive information will be exposed if this is enabled. A few considerations:

  • GET /metrics don't require authentication
  • SSL will need to be configured for rabbitmq-prometheus on top of rabbitmq-management
  • metrics in rabbitmq-management can be disabled and overhead can be significantly reduced if only rabbitmq-prometheus is used for monitoring

@wglambert wglambert added the Request Request for image modification or feature label Jun 22, 2020
@tianon
Copy link
Member

tianon commented Jun 22, 2020

I would like to see rabbitmq-prometheus be enabled by default. Because it currently depends on rabbitmq-management, only the management variant can have it enabled by default.

It only requires rabbitmq_management_agent, right? Not the full management plugin?

rabbitmq-prometheus overhead is minimal - think /proc subsystem - and metrics are aggregated by default, so no sensitive information will be exposed if this is enabled.

I think this sounds pretty reasonable.

Here's what I'd propose:

  1. we enable rabbitmq_prometheus in the default variant (which will pull in rabbitmq_management_agent)
  2. we include management_agent.disable_metrics_collector = true in the default config
  3. in the management variant, we delete the aforementioned default config addition
  4. we update docker-entrypoint.sh to handle SSL/TLS for rabbitmq_prometheus just like it does for management and server

My only real concern is the effect this might have on users who aren't using the management variant, but are enabling rabbitmq_management_agent themselves, and if they update their metrics could unexpectedly disappear, and this is what makes me think perhaps a variant for 3.8 makes sense, and we consider enabling by default for the next major release? (Perhaps also coupled with dropping the management variant altogether?)

@gerhard
Copy link
Contributor

gerhard commented Jun 24, 2020

It only requires rabbitmq_management_agent, right? Not the full management plugin?

Yes, you are right, I confused it with a past behaviour that has now been addressed 😉

  1. we enable rabbitmq_prometheus in the default variant (which will pull in rabbitmq_management_agent)
  2. we include management_agent.disable_metrics_collector = true in the default config
  3. in the management variant, we delete the aforementioned default config addition
  4. we update docker-entrypoint.sh to handle SSL/TLS for rabbitmq_prometheus just like it does for management and server

Yes, this is sensible. Port 15692 (+SSL/TLS) will also need to be exposed.

My only real concern is the effect this might have on users who aren't using the management variant, but are enabling rabbitmq_management_agent themselves, and if they update their metrics could unexpectedly disappear

I can't think of a use-case for enabling rabbitmq_management_agent outside of rabbitmq_management. Can you think of one @michaelklishin?

we consider enabling by default for the next major release?

What major release are you thinking about? If you mean 3.9.x, I don't think that we should wait for this to ship before enabling rabbitmq_prometheus by default.

Perhaps also coupled with dropping the management variant altogether?

I think that this is too useful to drop. Having a UI is valuable to many, especially as many messaging-related operations can be performed from the UI, without requiring a client. I would be more comfortable having Prometheus metrics enabled by default in both rabbitmq & -management variants, rather than introducing a new Prometheus variant.

@tianon
Copy link
Member

tianon commented Jun 25, 2020

Ok, that all makes sense! I've started a first pass implementing my proposal in #422 which could use some review. 😄

openstack-mirroring pushed a commit to openstack/openstack that referenced this issue Dec 18, 2023
* Update openstack-helm-infra from branch 'master'
  to 5cbce03f21cea3e6d1041afb6739280fb57e29f4
  - Enable management api metrics collection
    
    The default rabbitmq image disables metrics collection via the management
    api. This is implemented by adding a file named:
    
    /etc/rabbitmq/conf.d/management_agent.disable_metrics_collector.conf
    
    with the contents:
    
    management_agent.disable_metrics_collector = true
    
    The prometheus exporter currently used by osh requires this value to be
    false.
    
    This change was introduced when rabbit introduced the integrated
    prometheus exporter:
    
    docker-library/rabbitmq#419
    
    Change-Id: I9a94f49a7827bb4725ed3fd98404e637bfefa086
openstack-mirroring pushed a commit to openstack/openstack-helm-infra that referenced this issue Dec 18, 2023
The default rabbitmq image disables metrics collection via the management
api. This is implemented by adding a file named:

/etc/rabbitmq/conf.d/management_agent.disable_metrics_collector.conf

with the contents:

management_agent.disable_metrics_collector = true

The prometheus exporter currently used by osh requires this value to be
false.

This change was introduced when rabbit introduced the integrated
prometheus exporter:

docker-library/rabbitmq#419

Change-Id: I9a94f49a7827bb4725ed3fd98404e637bfefa086
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Request Request for image modification or feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants