SpringBoot Don't apply MeterFilter to auto-configured composite registry

Met with a team today that is configuring two PrometheusMeterRegistry instances, one for platform metrics and the other for application metrics. They expose two different scrape endpoints from the application. The platform metrics should include framework-level metrics like CPU, GC, http request timings, etc. The application metrics endpoint is reserved for teams to record business metrics.

Both of these registries wind up under the CompositeMeterRegistry Spring Boot creates. (I hope you enjoy my senseless ascii art ;) ).

                             Composite
                     ----------/    \
                   /                 \
        Prometheus (platform)     Prometheus (app)

The problem is that PropertiesMeterFilter is added to the composite in addition to both sub-registries. So if an app team adds a property like management.metrics.enable.tomcat=false the composite denies Tomcat metrics before it ever reaches the platform registry.

I suggest we weaken PropertiesMeterFilter and only apply it to sub-registries. In this way, a user-crafted registry could add meter filters that take precedence over PropertiesMeterFilter, like this:

@Bean
PrometheusMeterRegistry platformRegistry() {
    PrometheusMeterRegistry prometheusMeterRegistry = new PrometheusMeterRegistry(PrometheusConfig.DEFAULT);
    prometheusMeterRegistry
            .config()
            .meterFilter(MeterFilter.acceptNameStartsWith("tomcat"))
            .meterFilter(MeterFilter.acceptNameStartsWith("jvm"));
    return prometheusMeterRegistry;
}

cc / @wangyf2010 cc / @shakuzen

Comment From: philwebb

I'm a little worried that MeterRegistryConfigurer can make the assumption that a CompositeMeterRegistry can be skipped because it is only composed of other beans. The user could register their own CompositeMeterRegistry which is composed of instance that we have no visibility on.

Perhaps we need a AutoConfiguredCompositeMeterRegistry class so that we know it's the one that we auto-configured and it only contains beans?

Comment From: jkschneider

I agree @philwebb, I think that makes sense.

Comment From: jkschneider

Thanks for the quick action @wilkinsona. Could you try this out @wangyf2010?