Proper ordering of metrics auto-configurations

[bendiscz]

Metrics auto-configurations that depend on a MeterRegistry bean
should be configured AFTER configurations that create the
MeterRegistry. However, @AutoConfigureAfter(MetricsAutoConfiguration.class)
is not sufficient. MetricsAutoConfiguration does not export a MeterRegistry.

This commit changes the @AutoConfigureAfter annotations so that the
proper ordering of configurations in ensured.

The long story:

I ran into issues in one of my spring-boot services when trying to fix a Micrometer bug. After adding a simple custom auto-configuration with dependency on MeterRegistry and LogbackMetrics, the application failed to start because of a missing LogbackMetrics dependency. Debug output showed that the LogbackMetricsAutoConfiguration is disabled because the condition @ConditionalOnBean(MeterRegistry.class) is not met. After more debugging I found out that LogbackMetricsAutoConfiguration was evaluated before PrometheusMetricsExportAutoConfiguration.

I created a sample project that demonstrates this issue.

A simple workaround that I currently use in my projects is a dummy auto-configuration that inserts itself between LogbackMetricsAutoConfiguration and configurations that create the MeterRegistry:

@Configuration
@AutoConfigureAfter({ CompositeMeterRegistryAutoConfiguration.class, SimpleMetricsExportAutoConfiguration.class })
@AutoConfigureBefore({
        JvmMetricsAutoConfiguration.class,
        KafkaMetricsAutoConfiguration.class,
        Log4J2MetricsAutoConfiguration.class,
        LogbackMetricsAutoConfiguration.class,
        SystemMetricsAutoConfiguration.class
})
public class ProperMetricsOrderingAutoConfiguration {
    // empty
}

[pivotal-issuemaster]

@bendiscz Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

[pivotal-issuemaster]

@bendiscz Thank you for signing the Contributor License Agreement!

[snicoll]

Thanks for the PR. Did you fix those 5 auto-configurations because you were affected by them or have you reviewed all of them and those particular 5 were a problem?

I've also added a suggestion as I don't really understand looking at the code why those two are necessary as one runs before the other.

[snicoll]

SimpleMetricsExportAutoConfiguration runs before CompositeMeterRegistryAutoConfigurationso I don't understand why both of them have to be defined here. If you run afterCompositeMeterRegistryAutoConfiguration, SimpleMetricsExportAutoConfiguration` should have been processed too.

[bendiscz]

Yes, you are right, CompositeMeterRegistryAutoConfiguration should be sufficient. I added both because I saw this combination in other metrics auto-configurations, but –looking at it again – those were configurations for exporters (and the ordering was "before").

I'm going to change it.

[bendiscz]

Did you fix those 5 auto-configurations because you were affected by them or have you reviewed all of them and those particular 5 were a problem?

I was directly affected by LogbackMetricsAutoConfiguration. Then I checked all other metrics auto-configurations in the same package and saw that they have the same problem.

[philwebb]

Possible related to #11890

[philwebb]

Thanks very much for the PR @bendiscz. One of the reasons we've got away with this for so long is that CompositeMeterRegistryAutoConfiguration just happens to be alphabetically before the configurations that use it.

Unfortunately, despite my best efforts, I couldn't find an easy way to test the fix. Each test I tried ended up causing a order cycle. I'm pretty confident with the fix, so we'll have to just run with it.