spring-projects / spring-boot

Spring Boot
https://spring.io/projects/spring-boot
Apache License 2.0
74.8k stars 40.6k forks source link

Allow customizing rabbitmq metrics prefix #29549

Open FalconerTC opened 2 years ago

FalconerTC commented 2 years ago

Metrics for rabbitmq are bound using Micrometer to the prefix rabbitmq (https://github.com/spring-projects/spring-boot/blob/b8983cef590686a3b1d23ffef4113b024a9a13e3/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/amqp/RabbitMetrics.java#L55)

    @Override
    public void bindTo(MeterRegistry registry) {
        this.connectionFactory.setMetricsCollector(new MicrometerMetricsCollector(registry, "rabbitmq", this.tags));
    }

This is the same domain as rabbitmq server metrics and it can be confusing to pick these apart (I've seen recommendations for scoping them to rabbitmq.client). It would be nice to be able to customize this!

mbhave commented 2 years ago

Flagging for team-attention to see if the team has any thoughts around what the property used for configuring the prefix should be.

philwebb commented 2 years ago

I'm not sure if it's best under spring.rabbit or management.metrics. We could have spring.rabbitmq.metrics.micrometer.prefix but I'd probably lean more towards management.metrics.rabbit.prefix. Especially as we have things like management.metrics.mongo.command.enabled under management.metrics already.

mbhave commented 2 years ago

+1 for nesting under management.metrics since it depends on Micrometer.

snicoll commented 2 years ago

I don't think this should be a property at all. I don't think any of our metrics auto-configuration has a customizable prefix (from a quick search in the appendix for .prefix).

onobc commented 2 years ago

A workaround may be to create a MeterFilter that renames to include the prefix something like what is done for Prometheus.

wilkinsona commented 2 years ago

While I don't think we have any properties for configuring a prefix, we do allow complete customization of some metrics names:

It seems a little inconsistent to allow complete configuration of some metrics names while not allowing the prefix used by Rabbit metrics to be customised.

Having said that, if we allowed the Rabbit metrics prefix to be customised, it would then become inconsistent with things like Tomcat's metrics and Jetty thread pool metrics as io.micrometer.core.instrument.binder.tomcat.TomcatMetrics and io.micrometer.core.instrument.binder.jetty.JettyServerThreadPoolMetrics hardcode the metric names.

I agree that the current prefix isn't ideal and it would be good to change it, however it's not clear how best to do that. I don't think we can just change it in Boot, at least not without a way to change it back, as it would be a breaking change in people's dashboards on upgrade. Just adding a property for Rabbit metrics' prefix would seem a bit odd given that other prefixes cannot be configured, however there's currently no easy way to change the prefix programatically – you would need to exclude RabbitMetricsAutoConfiguration, copy-paste RabbitConnectionFactoryMetricsPostProcessor which is currently package-private, and create your own RabbitMetrics that uses a different prefix.