Polish "Prevent double update of metrics when CompositeMeterRegistry exists"

See gh-16221

spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/MeterRegistryConfigurer.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2018 the original author or authors.
+ * Copyright 2012-2019 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -63,12 +63,8 @@ class MeterRegistryConfigurer {
 		// tags or alter timer or summary configuration.
 		customize(registry);
 		addFilters(registry);
-		if (this.hasCompositeMeterRegistry) {
-			if (registry instanceof CompositeMeterRegistry) {
-				addBinders(registry);
-			}
-		}
-		else {
+		if (!this.hasCompositeMeterRegistry
+				|| registry instanceof CompositeMeterRegistry) {
 			addBinders(registry);
 		}
 		if (this.addToGlobalRegistry && registry != Metrics.globalRegistry) {

spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/MeterRegistryConfigurerIntegrationTests.java

@@ -18,12 +18,13 @@ package org.springframework.boot.actuate.autoconfigure.metrics;
 
 import java.util.Map;
 
+import ch.qos.logback.classic.Logger;
+import ch.qos.logback.classic.LoggerContext;
 import io.micrometer.core.instrument.MeterRegistry;
 import io.micrometer.core.instrument.binder.MeterBinder;
 import io.micrometer.core.instrument.composite.CompositeMeterRegistry;
 import org.junit.Test;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+import org.slf4j.impl.StaticLoggerBinder;
 
 import org.springframework.beans.BeansException;
 import org.springframework.beans.factory.config.BeanPostProcessor;
@@ -74,13 +75,14 @@ public class MeterRegistryConfigurerIntegrationTests {
 	}
 
 	@Test
-	public void counterWhenCompositeMeterRegistryDoesNotExistShouldWork() {
+	public void counterIsIncrementedOncePerEventWithoutCompositeMeterRegistry() {
 		new ApplicationContextRunner()
 				.with(MetricsRun.limitedTo(JmxMetricsExportAutoConfiguration.class))
 				.withConfiguration(
 						AutoConfigurations.of(LogbackMetricsAutoConfiguration.class))
 				.run((context) -> {
-					Logger logger = LoggerFactory.getLogger("test-logger");
+					Logger logger = ((LoggerContext) StaticLoggerBinder.getSingleton()
+							.getLoggerFactory()).getLogger("test-logger");
 					logger.error("Error.");
 
 					Map<String, MeterRegistry> registriesByName = context
@@ -93,16 +95,16 @@ public class MeterRegistryConfigurerIntegrationTests {
 	}
 
 	@Test
-	public void counterWhenCompositeMeterRegistryExistsShouldWork() {
+	public void counterIsIncrementedOncePerEventWithCompositeMeterRegistry() {
 		new ApplicationContextRunner()
 				.with(MetricsRun.limitedTo(JmxMetricsExportAutoConfiguration.class,
 						PrometheusMetricsExportAutoConfiguration.class))
 				.withConfiguration(
 						AutoConfigurations.of(LogbackMetricsAutoConfiguration.class))
 				.run((context) -> {
-					Logger logger = LoggerFactory.getLogger("test-logger");
+					Logger logger = ((LoggerContext) StaticLoggerBinder.getSingleton()
+							.getLoggerFactory()).getLogger("test-logger");
 					logger.error("Error.");
-
 					Map<String, MeterRegistry> registriesByName = context
 							.getBeansOfType(MeterRegistry.class);
 					assertThat(registriesByName).hasSize(3);

spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/MeterRegistryConfigurerTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2018 the original author or authors.
+ * Copyright 2012-2019 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -38,6 +38,7 @@ import static org.mockito.BDDMockito.given;
 import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyZeroInteractions;
 
 /**
  * Tests for {@link MeterRegistryConfigurer}.
@@ -80,7 +81,7 @@ public class MeterRegistryConfigurerTests {
 		MeterRegistryConfigurer configurer = new MeterRegistryConfigurer(
 				createObjectProvider(this.customizers),
 				createObjectProvider(this.filters), createObjectProvider(this.binders),
-				false, true);
+				false, false);
 		CompositeMeterRegistry composite = new CompositeMeterRegistry();
 		configurer.configure(composite);
 		verify(this.mockCustomizer).customize(composite);
@@ -119,6 +120,29 @@ public class MeterRegistryConfigurerTests {
 		verify(this.mockBinder).bindTo(this.mockRegistry);
 	}
 
+	@Test
+	public void configureShouldApplyBinderToComposite() {
+		this.binders.add(this.mockBinder);
+		MeterRegistryConfigurer configurer = new MeterRegistryConfigurer(
+				createObjectProvider(this.customizers),
+				createObjectProvider(this.filters), createObjectProvider(this.binders),
+				false, true);
+		CompositeMeterRegistry composite = new CompositeMeterRegistry();
+		configurer.configure(composite);
+		verify(this.mockBinder).bindTo(composite);
+	}
+
+	@Test
+	public void configureShouldNotApplyBinderWhenCompositeExists() {
+		this.binders.add(this.mockBinder);
+		MeterRegistryConfigurer configurer = new MeterRegistryConfigurer(
+				createObjectProvider(this.customizers),
+				createObjectProvider(this.filters), createObjectProvider(this.binders),
+				false, true);
+		configurer.configure(this.mockRegistry);
+		verifyZeroInteractions(this.mockBinder);
+	}
+
 	@Test
 	public void configureShouldBeCalledInOrderCustomizerFilterBinder() {
 		this.customizers.add(this.mockCustomizer);