Remove meters from child registries using mapped Id (micrometer-metri…

…cs#2362)

Child registries may have a `MeterFilter` configured that alter the Id of meters when registered. In this case, calling `remove` with the Id from the `CompositeMeterRegistry` will not match unless it also has the same `MeterFilter` configured. The behavior of the `remove` method has fluctuated as these cases come to light. This adds public API for removing a meter with its pre-mapped Id and clarifies the behavior in the JavaDoc of all `remove` methods. `CompositeMeterRegistry` can now use this new API to have the expected behavior.

Resolves micrometer-metricsgh-2354

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

@@ -604,6 +604,9 @@ private boolean accept(Meter.Id id) {
     }
 
     /**
+     * Remove a {@link Meter} from this {@link MeterRegistry registry}. This is expected to be a {@link Meter} with
+     * the same {@link Id} returned when registering a meter - which will have {@link MeterFilter}s applied to it.
+     *
      * @param meter The meter to remove
      * @return The removed meter, or null if the provided meter is not currently registered.
      * @since 1.1.0
@@ -614,15 +617,25 @@ public Meter remove(Meter meter) {
         return remove(meter.getId());
     }
 
+    /**
+     * Remove a {@link Meter} from this {@link MeterRegistry registry} based on its {@link Id}
+     * before applying this registry's {@link MeterFilter}s to the given {@link Id}.
+     *
+     * @param preFilterId the id of the meter to remove
+     * @return The removed meter, or null if the meter is not found
+     * @since 1.3.16
+     */
+    @Incubating(since = "1.3.16")
     @Nullable
-    Meter remove(Meter.Id id, boolean applyFilters) {
-        if (applyFilters) {
-            return remove(getMappedId(id));
-        }
-        return remove(id);
+    public Meter removeByPreFilterId(Meter.Id preFilterId) {
+        return remove(getMappedId(preFilterId));
     }
 
     /**
+     * Remove a {@link Meter} from this {@link MeterRegistry registry} based the given {@link Id} as-is. The registry's
+     * {@link MeterFilter}s will not be applied to it. You can use the {@link Id} of the {@link Meter} returned
+     * when registering a meter, since that will have {@link MeterFilter}s already applied to it.
+     *
      * @param mappedId The id of the meter to remove
      * @return The removed meter, or null if no meter matched the provided id.
      * @since 1.1.0

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

@@ -66,7 +66,7 @@ public void register(Iterable<Row<?>> rows, boolean overwrite) {
                         boolean previouslyDefined = oldRows.contains(rowId);
 
                         if (overwrite && previouslyDefined) {
-                            registry.remove(rowId, true);
+                            registry.removeByPreFilterId(rowId);
                         }
 
                         if (overwrite || !previouslyDefined) {
@@ -81,7 +81,7 @@ public void register(Iterable<Row<?>> rows, boolean overwrite) {
 
             for (Meter.Id oldRow : oldRows) {
                 if (!newRows.contains(oldRow))
-                    registry.remove(oldRow, true);
+                    registry.removeByPreFilterId(oldRow);
             }
 
             return newRows;

micrometer-core/src/main/java/io/micrometer/core/instrument/composite/CompositeMeterRegistry.java

@@ -67,7 +67,7 @@ public CompositeMeterRegistry(Clock clock, Iterable<MeterRegistry> registries) {
                 })
                 .onMeterRemoved(m -> {
                     if (m instanceof CompositeMeter) { // should always be
-                        lock(registriesLock, () -> nonCompositeDescendants.forEach(r -> r.remove(m)));
+                        lock(registriesLock, () -> nonCompositeDescendants.forEach(r -> r.removeByPreFilterId(m.getId())));
                     }
                 });
 

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

@@ -475,4 +475,17 @@ void meterRegistrationShouldWorkConcurrently() throws InterruptedException {
         }
     }
 
+    @Test
+    @Issue("#2354")
+    void meterRemovalPropagatesToChildRegistryWithModifyingFilter() {
+        this.simple.config().commonTags("host", "localhost");
+        this.composite.add(this.simple);
+
+        Counter counter = this.composite.counter("my.counter");
+        this.composite.remove(counter);
+
+        assertThat(this.composite.getMeters()).isEmpty();
+        assertThat(this.simple.getMeters()).isEmpty();
+    }
+
 }