fix reactor#1383 metrics() without Micrometer triggers NoClassDefFoundError

Reorganizes the micrometer detection into a dedicated class `Metrics`, which avoids loading the entire `FluxMetrics` class, including Micrometer-dependent code.

Author: simonbasle

build.gradle

@@ -236,10 +236,21 @@ project('reactor-core') {
   apply plugin: 'idea' //needed to avoid IDEA seeing the jmh folder as source
   apply plugin: 'me.champeau.gradle.jmh'
 
+  sourceSets {
+	noMicrometerTest {
+	  java.srcDir 'src/test/java'
+	  resources.srcDir 'src/test/resources'
+	}
+  }
+
   configurations {
 	compileOnly.extendsFrom jsr166backport
 	testCompile.extendsFrom jsr166backport
 	baseline
+	noMicrometerTestRuntime {
+	  extendsFrom testRuntime
+	  exclude group:"io.micrometer", module:"micrometer-core"
+	}
   }
 
   dependencies {
@@ -283,6 +294,10 @@ project('reactor-core') {
 		force = true
 	  }
 	}
+
+	noMicrometerTestCompile sourceSets.main.output
+	noMicrometerTestCompile sourceSets.test.output
+	noMicrometerTestCompile configurations.testCompile
   }
 
   jmh {
@@ -410,11 +425,22 @@ project('reactor-core') {
 	}
   }
 
+  task testNoMicrometer(type: Test, group: 'verification') {
+	testClassesDirs = sourceSets.noMicrometerTest.output.classesDirs
+	classpath = sourceSets.noMicrometerTest.runtimeClasspath
+	include '**/*NoMicrometerTest.*'
+
+	doFirst {
+	  println "Additional tests without Micrometer ($includes)"
+	}
+  }
+
   //inherit basic test task + common configuration in root
   //always depend on testStaticInit, skip testNG on Travis, skip loops when not releasing
   //note that this way the tasks can be run individually
   test {
 	dependsOn testStaticInit
+	dependsOn testNoMicrometer
 	if (System.env.TRAVIS != "true") {
 	  dependsOn testNG
 	}

reactor-core/src/main/java/reactor/core/publisher/Flux.java

@@ -53,6 +53,7 @@
 import reactor.core.Disposable;
 import reactor.core.Exceptions;
 import reactor.core.Fuseable;
+import reactor.util.Metrics;
 import reactor.core.Scannable;
 import reactor.core.publisher.FluxSink.OverflowStrategy;
 import reactor.core.scheduler.Scheduler;
@@ -5738,7 +5739,7 @@ public final Flux<T> mergeWith(Publisher<? extends T> other) {
 	 * @return an instrumented {@link Flux}
 	 */
 	public final Flux<T> metrics() {
-		if (!FluxMetrics.isMicrometerAvailable()) {
+		if (!Metrics.isInstrumentationAvailable()) {
 			return this;
 		}
 

reactor-core/src/main/java/reactor/core/publisher/FluxMetrics.java

@@ -43,7 +43,7 @@
 /**
  * Activate metrics gathering on a {@link Flux}, assuming Micrometer is on the classpath.
  *
- * @implNote Metrics.isMicrometerAvailable() test should be performed BEFORE instantiating
+ * @implNote Metrics.isInstrumentationAvailable() test should be performed BEFORE instantiating
  * or referencing this class, otherwise a {@link NoClassDefFoundError} will be thrown if
  * Micrometer is not there.
  *
@@ -53,27 +53,6 @@ final class FluxMetrics<T> extends FluxOperator<T, T> {
 
 	private static final Logger LOGGER = Loggers.getLogger(FluxMetrics.class);
 
-	private static final boolean isMicrometerAvailable;
-
-	static {
-		boolean micrometer;
-		try {
-			io.micrometer.core.instrument.Metrics.globalRegistry.getRegistries();
-			micrometer = true;
-		}
-		catch (Throwable t) {
-			micrometer = false;
-		}
-		isMicrometerAvailable = micrometer;
-	}
-
-	/**
-	 * @return true if the Micrometer instrumentation facade is available
-	 */
-	static boolean isMicrometerAvailable() {
-		return isMicrometerAvailable;
-	}
-
 	//=== Constants ===
 
 	/**
@@ -184,8 +163,6 @@ static Tuple2<String, List<Tag>> resolveNameAndTags(Publisher<?> source) {
 
 	final String    name;
 	final List<Tag> tags;
-
-	@Nullable
 	final MeterRegistry    registryCandidate;
 
 	FluxMetrics(Flux<? extends T> flux) {
@@ -204,16 +181,18 @@ static Tuple2<String, List<Tag>> resolveNameAndTags(Publisher<?> source) {
 		this.name = nameAndTags.getT1();
 		this.tags = nameAndTags.getT2();
 
-		this.registryCandidate = registry;
+		if (registry == null) {
+			this.registryCandidate = Metrics.globalRegistry;
+		}
+		else {
+			this.registryCandidate = registry;
+		}
+
 	}
 
 	@Override
 	public void subscribe(CoreSubscriber<? super T> actual) {
-		MeterRegistry registry = Metrics.globalRegistry;
-		if (registryCandidate != null) {
-			registry = registryCandidate;
-		}
-		source.subscribe(new MicrometerFluxMetricsSubscriber<>(actual, registry,
+		source.subscribe(new MicrometerFluxMetricsSubscriber<>(actual, registryCandidate,
 				Clock.SYSTEM, this.name, this.tags));
 	}
 

reactor-core/src/main/java/reactor/core/publisher/FluxMetricsFuseable.java

@@ -33,7 +33,7 @@
  * Activate metrics gathering on a {@link Flux} (Fuseable version), assumes Micrometer is
  * on the classpath.
  *
- * @implNote Metrics.isMicrometerAvailable() test should be performed BEFORE instantiating
+ * @implNote Metrics.isInstrumentationAvailable() test should be performed BEFORE instantiating
  * or referencing this class, otherwise a {@link NoClassDefFoundError} will be thrown if
  * Micrometer is not there.
  *

reactor-core/src/main/java/reactor/core/publisher/Mono.java

@@ -44,6 +44,7 @@
 import reactor.core.CoreSubscriber;
 import reactor.core.Disposable;
 import reactor.core.Fuseable;
+import reactor.util.Metrics;
 import reactor.core.Scannable;
 import reactor.core.scheduler.Scheduler;
 import reactor.core.scheduler.Scheduler.Worker;
@@ -2803,7 +2804,7 @@ public final Flux<T> mergeWith(Publisher<? extends T> other) {
 	 * @return an instrumented {@link Mono}
 	 */
 	public final Mono<T> metrics() {
-		if (!FluxMetrics.isMicrometerAvailable()) {
+		if (!Metrics.isInstrumentationAvailable()) {
 			return this;
 		}
 

reactor-core/src/main/java/reactor/core/publisher/MonoMetrics.java

@@ -34,7 +34,7 @@
 /**
  * Activate metrics gathering on a {@link Mono}, assumes Micrometer is on the classpath.
  *
- * @implNote Metrics.isMicrometerAvailable() test should be performed BEFORE instantiating
+ * @implNote Metrics.isInstrumentationAvailable() test should be performed BEFORE instantiating
  * or referencing this class, otherwise a {@link NoClassDefFoundError} will be thrown if
  * Micrometer is not there.
  *
@@ -44,8 +44,6 @@ final class MonoMetrics<T> extends MonoOperator<T, T> {
 
 	final String    name;
 	final List<Tag> tags;
-
-	@Nullable
 	final MeterRegistry meterRegistry;
 
 	MonoMetrics(Mono<? extends T> mono) {
@@ -64,16 +62,17 @@ final class MonoMetrics<T> extends MonoOperator<T, T> {
 		this.name = nameAndTags.getT1();
 		this.tags = nameAndTags.getT2();
 
-		this.meterRegistry = meterRegistry;
+		if (meterRegistry == null) {
+			this.meterRegistry = Metrics.globalRegistry;
+		}
+		else {
+			this.meterRegistry = meterRegistry;
+		}
 	}
 
 	@Override
 	public void subscribe(CoreSubscriber<? super T> actual) {
-		MeterRegistry registry = Metrics.globalRegistry;
-		if (meterRegistry != null) {
-			registry = meterRegistry;
-		}
-		source.subscribe(new MicrometerMonoMetricsSubscriber<>(actual, registry,
+		source.subscribe(new MicrometerMonoMetricsSubscriber<>(actual, meterRegistry,
 				Clock.SYSTEM, this.name, this.tags));
 	}
 

reactor-core/src/main/java/reactor/core/publisher/MonoMetricsFuseable.java

@@ -32,7 +32,7 @@
  * Activate metrics gathering on a {@link Mono} (Fuseable version), assumes Micrometer is
  * on the classpath.
  *
- * @implNote Metrics.isMicrometerAvailable() test should be performed BEFORE instantiating
+ * @implNote Metrics.isInstrumentationAvailable() test should be performed BEFORE instantiating
  * or referencing this class, otherwise a {@link NoClassDefFoundError} will be thrown if
  * Micrometer is not there.
  *

reactor-core/src/main/java/reactor/util/Metrics.java

@@ -0,0 +1,52 @@
+/*
+ * Copyright (c) 2011-2018 Pivotal Software Inc, All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *        http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package reactor.util;
+
+import static io.micrometer.core.instrument.Metrics.globalRegistry;
+
+/**
+ * Utilities around instrumentation and metrics with Micrometer.
+ *
+ * @author Simon Baslé
+ */
+public class Metrics {
+
+	static final boolean isMicrometerAvailable;
+
+	static {
+		boolean micrometer;
+		try {
+			globalRegistry.getRegistries();
+			micrometer = true;
+		}
+		catch (Throwable t) {
+			micrometer = false;
+		}
+		isMicrometerAvailable = micrometer;
+	}
+
+	/**
+	 * Check if the current runtime supports metrics / instrumentation, by
+	 * verifying if Micrometer is on the classpath.
+	 *
+	 * @return true if the Micrometer instrumentation facade is available
+	 */
+	public static final boolean isInstrumentationAvailable() {
+		return isMicrometerAvailable;
+	}
+
+}

reactor-core/src/test/java/reactor/core/publisher/MonoSourceTest.java

@@ -18,17 +18,15 @@
 import java.util.concurrent.Callable;
 import java.util.concurrent.atomic.AtomicBoolean;
 
-import jdk.nashorn.internal.codegen.CompilerConstants;
 import org.junit.Test;
 import org.reactivestreams.Publisher;
 import org.reactivestreams.Subscriber;
+
 import reactor.core.Fuseable;
 import reactor.core.Scannable;
 import reactor.test.StepVerifier;
-import reactor.test.publisher.PublisherProbe;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
 import static org.junit.Assert.assertTrue;
 
 public class MonoSourceTest {

reactor-core/src/test/java/reactor/util/MetricsNoMicrometerTest.java

@@ -0,0 +1,74 @@
+/*
+ * Copyright (c) 2011-2018 Pivotal Software Inc, All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *        http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package reactor.util;
+
+import org.assertj.core.api.Assumptions;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatCode;
+
+/**
+ * This test case should be OK to run in the normal test source set, but is intended
+ * for a test sourceset that doesn't include Micrometer dependency, and which only runs
+ * tests validating the likes of {@link Flux#metrics()} are NO-OP in that context.
+ *
+ * @author Simon Baslé
+ */
+public class MetricsNoMicrometerTest {
+
+
+	@BeforeClass
+	public static void assumeNoMicrometer() {
+		Assumptions.assumeThat(Metrics.isInstrumentationAvailable())
+		           .as("Micrometer on the classpath").isFalse();
+	}
+
+	@Test
+	public void isMicrometerAvailable() {
+		assertThat(Metrics.isInstrumentationAvailable()).isFalse();
+	}
+
+	@Test
+	public void FluxMetricsNoOp() {
+		assertThatCode(() -> Flux.just("foo").hide().metrics().blockLast())
+				.doesNotThrowAnyException();
+	}
+
+	@Test
+	public void FluxMetricsFusedNoOp() {
+		assertThatCode(() -> Flux.just("foo").metrics().blockLast())
+				.doesNotThrowAnyException();
+	}
+
+	@Test
+	public void MonoMetricsNoOp() {
+		assertThatCode(() -> Mono.just("foo").hide().metrics().block())
+				.doesNotThrowAnyException();
+	}
+
+	@Test
+	public void MonoMetricsFusedNoOp() {
+		assertThatCode(() -> Mono.just("foo").metrics().block())
+				.doesNotThrowAnyException();
+	}
+
+}