CompositeMeterRegistry removes meters from descendants that are still referenced by composite meters.

[keith-turner]

Describe the bug

When a CompositeMeterRegistry has a descendant MeterRegistry that has filters it is possible to create a situation where multiple composite meters created by the CompositeMeterRegistry point to a single meter in the descendant registry. If any of these composite meters are removed from the CompositeMeterRegistry it will remove the meter in the descendant registry even if its still referenced by other composite meters.

Environment

• Micrometer version 1.13.6
• Micrometer registry CompositeMeterRegistry
• OS: Linux
• Java version: openjdk version "17.0.13" 2024-10-15 LTS

To Reproduce

The following code demonstrates the problem. After meter2 is removed in the following example, no output will ever be seen from meter1.

import io.micrometer.core.instrument.DistributionSummary;
import io.micrometer.core.instrument.composite.CompositeMeterRegistry;
import io.micrometer.core.instrument.config.MeterFilter;
import io.micrometer.core.instrument.logging.LoggingMeterRegistry;
import io.micrometer.core.instrument.logging.LoggingRegistryConfig;

public class Test {

  public static void main(String[] args) throws Exception {
    LoggingRegistryConfig lconf = c -> {
      if (c.equals("logging.step")) {
        return "1s";
      }
      return null;
    };

    var compositeRegistry = new CompositeMeterRegistry();
    var loggingRegistry =
        LoggingMeterRegistry.builder(lconf).loggingSink(System.out::println).build();
    loggingRegistry.config().meterFilter(MeterFilter.ignoreTags("tableId"));
    compositeRegistry.add(loggingRegistry);

    var meter1 = DistributionSummary.builder("scan.results").tag("address", "localhost:1234")
        .tag("tableId", "2").register(compositeRegistry);
    var meter2 = DistributionSummary.builder("scan.results").tag("address", "localhost:1234")
        .tag("tableId", "3").register(compositeRegistry);
    var meter3 = DistributionSummary.builder("scan.results").tag("address", "localhost:6789")
            .tag("tableId", "3").register(compositeRegistry);

    System.out.println("loggingRegistry meters   : ");
    loggingRegistry.getMeters().forEach(meter -> System.out.println("  " + meter.getId()));

    System.out.println("compositeRegistry meters : ");
    compositeRegistry.getMeters().forEach(meter -> System.out.println("  " + meter.getId()));

    meter1.record(5);
    meter2.record(10);
    meter3.record(15);
    // let the logger run
    Thread.sleep(2000);

    var removedMeter = compositeRegistry.remove(meter2);
    System.out.println("removed " + removedMeter.getId() + " from compositeRegistry");

    System.out.println("loggingRegistry meters   : ");
    loggingRegistry.getMeters().forEach(meter -> System.out.println("  " + meter.getId()));

    System.out.println("compositeRegistry meters : ");
    compositeRegistry.getMeters().forEach(meter -> System.out.println("  " + meter.getId()));

    meter1.record(6);
    meter3.record(8);
    // give the loggingRegistry a chance to run
    Thread.sleep(5000);
  }
}

Running the example outputs the following.

loggingRegistry meters   : 
  MeterId{name='scan.results', tags=[tag(address=localhost:6789)]}
  MeterId{name='scan.results', tags=[tag(address=localhost:1234)]}
compositeRegistry meters : 
  MeterId{name='scan.results', tags=[tag(address=localhost:6789),tag(tableId=3)]}
  MeterId{name='scan.results', tags=[tag(address=localhost:1234),tag(tableId=3)]}
  MeterId{name='scan.results', tags=[tag(address=localhost:1234),tag(tableId=2)]}
scan.results{address=localhost:6789} throughput=1/s mean=15 max=15
scan.results{address=localhost:1234} throughput=2/s mean=7.5 max=10
removed MeterId{name='scan.results', tags=[tag(address=localhost:1234),tag(tableId=3)]} from compositeRegistry
loggingRegistry meters   : 
  MeterId{name='scan.results', tags=[tag(address=localhost:6789)]}
compositeRegistry meters : 
  MeterId{name='scan.results', tags=[tag(address=localhost:6789),tag(tableId=3)]}
  MeterId{name='scan.results', tags=[tag(address=localhost:1234),tag(tableId=2)]}
scan.results{address=localhost:6789} throughput=1/s mean=8 max=8

Expected behavior

Meters are only removed from descendants when no composite meters reference them. In the example above the meter MeterId{name='scan.results', tags=[tag(address=localhost:1234)]} should only be removed from loggingRegistry after meter1 and meter2 are removed from compositeRegistry.

To implement this change, maybe a reference check could be added here prior to removal.

Additional context

Issue #1159 seems related, but its not the exact same problem. This issue and #1159 both point to a more general problem that CompositeMeterRegistry and its descendants form a graph of meters and referential integrity of this graph is not always being properly maintained.