Track meters made stale by a MeterFilter registration

This makes a compromise between optimizing avoiding applying filters and maintaining existing behavior where it would make a difference. The tests demonstrate the two behaviors we want. Concurrency concerns may need to be considered more, but this demonstrates the idea.

micrometer-core/src/main/java/io/micrometer/core/instrument/MeterRegistry.java

@@ -101,6 +101,8 @@ public abstract class MeterRegistry {
 
     private final Map<Id, Meter> preFilterIdToMeterMap = new HashMap<>();
 
+    private final Set<Id> stalePreFilterIds = new HashSet<>();
+
     /**
      * Map of meter id whose associated meter contains synthetic counterparts to those
      * synthetic ids. We maintain these associations so that when we remove a meter with
@@ -615,7 +617,7 @@ private Meter getOrCreateMeter(@Nullable DistributionStatisticConfig config,
             Function<Meter.Id, ? extends Meter> noopBuilder) {
 
         Meter m = preFilterIdToMeterMap.get(originalId);
-        if (m != null) {
+        if (m != null && !unmarkStaleId(originalId)) {
             return m;
         }
 
@@ -664,6 +666,15 @@ private Meter getOrCreateMeter(@Nullable DistributionStatisticConfig config,
         return m;
     }
 
+    /**
+     * Marks the given ID as no longer stale, if it is stale. If it isn't stale, does nothing.
+     * @param originalId id before any filter mapping has been applied
+     * @return {@code true} if the id is stale
+     */
+    private boolean unmarkStaleId(Id originalId) {
+        return !stalePreFilterIds.isEmpty() && stalePreFilterIds.remove(originalId);
+    }
+
     private boolean accept(Meter.Id id) {
         for (MeterFilter filter : filters) {
             MeterFilterReply reply = filter.accept(id);
@@ -760,10 +771,6 @@ public class Config {
         /**
          * Append a list of common tags to apply to all metrics reported to the monitoring
          * system.
-         * <p>
-         * </p>
-         * <strong>NOTE: A no-op operation if meters are already registered to the
-         * registry.</strong>
          * @param tags Tags to add to every metric.
          * @return This configuration instance.
          */
@@ -775,10 +782,6 @@ public Config commonTags(Iterable<Tag> tags) {
          * Append a list of common tags to apply to all metrics reported to the monitoring
          * system. Must be an even number of arguments representing key/value pairs of
          * tags.
-         * <p>
-         * </p>
-         * <strong>NOTE: A no-op operation if meters are already registered to the
-         * registry.</strong>
          * @param tags MUST be an even number of arguments representing key/value pairs of
          * tags.
          * @return This configuration instance.
@@ -790,20 +793,17 @@ public Config commonTags(String... tags) {
         /**
          * Add a meter filter to the registry. Filters are applied in the order in which
          * they are added.
-         * <p>
-         * </p>
-         * <strong>NOTE: A no-op operation if meters are already registered to the
-         * registry.</strong>
          * @param filter The filter to add to the registry.
          * @return This configuration instance.
          */
         public synchronized Config meterFilter(MeterFilter filter) {
-            if (meterMap.isEmpty()) {
-                MeterFilter[] newFilters = new MeterFilter[filters.length + 1];
-                System.arraycopy(filters, 0, newFilters, 0, filters.length);
-                newFilters[filters.length] = filter;
-                filters = newFilters;
+            if (!preFilterIdToMeterMap.isEmpty()) {
+                stalePreFilterIds.addAll(preFilterIdToMeterMap.keySet());
             }
+            MeterFilter[] newFilters = new MeterFilter[filters.length + 1];
+            System.arraycopy(filters, 0, newFilters, 0, filters.length);
+            newFilters[filters.length] = filter;
+            filters = newFilters;
             return this;
         }
 

micrometer-core/src/test/java/io/micrometer/core/instrument/MeterRegistryTest.java

@@ -27,6 +27,9 @@
 
 import javax.annotation.Nonnull;
 
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.atomic.AtomicInteger;
+
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
@@ -221,4 +224,35 @@ void acceptPercentilesNullOrEmpty() {
         LongTaskTimer.builder("timer.percentiles.empty").publishPercentiles(new double[] {}).register(registry);
     }
 
+    @Test
+    void filterConfiguredAfterMeterRegistered() {
+        Counter c1 = registry.counter("counter");
+        registry.config().commonTags("common", "tag");
+        Counter c2 = registry.counter("counter");
+
+        assertThat(c1.getId().getTags()).isEmpty();
+        assertThat(c2.getId().getTags()).containsExactly(Tag.of("common", "tag"));
+    }
+
+    @Test
+    void doNotCallFiltersWhenUnnecessary() {
+        AtomicInteger filterCallCount = new AtomicInteger();
+        registry.config().meterFilter(new MeterFilter() {
+            @Override
+            public Meter.Id map(Meter.Id id) {
+                filterCallCount.incrementAndGet();
+                return id;
+            }
+        });
+        Counter c1 = registry.counter("counter");
+        assertThat(filterCallCount.get()).isOne();
+        Counter c2 = registry.counter("counter");
+        assertThat(filterCallCount.get()).isOne();
+
+        assertThat(c1).isSameAs(c2);
+
+        registry.counter("other");
+        assertThat(filterCallCount.get()).isEqualTo(2);
+    }
+
 }