From 878781ca1d5e87d29b4143993cdc01de1a7b2b58 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Fri, 19 Jan 2024 15:10:43 +0100 Subject: [PATCH] Do not ignore composite meter filters child meter filters without this change non composite descendant do not know about the configuration of their parents with this change we're merging that configuration fixes gh-3020 --- .../core/instrument/MeterRegistry.java | 18 +++++++++++++++ .../composite/CompositeMeterRegistry.java | 7 ++++++ .../composite/CompositeMeterRegistryTest.java | 23 +++++++++++++++++-- 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/MeterRegistry.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/MeterRegistry.java index 8dd22eef63..e54f355f64 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/MeterRegistry.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/MeterRegistry.java @@ -849,6 +849,24 @@ public Config pauseDetector(PauseDetector detector) { return this; } + MeterFilter[] getFilters() { + return filters; + } + + /** + * Merges the provided configuration with this one. + * @param config configuration to merge + * @return this configuration with merged elements from the provided configuration + * @since 1.9.18 + */ + public Config merge(Config config) { + for (MeterFilter filter : config.getFilters()) { + meterFilter(filter); + } + // TODO: What else should we merge? + return this; + } + /** * @return The pause detector that is currently in effect. */ diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/composite/CompositeMeterRegistry.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/composite/CompositeMeterRegistry.java index 58c4bf03be..8ebaa7b760 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/composite/CompositeMeterRegistry.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/composite/CompositeMeterRegistry.java @@ -227,6 +227,13 @@ private void updateDescendants() { nonCompositeDescendants = descendants; + for (MeterRegistry nonCompositeDescendant : nonCompositeDescendants) { + Config config = nonCompositeDescendant.config(); + if (config != null) { // for tests + config.merge(config()); + } + } + lock(parentLock, () -> parents.forEach(CompositeMeterRegistry::updateDescendants)); } 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..b01f9ef497 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 @@ -44,8 +44,7 @@ import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.*; /** * Tests for {@link CompositeMeterRegistry}. @@ -493,4 +492,24 @@ void meterRemovalPropagatesToChildRegistryWithModifyingFilter() { assertThat(this.simple.getMeters()).isEmpty(); } + @Test + void filtersOnIntermediateCompositeMeterRegistryShouldWork() { + CompositeMeterRegistry parentMeterRegistry = new CompositeMeterRegistry(); + + CompositeMeterRegistry intermediateMeterRegistry = new CompositeMeterRegistry(); + intermediateMeterRegistry.config().meterFilter(MeterFilter.denyNameStartsWith("deny")); + parentMeterRegistry.add(intermediateMeterRegistry); + + SimpleMeterRegistry leafMeterRegistry = new SimpleMeterRegistry(); + intermediateMeterRegistry.add(leafMeterRegistry); + + parentMeterRegistry.counter("deny.item"); + + assertThat(parentMeterRegistry.getMeters()).hasSize(1); + // This seems to be empty with and without the filter. + assertThat(intermediateMeterRegistry.getMeters()).isEmpty(); + // Filters on intermediate composite meter registries don't seem to work. + assertThat(leafMeterRegistry.getMeters()).isEmpty(); + } + }