micronaut-micrometer icon indicating copy to clipboard operation
micronaut-micrometer copied to clipboard

CompositeMeterRegistry configured repeatedly

Open TimothyL96 opened this issue 2 years ago • 0 comments

Issue description

Not sure if it's intended, but the composite registry is repeatedly reconfigured.

In the MeterRegistryFactory:

        for (MeterRegistry registry : registries) {
            compositeMeterRegistry.add(registry);
            for (MeterRegistryConfigurer<MeterRegistry> configurer : configurers) {
                // Let configurers configure the composite registry
                if (configurer.getType().isAssignableFrom(CompositeMeterRegistry.class) && configurer.supports(compositeMeterRegistry)) {
                    configurer.configure(compositeMeterRegistry);
                }
                
                //Let configurers configure individual registries
                if (configurer.getType().isAssignableFrom(registry.getClass()) && configurer.supports(registry)) {
                    configurer.configure(registry);
                }
            }
        }

Assuming we only have the default CompositeMeterRegistryConfigurer, and have multiple registries like NewRelic and Prometheus, iterating them in the outer loop above will also call the default composite configurer in every iteration.

The configurer basically set the filters and binders, I'm not sure why not to configure it after all registries is added to the composite registry.

I encountered this when trying to manually configure Hikari CP metrics with their setMetricsRegistry(), and setting it more than once will cause exception.

So with a custom meter binder that register Hikari metrics, the custom binder will be called multiple times. Workaround will be to check if Hikari registry is already configured before calling setMetricsRegistry().

If there's no reason to configure it repeatedly (to prevent double loop on configurer?), perhaps moving the composite configurer loop out will work

        for (MeterRegistry registry : registries) {
            compositeMeterRegistry.add(registry);
            for (MeterRegistryConfigurer<MeterRegistry> configurer : configurers) {
                //Let configurers configure individual registries
                if (configurer.getType().isAssignableFrom(registry.getClass()) && configurer.supports(registry)) {
                    configurer.configure(registry);
                }
            }
        }

        for (MeterRegistryConfigurer<MeterRegistry> configurer : configurers) {
            // Let configurers configure the composite registry
            if (configurer.getType().isAssignableFrom(CompositeMeterRegistry.class) && configurer.supports(compositeMeterRegistry)) {
                configurer.configure(compositeMeterRegistry);
            }
        }

TimothyL96 avatar Aug 19 '22 20:08 TimothyL96