echauchot commented on a change in pull request #14510:
URL: https://github.com/apache/flink/pull/14510#discussion_r555045820
##########
File path:
flink-runtime/src/test/java/org/apache/flink/runtime/metrics/groups/AbstractMetricGroupTest.java
##########
@@ -415,4 +419,119 @@ public void testGetAllVariablesDoesNotDeadlock() throws
InterruptedException {
childRegisteringThread.join();
}
}
+
+ @Test
+ public void testReporterDelimiterIsFiltered() throws Exception {
+ MetricConfig metricConfig = new MetricConfig();
+ metricConfig.setProperty(
+ ConfigConstants.METRICS_REPORTER_SCOPE_DELIMITER,
String.valueOf('*'));
+
+ MetricRegistryImpl metricRegistry =
+ new MetricRegistryImpl(
+
MetricRegistryConfiguration.defaultMetricRegistryConfiguration(),
+ Collections.singletonList(
+ ReporterSetup.forReporter(
+ "test", metricConfig, new
TestReporter())));
+ try {
+ String metricName = "Test*Counter";
+ TaskManagerMetricGroup metricGroup =
+ new TaskManagerMetricGroup(metricRegistry, "hos*t", "i*d");
+ Counter counter = metricGroup.counter(metricName);
+ TestReporter reporter = (TestReporter)
metricRegistry.getReporters().get(0);
+ final String expected =
+ String.format(
+ "%s%s%s%s%s%s%s",
+ "hos*t".replace('*',
AbstractMetricGroup.DEFAULT_REPLACEMENT),
+ '*',
+ TaskExecutor.TASK_MANAGER_NAME.replace(
+ '*',
AbstractMetricGroup.DEFAULT_REPLACEMENT),
+ '*',
+ "i*d".replace('*',
AbstractMetricGroup.DEFAULT_REPLACEMENT),
+ '*',
+ metricName.replace('*',
AbstractMetricGroup.DEFAULT_REPLACEMENT));
+ assertEquals(expected, reporter.getCounters().get(counter));
+ } finally {
+ metricRegistry.shutdown().get();
+ }
+ }
+
+ private static class TestReporter extends AbstractReporter {
+ @Override
+ public void open(MetricConfig config) {}
+
+ @Override
+ public void close() {}
+
+ public Map<Counter, String> getCounters() {
+ return counters;
+ }
+
+ @Override
+ public String filterCharacters(String input) {
+ return input;
+ }
+ }
+
+ @Test
+ public void testLowLevelGetAllVariablesWithFilterAndDelimiter() throws
Exception {
+ MetricRegistry registry =
TestingMetricRegistry.builder().setNumberReporters(2).build();
+
+ AbstractMetricGroup<?> group =
+ new GenericMetricGroup(registry, null, "test") {
+ @Override
+ protected void putVariables(Map<String, String> variables)
{
+ variables.put("k*1", "v*1");
+ variables.put("k*2", "v*2");
+ }
+ };
+
+ Map<String, String> allVariables;
+ final CharacterFilter simpleFilter = input -> input.replace('1', 'a');
+
+ // test filter wrapping
+ allVariables = group.getAllVariables(simpleFilter, 0,
Collections.emptySet(), '*');
+ assertThat(allVariables, IsMapContaining.hasKey("k_a"));
+ assertThat(allVariables, IsMapContaining.hasValue("v_a"));
+
+ // test default filter, add exclusions to avoid cache hit
+ allVariables = group.getAllVariables(null, 1,
Collections.singleton("k*2"), '*');
+ assertThat(allVariables, IsMapContaining.hasKey("k_1"));
+ assertThat(allVariables, IsMapContaining.hasValue("v_1"));
Review comment:
as stated in the comment in the code, this exclusion is added only to
avoid cache hit. It is not clear enough, I'll replace it by just recreating the
group.
Regarding exclusions, I'll add proper tests for filtered exclusions, thanks
for pointing out.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
