Missing Spring Integration metrics due to the MeterRegistry bean being looked for before it has been defined

[artembilan]

Related to spring-projects/spring-integration#3420

After fixing spring-projects/spring-integration#3376
to make sure that Spring Integration has a dependency on the MeterRegistry
for the proper lifecycle on shutdown, it turns out that Spring Integration
metrics features must be configured after the MeterRegistry is
provided properly by Spring Boot

Note: we start observing the wrong order problem when we also have an
IntegrationGraphEndpointAutoConfiguration which is ordered alphabetically
(therefore before MetricsAutoConfiguration) and pushes an
IntegrationAutoConfiguration up - before MetricsAutoConfiguration

• Add @AutoConfigureBefore(IntegrationAutoConfiguration.class)
  to the CompositeMeterRegistryAutoConfiguration to ensure that MeterRegistry
  bean is provided before @EnableIntegrationManagement logic

Back-port to master moving @AutoConfigureBefore(IntegrationAutoConfiguration.class)
onto the MeterRegistryAutoConfiguration already

[spencergibb]

Wouldn't it make sense to flip the dependency so integration does auto configure after metrics?

[artembilan]

I'm not sure what you mean, but I feel like I have done exactly what you just said: the IntegrationAutoConfiguration is going to be auto-configured exactly after metrics.
If you are talking about having an @AutoConfigureAfter on the IntegrationAutoConfiguration instead, then we would introduce a logical dependency between spring-boot-autoconfigure and spring-boot-actuator-autoconfigure modules even if we would use a FQCN for the mentioned CompositeMeterRegistryAutoConfiguration with MeterRegistry definition.

[spencergibb]

Ah, didn't realize the cross module bit

[wilkinsona]

I think we'll need to find another approach if we can. The conceptual relationship is backwards if the general metrics auto-configuration knows about Integration auto-config.

[artembilan]

Yeah... Probably another way is to extract an IntegrationAutoConfiguration.IntegrationManagementConfiguration into a separate class under the spring-boot-actuator-autoconfigure and make it already dependent on the MetricsAutoConfiguration as it is done in many other *MetricsAutoConfiguration, e.g. RabbitMetricsAutoConfiguration or KafkaMetricsAutoConfiguration.

However it is still not clean solution since @EnableIntegrationManagement has its own value even without Micrometer.
On the other hand in Spring Integration 6.0 we are going to get rid off MetricsCaptor facade alltogether to make integration metrics directly based on the Micrometer API. So, our dependency order will become more tighter.
Although that one (I believe) will let us to remove the MicrometerMetricsCaptorRegistrar at all and we won't have that beanFactory.getBeanNamesForType(METER_REGISTRY_CLASS, false, false) logic any more, therefore MeterRegistry detection will be deferred to later phase when we already try to resolve an ObjectProvider<MeterRegistry>.

We probably can trick it exactly on that "guilty" IntegrationGraphEndpointAutoConfiguration which is already here in the spring-boot-actuator-autoconfigure leaving the natural alphabetic order for those IntegrationAutoConfiguration and MetricsAutoConfiguration...

Any other thoughts?

Thank you for looking into this!

[wilkinsona]

The ordering could be changed with an auto-configuration that does nothing other than influencing the ordering. Something like this:

package org.springframework.boot.actuate.autoconfigure.metrics.integration;

import io.micrometer.core.instrument.MeterRegistry;

import org.springframework.boot.actuate.autoconfigure.metrics.CompositeMeterRegistryAutoConfiguration;
import org.springframework.boot.actuate.autoconfigure.metrics.MetricsAutoConfiguration;
import org.springframework.boot.autoconfigure.AutoConfigureAfter;
import org.springframework.boot.autoconfigure.AutoConfigureBefore;
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
import org.springframework.boot.autoconfigure.integration.IntegrationAutoConfiguration;
import org.springframework.context.annotation.Configuration;

/**
 * {@link EnableAutoConfiguration Auto-configuration} for Spring Integration's metrics.
 * Orders auto-configuration classes to ensure that the {@link MeterRegistry} bean has
 * been defined before Spring Integration's Micrometer support queries the bean factory
 * for it.
 *
 * @author Andy Wilkinson
 */
@AutoConfigureAfter({ MetricsAutoConfiguration.class, CompositeMeterRegistryAutoConfiguration.class })
@AutoConfigureBefore(IntegrationAutoConfiguration.class)
@Configuration(proxyBeanMethods = false)
public class IntegrationMetricsAutoConfiguration {

}

This improves the situation but it doesn't fix the test. It fails as the tag hasn't been applied to the spring.integration.channels meter. The tag is missing due to a number of beans not being post-processed:

2020-11-10 10:10:07.483  INFO   --- [           main] trationDelegate$BeanPostProcessorChecker : Bean 'integrationChannelResolver' of type [org.springframework.integration.support.channel.BeanFactoryChannelResolver] is not eligible for getting processed by all BeanPostProcessors (for example: not eligible for auto-proxying)
2020-11-10 10:10:07.484  INFO   --- [           main] trationDelegate$BeanPostProcessorChecker : Bean 'integrationDisposableAutoCreatedBeans' of type [org.springframework.integration.config.annotation.Disposables] is not eligible for getting processed by all BeanPostProcessors (for example: not eligible for auto-proxying)
2020-11-10 10:10:07.497  INFO   --- [           main] trationDelegate$BeanPostProcessorChecker : Bean 'org.springframework.integration.config.IntegrationManagementConfiguration' of type [org.springframework.integration.config.IntegrationManagementConfiguration] is not eligible for getting processed by all BeanPostProcessors (for example: not eligible for auto-proxying)
2020-11-10 10:10:07.521  INFO   --- [           main] trationDelegate$BeanPostProcessorChecker : Bean 'simpleMetricsExportAutoConfiguration' of type [org.springframework.boot.actuate.autoconfigure.metrics.export.simple.SimpleMetricsExportAutoConfiguration] is not eligible for getting processed by all BeanPostProcessors (for example: not eligible for auto-proxying)
2020-11-10 10:10:07.535  INFO   --- [           main] trationDelegate$BeanPostProcessorChecker : Bean 'management.metrics.export.simple-org.springframework.boot.actuate.autoconfigure.metrics.export.simple.SimpleProperties' of type [org.springframework.boot.actuate.autoconfigure.metrics.export.simple.SimpleProperties] is not eligible for getting processed by all BeanPostProcessors (for example: not eligible for auto-proxying)
2020-11-10 10:10:07.537  INFO   --- [           main] trationDelegate$BeanPostProcessorChecker : Bean 'simpleConfig' of type [org.springframework.boot.actuate.autoconfigure.metrics.export.simple.SimplePropertiesConfigAdapter] is not eligible for getting processed by all BeanPostProcessors (for example: not eligible for auto-proxying)
2020-11-10 10:10:07.538  INFO   --- [           main] trationDelegate$BeanPostProcessorChecker : Bean 'metricsAutoConfiguration' of type [org.springframework.boot.actuate.autoconfigure.metrics.MetricsAutoConfiguration] is not eligible for getting processed by all BeanPostProcessors (for example: not eligible for auto-proxying)
2020-11-10 10:10:07.538  INFO   --- [           main] trationDelegate$BeanPostProcessorChecker : Bean 'micrometerClock' of type [io.micrometer.core.instrument.Clock$1] is not eligible for getting processed by all BeanPostProcessors (for example: not eligible for auto-proxying)
2020-11-10 10:10:07.558  INFO   --- [           main] trationDelegate$BeanPostProcessorChecker : Bean 'simpleMeterRegistry' of type [io.micrometer.core.instrument.simple.SimpleMeterRegistry] is not eligible for getting processed by all BeanPostProcessors (for example: not eligible for auto-proxying)

I think this is a bug in Spring Integration. SimpleMeterRegistry isn't eligible for post-processing due to the following dependency chain:

• integrationManagementConfigurer (a BeanPostProcessor)
• managementConfigurer
• integrationMicrometerMetricsCaptor
• simpleMeterRegistry

Generally speaking, injecting dependencies into bean post-processors isn't safe and should be avoided. In this case, it's causing simpleMeterRegistry to be ineligible for post-processing which prevents the meter filter that honours management.metrics.tags from being applied.

[artembilan]

Thank you, @wilkinsona , for looking into this!

So, I think you found the way it is problematic.
After fixing spring-projects/spring-integration#3425 to rely already on the ContextClosedEvent instead of destroy(), it looks like our attempt to have a proper dependency is not going to be satisfied anyway.
Therefore I will change the IntegrationManagementConfigurer to be based on the ObjectProvider<MetricsCaptor> to avoid that hard dependency for its BeanPostProcessor nature.

From here it sounds like we may not need this fix in Spring Boot again...
Stay tuned: I'll update today!

[artembilan]

So, I pushed the fix according our discussion.
And now this depends on the: spring-projects/spring-integration#3427.

We still need to have an auto-configuration order between metrics and Integration since we have a logic in Spring Integration to register or not some bean when MeterRegistry is present in the context or not.
We probably will get rid of it in the next Spring Integration 6.0.

Thanks

[wilkinsona]

I've pushed a squashed and slightly polished version of the proposal here. I'd like the rest of the team to take a look before merging as the new auto-configuration class is one-of-a-kind.

[wilkinsona]

@artembilan I see you've made the Integration change in 5.2 and later but the base branch of this PR is 2.3.x which uses SI 5.3.x. Should we be looking to make the changes proposed here in Boot 2.2.x and later?

[artembilan]

Thank you, @wilkinsona , for your help!
Yes, I think this has to be back-ported to Spring Boot 2.2.x and of course to master.
I missed somehow the fact that Boot 2.2.x is still in support.