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

CompositeMeterRegistry configured repeatedly #439

Open
TimothyL96 opened this issue Aug 19, 2022 · 0 comments
Open

CompositeMeterRegistry configured repeatedly #439

TimothyL96 opened this issue Aug 19, 2022 · 0 comments

Comments

@TimothyL96
Copy link

TimothyL96 commented Aug 19, 2022

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);
            }
        }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant