MeterBinder beans that directly or indirectly depend on MeterRegistry beans cause a cycle

[deripas]

Hi, after upgrading Spring Boot from 2.1 to 2.6, I have a problem (project sample)

The dependencies of some of the beans in the application context form a cycle:

┌─────┐
|  service defined in com.example.sample.SampleApplication
↑     ↓
|  rabbitTemplate defined in class path resource [org/springframework/boot/autoconfigure/amqp/RabbitAutoConfiguration$RabbitTemplateConfiguration.class]
↑     ↓
|  rabbitConnectionFactory defined in class path resource [org/springframework/boot/autoconfigure/amqp/RabbitAutoConfiguration$RabbitConnectionFactoryCreator.class]
↑     ↓
|  simpleMeterRegistry defined in class path resource [org/springframework/boot/actuate/autoconfigure/metrics/export/simple/SimpleMetricsExportAutoConfiguration.class]
└─────┘

Spring Boot version: 2.6.6

Spring dependency:

• spring-boot-starter-actuator
• spring-boot-starter-amqp

Problem
I can't use MeterBinder in a service that is injected with something that depends on the ConnectionFactory.
Example of a problematic service:

@RequiredArgsConstructor
public class MyService implements MeterBinder {
    private final RabbitTemplate sender;

    @Override
    public void bindTo(MeterRegistry meterRegistry) {
        // some service metric
    }
}

Workaround
As a workaround, you can use the annotation @Lazy:

    @Bean
    public MyService service(@Lazy RabbitTemplate template) {
        return new MyService(template);
    }

But it doesn't seem to me that main stream.

[wilkinsona]

There has always been a cycle if you inject a RabbitTemplate into a MeterBinder. What has changed is that cycles are now prohibited by default. I would recommend restructuring your application so that your MeterBinder implementation is a separate component.

[deripas]

I understand that the dependency was earlier.

I would recommend restructuring your application so that your MeterBinder implementation is a separate component.

Сan you give an example of how to restructure?
The implementation of MeterBinder can be separate, but if it will inject something dependent (transitively) from the ConnectionFactory, this will result in an error.

I see the problem in the fact that RabbitConnectionFactoryMetricsPostProcessor calls getMeterRegistry()which leads to the construction of the MeterRegistry bean:

class RabbitConnectionFactoryMetricsPostProcessor implements BeanPostProcessor, Ordered {

	@Override
	public Object postProcessAfterInitialization(Object bean, String beanName) {
		if (bean instanceof AbstractConnectionFactory) {
			bindConnectionFactoryToRegistry(getMeterRegistry(), beanName, (AbstractConnectionFactory) bean);
		}
		return bean;
	}

	private MeterRegistry getMeterRegistry() {
		if (this.meterRegistry == null) {
			this.meterRegistry = this.context.getBean(MeterRegistry.class);
		}
		return this.meterRegistry;
	}

this leads to a call to the MeterRegistryPostProcessor and calling the method configure:

class MeterRegistryPostProcessor implements BeanPostProcessor {

	@Override
	public Object postProcessAfterInitialization(Object bean, String beanName) throws BeansException {
		if (bean instanceof MeterRegistry) {
			getConfigurer().configure((MeterRegistry) bean);
		}
		return bean;
	}

and in this method there is an appeal to MeterBinder:

	void configure(MeterRegistry registry) {
...
		if (!this.hasCompositeMeterRegistry || registry instanceof CompositeMeterRegistry) {
			addBinders(registry); // <--- throw here
		}
	}

And then an error occurs, MeterBinder cannot be built because the initialization of the ConnectionFactory has not yet been completed.

This makes it impossible to use MeterBinder and ConnectionFactorytogether.

[wilkinsona]

Unfortunately, I don't think there's anything we can do about that without regressing #12855. Why does your MeterBinder need the ConnectionFactory or something that depends on the ConnectionFactory?

[deripas]

We have a lot of services that use MeterBinder to register metric for monitoring status, performance. One of the services sends messages/notifications using a RabbitTemplate-based client.
When updating the springboot version, we encountered an unexpected problem.

In such an application of MeterBinder and a RabbitTemplate-based client, I do not see anything illegal.
Otherwise, it should be specified in the documentation.

[deripas]

A small comment about the problem #12855.

Here is the example. Hope that it helps.

It seems to me that the example is not quite correct.
At the stage of building the Spring context, there is an appeal to RabbitMQ, it seems to me that this should be done at another stage of the lifecycle.

    @Bean
    Exchange configure(RabbitAdmin rabbitAdmin) {
        Exchange topicExchange = ExchangeBuilder.topicExchange(EXCHANGE_NAME).build();
        rabbitAdmin.declareExchange(topicExchange);
        Queue queue = QueueBuilder.durable(QUEUE_NAME).build();
        rabbitAdmin.declareQueue(queue);
        rabbitAdmin.declareBinding(BindingBuilder.bind(queue).to(topicExchange).with(ROUTING_KEY).noargs());
        return topicExchange;
    }

RabbitMQ initialization can be redone according to the documentation Spring AMQP:

    @Bean
    Exchange topicExchange() {
        return ExchangeBuilder.topicExchange(EXCHANGE_NAME).build();
    }

    @Bean
    Queue queue() {
        return QueueBuilder.durable(QUEUE_NAME).build();
    }

    @Bean
    Binding binding(Queue queue, Exchange topicExchange) {
        return BindingBuilder.bind(queue).to(topicExchange).with(ROUTING_KEY).noargs();
    }

In this case, the example will work correctly:

2022-04-12 17:35:21.332  INFO 55255 --- [           main] com.example.sample.SampleApplication     : Expected 5 messages. Checking metric registry...
2022-04-12 17:35:21.334  INFO 55255 --- [           main] com.example.sample.SampleApplication     : Counter returns: 5.0

If incorrect usage is not encouraged, then RabbitConnectionFactoryMetricsPostProcessor can be replaced with a simple construction of the form:

@AllArgsConstructor
public class RabbitMetricsInitializing implements MeterBinder {

    private final Map<String, ? extends AbstractConnectionFactory> factories;

    @Override
    public void bindTo(MeterRegistry registry) {
        factories.forEach((name, factory) -> bindConnectionFactoryToRegistry(registry, name, factory));
    }
}

Thank you for your attention, I will no longer distract you with my reasoning.

[wilkinsona]

We may be able to improve the situation by using MicrometerMetricsCollector(Function<Metrics, Object> metricsCreator).

[philwebb]

It turns out that we can't use MicrometerMetricsCollector(Function<Metrics, Object> metricsCreator) after all since the function is immediately called by the constructor.

Stepping back a bit, the issue is probably broader than Rabbit and will apply to any MeterBinder bean that directly or indirectly depends on a MeterRegistry bean. I think it might be possible to update MeterRegistryPostProcessor so that MeterBinder beans are setup when the context is refreshed, rather than when the bean is created.

I've pushed some prototype code here but it could do with a review.

[wilkinsona]

I'm a bit concerned that the prototype may swing things too far in the other direction. With the proposed changes in place, I think I'd find it hard to explain why we have support for MeterBinder and when you'd want to implement it. Wouldn't you only want to use it to break a dependency cycle? If so, @Lazy feels like a better way to express that. For the majority of cases, it feels like you'd be better just injecting MeterRegistry and binding the metrics directly rather than implementing MeterBinder and only having them bound once refresh has completed.

The more I think about this, the more it reminds me of the problems we had with DataSource initialization. It feels like another use case for a callback in Framework after a bean has been created but before it's injected any where. I opened spring-projects/spring-framework#21362 for that when we were looking at DataSource initialization but we found a different approach in the end.

[ailjushkin]

@wilkinsona Hi, I'm also faced with the problem while dealing with rabbitmq metrics, could you please explain how to work around this problem?

Running with Spring Boot v2.7.1, Spring v5.3.21

[C:\work\vcs\xxx\xxx-backend-webapp\target\classes\com\xxx\xxx\billing\openapi\notifications\rabbit\RabbitConnectionFactoryConfiguration.class]: Unsatisfied dependency expressed through constructor parameter 7; nested exception is org.springframework.beans.factory.BeanCurrentlyInCreationException: Error creating bean with name 'prometheusMeterRegistry': Requested bean is currently in creation: Is there an unresolvable circular reference?

Description:

The dependencies of some of the beans in the application context form a cycle:

   webMvcMetricsFilter defined in class path resource [org/springframework/boot/actuate/autoconfigure/metrics/web/servlet/WebMvcMetricsAutoConfiguration.class]
┌─────┐
|  prometheusMeterRegistry defined in class path resource [org/springframework/boot/actuate/autoconfigure/metrics/export/prometheus/PrometheusMetricsExportAutoConfiguration.class]
↑     ↓
|  commonMetricsConfiguration defined in file [C:\work\vcs\xxx\xxx-backend-webapp\target\classes\com\xxx\xxx\billing\openapi\app\configs\metrics\CommonMetricsConfiguration.class]
↑     ↓
|  componentStatusService defined in class path resource [com/peterservice/bssbox/common/autoconfigure/HealthCheckAutoConfiguration.class]
↑     ↓
|  rabbitHealthCheck defined in class path resource [com/peterservice/bssbox/common/autoconfigure/RabbitHealthCheckAutoConfiguration.class]
↑     ↓
|  rabbitTemplate defined in class path resource [org/springframework/boot/autoconfigure/amqp/RabbitAutoConfiguration$RabbitTemplateConfiguration.class]
↑     ↓
|  rabbitConnectionFactoryConfiguration defined in file [C:\work\vcs\xxx\xxx-backend-webapp\target\classes\com\xxx\xxx\billing\openapi\notifications\rabbit\RabbitConnectionFactoryConfiguration.class]
└─────┘

MeterRegistry bean is autowired into my configuration to bind meter registry into rabbitmq connectionfactory

private void bindMetrics(CachingConnectionFactory connectionFactory) {
        connectionFactory.getRabbitConnectionFactory().setMetricsCollector(
                new MicrometerMetricsCollector(
                        meterRegistry,
                        METRIC_NAME_PREFIX + "rabbitmq",
                        Tags.of(TAG_VIRTUAL_HOST, connectionFactory.getVirtualHost(),
                                TAG_CONNECTION_STRING, rabbitProperties.determineHost() + ":" + rabbitProperties.determinePort()))
        );
    }

[wilkinsona]

Boot will automatically configure metrics for the Rabbit connection factory of each AbstractConnectionFactory in the context. This is done through RabbitMetricsAutoConfiguration and RabbitConnectionFactoryMetricsPostProcessor. If you didn't have a cycle I think you'd end up with two lots of metrics for each connection factory and I suspect that isn't what you want. You may be able to work around this with an auto-configuration exclude for org.springframework.boot.actuate.autoconfigure.metrics.amqp.RabbitMetricsAutoConfiguration.

[ailjushkin]

@wilkinsona I've added an exclude in the application class, but the problem is still there. Maybe we had the cycle before, because I've got this error after I've upgraded from spring boot 2.3 to 2.7 where as you say any cyclic dependencies are prohibited by default.

[wilkinsona]

Without the post-processor in place, I'm struggling to think of what could be creating the cycle. Can you share some code that reproduces the problem?

[wilkinsona]

Ah, I'd missed that you weren't already using a MeterBinder implementation. Thanks for letting us know it's addressed your problem.

[ailjushkin]

@wilkinsona Actually, MeterBinder didn't work here, but my metrics were shown up on /metrics when I created a customizer bean

    @Bean
    public MeterRegistryCustomizer<MeterRegistry> billingRabbitmqMeterBinder(@Qualifier(value = "objectConnectionFactoryMap") Map<Object, ConnectionFactory> objectConnectionFactoryMap) {
        return meterRegistry -> StreamEx.of(objectConnectionFactoryMap.values())
                .map(CachingConnectionFactory.class::cast)
                .forEach(connectionFactory -> connectionFactory.getRabbitConnectionFactory().setMetricsCollector(
                        new MicrometerMetricsCollector(meterRegistry, METRIC_NAME_PREFIX + "rabbitmq",
                                Tags.of(
                                        TAG_VIRTUAL_HOST,
                                        connectionFactory.getVirtualHost(),
                                        TAG_CONNECTION_STRING,
                                        rabbitProperties.determineHost() + ":" + rabbitProperties.determinePort())
                        )
                ));
    }

Could you explain in two words, when MeterBinder is needed?

[wilkinsona]

Could you explain in two words?

Maybe 200…

Actually, MeterBinder didn't work here

In what way didn't it work? Same problem with a cycle or something else?

[ailjushkin]

In what way didn't it work? Same problem with a cycle or something else?

Sorry, man. Meter binder was autowired, but when I call /metrics, there are no any rabbit mq metrics.

Method bindTo of a meter binder wasn't executed.

[k-tomaszewski]

I was hit by the very similar problem while using Spring Boot 2.6.12 with Spring Cloud 2021.0.3. I was able to narrow down the cyclic dependency problem to occur only with configuration classes from Spring Boot ecosystem:

...
      ↓
   healthEndpointWebFluxHandlerMapping defined in class path resource [org/springframework/boot/actuate/autoconfigure/health/HealthEndpointReactiveWebExtensionConfiguration$WebFluxAdditionalHealthEndpointPathsConfiguration.class]
      ↓
   healthEndpoint defined in class path resource [org/springframework/boot/actuate/autoconfigure/health/HealthEndpointConfiguration.class]
┌─────┐
|  healthContributorRegistry defined in class path resource [org/springframework/boot/actuate/autoconfigure/health/HealthEndpointConfiguration.class]
↑     ↓
|  rabbitHealthContributor defined in class path resource [org/springframework/boot/actuate/autoconfigure/amqp/RabbitHealthContributorAutoConfiguration.class]
↑     ↓
|  rabbitTemplate defined in class path resource [org/springframework/boot/autoconfigure/amqp/RabbitAutoConfiguration$RabbitTemplateConfiguration.class]
↑     ↓
|  rabbitConnectionFactory defined in class path resource [org/springframework/boot/autoconfigure/amqp/RabbitAutoConfiguration$RabbitConnectionFactoryCreator.class]
↑     ↓
|  simpleMeterRegistry defined in class path resource [org/springframework/boot/actuate/autoconfigure/metrics/export/simple/SimpleMetricsExportAutoConfiguration.class]
└─────┘

What helped in my case was resigning from RabbitMQ health indicator by setting following property:

management.health.rabbit.enabled=false

This basically removes rabbitHealthContributor bean.

Maybe it will help the others.

[wilkinsona]

I'm a bit concerned that the prototype may swing things too far in the other direction. With the proposed changes in place, I think I'd find it hard to explain why we have support for MeterBinder and when you'd want to implement it.

I think #33070 has addressed this concern. Implementing MeterBinder and their meter binding then being deferred until afterSingletonsInstantiated eliminates a potential source of deadlock during context refresh processing. It also means that meters are bound at a defined point in the application's lifecycle, rather than this binding happening at a rather loosely defined point that will vary depending on bean creation ordered.

We're going to try deferring the meter binding in 3.0.

[Bennett-Lynch]

@wilkinsona For those not able to upgrade to 3.x yet, can you please advise how to workaround this issue in 2.7?

I'm encountering this issue with a setup like so:

@Bean
fun httpClient(
    meterRegistry: MeterRegistry,
): OkHttpClient {
    val metricsListener = OkHttpMetricsEventListener.builder(meterRegistry, "okhttp")
        .build()
    return OkHttpClient.Builder()
        .eventListener(metricsListener)
        .build()
}

@Bean
fun connectionPoolMetrics(
    httpClient: OkHttpClient,
): MeterBinder {
    return OkHttpConnectionPoolMetrics(httpClient.connectionPool)
}

Is the guidance to use @Lazy? If so, where should I place it?

[wilkinsona]

@Lazy on the injection of the MeterRegistry into your httpClient method may help, but it's hard to be certain with only code snippets to look at. If it doesn't work and you'd like some more help, please follow up on Stack Overflow with a minimal reproducible example.

[Bennett-Lynch]

That seems to have done the trick. Thank you, @wilkinsona.