CompositeMeterRegistry fails to propagate Meter removals to a MeterRegistry that has Filters

[ldesgoui]

Hello,

Consider the simple set up:
Metrics.globalRegistry has no Filters applied
A DatadogMeterRegistry instance has a CommonTags Filter applied and is parented by Metrics.globalRegistry
When Meters are written to Metrics.globalRegistry, they are propagated to the DatadogMeterRegister instance and the Filters are applied
When Meters are removed from Metrics.globalRegistry, they are propagated to the DatadogMeterRegister instance but the Filters are not applied, causing the lookup to fail and them to not get properly deleted

This is caused by this event handler which calls this method, in which the filters are not applied.

This causes the equivalent of a memory leak and aggravates heavily with high-cardinality Meters (such as metrics from reactor executors while using "elastic" schedulers)

I don't have enough Java expertise to offer an accompanying fix and tests, apologies.

[shakuzen]

Thanks for the detailed report. The behavior of the remove method has changed since it was introduced - it used to apply MeterFilters, now it doesn't. We've run into the issue of one behavior being desired in some cases in not others.

While I think we should be able to fix this regardless, can share more details about your use case for removing meters? This is just so we have a more complete picture of the background behind the use case.

[shakuzen]

Here is a unit test that I believe captures the scenario (written in CompositeMeterRegistryTest):

@Test
@Issue("#2354")
void meterRemovedInChildRegistryWithModifyingFilter() {
    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(); // this assertion fails
}

[ldesgoui]

We use Spring which configures and manages its own CompositeMeterRegistry, and reactor which simply writes to micrometer's globalRegistry, globalRegistry is not modified by Spring (aside from adding children), we end up with plenty of Meters from reactor that don't get removed over time.
It seems that reactor recently added functionality to use a different registry than globalRegistry, which could solve our problem I suppose.
We ended up applying the CommonTags configurations to globalRegistry as a temporary workaround.

Test looks good to me.

[admoca60]

Please @shakuzen , take into consideration the PR https://github.com/micrometer-metrics/micrometer/pull/2281/files. This solves the problem.

[shakuzen]

The fix for this is available now in snapshots for 1.3.16, 1.5.8, and 1.6.2. I will be releasing these soon, so feel free to test the snapshots now or the release versions once available to confirm the issue reported here is fixed.