Ignore noop children when polling composite meters

When a CompositeMeterRegistry is used and one of its CompositeMeters
is polled (its value is fetched), the returned value can depend on
the order of the registries inside of the composite if the composite
contains a registry that has any NoopMeters.

Example: a CompositeMeterRegistry contains two registries, A and B.
We create a counter in the composite and increment it once.
After this both A and B contain one counter but lets say that
in A the counter is ignored so it will be noop.
When the count called on CompositeCounter, it can return
either 0 (if NoopCounter was used)
or 1 (if the non-noop counter was used).

In order to fix this, we can ignore the NoopMeters
when Meters are polled in a composite registry.

Closes micrometer-metricsgh-1441

Author: jonatan-ivanov

Diff for: micrometer-core/src/main/java/io/micrometer/core/instrument/composite/AbstractCompositeMeter.java

@@ -18,11 +18,11 @@
 import io.micrometer.core.instrument.AbstractMeter;
 import io.micrometer.core.instrument.Meter;
 import io.micrometer.core.instrument.MeterRegistry;
+import io.micrometer.core.instrument.noop.NoopMeter;
 import io.micrometer.core.lang.Nullable;
 
 import java.util.Collections;
 import java.util.IdentityHashMap;
-import java.util.Iterator;
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicBoolean;
 
@@ -49,9 +49,11 @@ final Iterable<T> getChildren() {
     }
 
     T firstChild() {
-        final Iterator<T> i = children.values().iterator();
-        if (i.hasNext())
-            return i.next();
+        for (T next : children.values()) {
+            if (!(next instanceof NoopMeter)) {
+                return next;
+            }
+        }
 
         // There are no child meters. Return a lazily instantiated no-op meter.
         final T noopMeter = this.noopMeter;

Diff for: micrometer-core/src/test/java/io/micrometer/core/instrument/composite/CompositeMeterRegistryTest.java

@@ -28,18 +28,23 @@
 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.RepeatedTest;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
 import reactor.core.publisher.Flux;
 import reactor.core.scheduler.Schedulers;
 
 import java.time.Duration;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.stream.Stream;
 
+import static java.util.Arrays.asList;
 import static java.util.Collections.emptyList;
 import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
@@ -493,4 +498,31 @@ void meterRemovalPropagatesToChildRegistryWithModifyingFilter() {
         assertThat(this.simple.getMeters()).isEmpty();
     }
 
+    @Issue("#1441")
+    @ParameterizedTest
+    @MethodSource("registriesProvider")
+    void whenMetersArePolledNoopChildrenShouldBeIgnored(List<MeterRegistry> registries) {
+        // this means that firstChild() practically should be firstNonNoopChild()
+        CompositeMeterRegistry composite = new CompositeMeterRegistry(Clock.SYSTEM, registries);
+        Counter counter = composite.counter("my.counter");
+        counter.increment();
+        assertThat(counter.count()).isEqualTo(1);
+    }
+
+    static Stream<List<MeterRegistry>> registriesProvider() {
+        // Since the order is non-deterministic, the best effort is testing both orders
+        SimpleMeterRegistry denyAllRegistry1 = new SimpleMeterRegistry();
+        denyAllRegistry1.config().meterFilter(MeterFilter.deny());
+
+        SimpleMeterRegistry denyAllRegistry2 = new SimpleMeterRegistry();
+        denyAllRegistry2.config().meterFilter(MeterFilter.deny());
+
+        // @formatter:off
+        return Stream.of(
+                asList(denyAllRegistry1, new SimpleMeterRegistry()), // denyAll,  allowAll
+                asList(new SimpleMeterRegistry(), denyAllRegistry2)  // allowAll, denyAll
+        );
+        // @formatter:on
+    }
+
 }