Make sure MeterBinder and CompositeMeterRegistry registration behavior is consistent

[jonatan-ivanov]

First of all, maybe this is intentional and Boot should do what it does today but maybe the behavior of registering MeterBinder and CompositeMeterRegistry have some inconsistencies (maybe MeterFilter too).

In the latest GA version of Boot (3.3.3), if I create a custom registry bean, let's say a SimpleMeterRegistry in addition to what Boot creates, (let's say PrometheusMeterRegistry), binders are bound once, and the registry that will be injected if I ask for a MeterRegistry is AutoConfiguredCompositeMeterRegistry (what binders bound to).

@Bean
SimpleMeterRegistry simpleMeterRegistry() {
    return new SimpleMeterRegistry();
}

@Bean
MeterBinder debuggingHelperBinder() {
	return registry -> {
		System.out.println("Binder bound to " + registry);
		registry.counter("test").increment();
	};
}

This behavior looks expected to me.

If I also create a CompositeMeterRegistry on top of the previous scenario (so I will have 1. Composite, 2. Simple, 3. Prometheus) Boot will create an AutoConfiguredCompositeMeterRegistry where all three registries are registered (expected) but it will also bind every binders to two registries: to AutoConfiguredCompositeMeterRegistry (expected) and to the CompositeMeterRegistry that I created (unexpected?).

@Bean
SimpleMeterRegistry simpleMeterRegistry() {
	return new SimpleMeterRegistry();
}

@Bean
CompositeMeterRegistry compositeMR() {
	CompositeMeterRegistry compositeMeterRegistry = new CompositeMeterRegistry();
	compositeMeterRegistry.add(new SimpleMeterRegistry());
	return compositeMeterRegistry;
}

@Bean
MeterBinder debuggingHelperBinder() {
	return registry -> {
		System.out.println("Binder bound to " + registry);
		registry.counter("test").increment();
	};
}

This will result in the following structure:

AutoConfiguredCompositeMeterRegistry
    PrometheusMeterRegistry
    SimpleMeterRegistry
    CompositeMeterRegistry
        SimpleMeterRegistry

Should there be two composites or should Boot back off and use the user-created composite to register other registries? If so, should binders be bound to all the composites?

[wilkinsona]

Should there be two composites or should Boot back off and use the user-created composite to register other registries?

I think there should be two composites. Boot using the user-created composite wouldn't really be in keeping with the auto-configuration approach as we'd be changing the user's configuration, leaving them with something that they may not want. For example, in your example above, compositeMR would end up containing two SimpleMeterRegistry instances: one that the compositeMR bean method adds itself and the Boot-added instance from the simpleMeterRegistry bean.

If so, should binders be bound to all the composites?

I think what we're missing is some logic in Boot to avoid binding meters to a composite that's part of another composite. That missing logic results in double binding which we should probably avoid, for efficiency if nothing else.

WDYT, @jonatan-ivanov? Does that make sense?

[wilkinsona]

This branch contains an implementation of the fix that I've tried to describe above.

[wilkinsona]

I've since realised that this fix won't always work as it relies on the meter registries being processed in a particular order so that a nested composite is seen after the composite that contains it. I'm not yet sure what we can do about that.

[jonatan-ivanov]

leaving them with something that they may not want.

what we're missing is some logic in Boot to avoid binding meters to a composite that's part of another composite

All of this makes sense, thank you!

I'm not sure what to do with the ordering issue but I think your changes could make things better even if it won't work in all cases.
I'm also thinking how unexpected would be excluding user-created composites from the processor?
So Boot would configure binders/filters/etc. for AutoConfiguredCompositeMeterRegistry but not for user-created composite or non-composite registries because those would be a direct or transitive children of AutoConfiguredCompositeMeterRegistry.

[wilkinsona]

Excluding user-created composites has the potential to be a breaking change. They may have defined a single MeterRegistry bean that's a composite. Stopping binding to it would mean they lose all their metrics. I think it's better to risk binding twice (when there's a primary user-defined composite bean that contains a registry that itself is also a bean) than to not bind at all.

One change that I think we can make is that, when there's an AutoConfiguredCompositeMeterRegistry bean, we bind to it and nothing else. The auto-configured composite contains all other MeterRegistry beans. Therefore, if we bind to the auto-configured composite, we should not bind to any other meter registry beans otherwise they will be bound twice. I've opened #42396 to do that.

I'll leave this open, for now at least, until we decide what to do about the duplicate binding that remains.

[wilkinsona]

We discussed this one today and decided that we're going to leave things as they are for now. We can look at this again if it's still causing problems after the improvements made in #42396.