diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/composite/AbstractCompositeMeter.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/composite/AbstractCompositeMeter.java index 92ebc857cd..e9d5c58cbb 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/composite/AbstractCompositeMeter.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/composite/AbstractCompositeMeter.java @@ -15,14 +15,14 @@ */ package io.micrometer.core.instrument.composite; -import io.micrometer.common.lang.Nullable; 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 getChildren() { } T firstChild() { - final Iterator 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 --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/composite/CompositeMeterRegistryTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/composite/CompositeMeterRegistryTest.java index 4d95274858..4a07b63dc8 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/instrument/composite/CompositeMeterRegistryTest.java +++ b/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 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> 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 + } + }