Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not ignore composite meter filters child meter filters #4601

Draft
wants to merge 1 commit into
base: 1.9.x
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -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.
*/
Expand Down
Expand Up @@ -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));
}

Expand Down
Expand Up @@ -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}.
Expand Down Expand Up @@ -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();
}

}