[flink] branch release-1.6 updated: [FLINK-11251][metrics] Exclude variable values from logical scope of generic groups

[ch...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

chesnay pushed a commit to branch release-1.6
in repository https://gitbox.apache.org/repos/asf/flink.git


The following commit(s) were added to refs/heads/release-1.6 by this push:
     new fb9b15c  [FLINK-11251][metrics] Exclude variable values from logical scope of generic groups
fb9b15c is described below

commit fb9b15cce151d4f7783a1c2cce33df58004f865f
Author: Tony Wei <to...@gmail.com>
AuthorDate: Wed Jan 9 16:12:08 2019 +0800

    [FLINK-11251][metrics] Exclude variable values from logical scope of generic groups
---
 .../metrics/groups/AbstractMetricGroup.java        |  2 +-
 .../metrics/groups/GenericValueMetricGroup.java    |  2 +-
 .../runtime/metrics/groups/MetricGroupTest.java    | 33 ++++++++++++++++++++++
 3 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/metrics/groups/AbstractMetricGroup.java b/flink-runtime/src/main/java/org/apache/flink/runtime/metrics/groups/AbstractMetricGroup.java
index fb32130..42e1cbb 100644
--- a/flink-runtime/src/main/java/org/apache/flink/runtime/metrics/groups/AbstractMetricGroup.java
+++ b/flink-runtime/src/main/java/org/apache/flink/runtime/metrics/groups/AbstractMetricGroup.java
@@ -172,7 +172,7 @@ public abstract class AbstractMetricGroup<A extends AbstractMetricGroup<?>> impl
 		}
 	}
 
-	private String createLogicalScope(CharacterFilter filter, char delimiter) {
+	protected String createLogicalScope(CharacterFilter filter, char delimiter) {
 		final String groupName = getGroupName(filter);
 		return parent == null
 			? groupName
diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/metrics/groups/GenericValueMetricGroup.java b/flink-runtime/src/main/java/org/apache/flink/runtime/metrics/groups/GenericValueMetricGroup.java
index ef8e6e8..41e9a91 100644
--- a/flink-runtime/src/main/java/org/apache/flink/runtime/metrics/groups/GenericValueMetricGroup.java
+++ b/flink-runtime/src/main/java/org/apache/flink/runtime/metrics/groups/GenericValueMetricGroup.java
@@ -51,7 +51,7 @@ public class GenericValueMetricGroup extends GenericMetricGroup {
 	}
 
 	@Override
-	public String getLogicalScope(CharacterFilter filter, char delimiter) {
+	protected String createLogicalScope(CharacterFilter filter, char delimiter) {
 		return parent.getLogicalScope(filter, delimiter);
 	}
 }
diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/metrics/groups/MetricGroupTest.java b/flink-runtime/src/test/java/org/apache/flink/runtime/metrics/groups/MetricGroupTest.java
index 71ae7f1..41d3fc9 100644
--- a/flink-runtime/src/test/java/org/apache/flink/runtime/metrics/groups/MetricGroupTest.java
+++ b/flink-runtime/src/test/java/org/apache/flink/runtime/metrics/groups/MetricGroupTest.java
@@ -19,6 +19,8 @@
 package org.apache.flink.runtime.metrics.groups;
 
 import org.apache.flink.api.common.JobID;
+import org.apache.flink.configuration.ConfigConstants;
+import org.apache.flink.configuration.Configuration;
 import org.apache.flink.metrics.CharacterFilter;
 import org.apache.flink.metrics.Gauge;
 import org.apache.flink.metrics.Metric;
@@ -31,6 +33,7 @@ import org.apache.flink.runtime.metrics.NoOpMetricRegistry;
 import org.apache.flink.runtime.metrics.dump.QueryScopeInfo;
 import org.apache.flink.runtime.metrics.scope.ScopeFormat;
 import org.apache.flink.runtime.metrics.util.DummyCharacterFilter;
+import org.apache.flink.runtime.metrics.util.TestReporter;
 import org.apache.flink.util.AbstractID;
 import org.apache.flink.util.TestLogger;
 
@@ -38,10 +41,13 @@ import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.not;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -216,6 +222,33 @@ public class MetricGroupTest extends TestLogger {
 		assertFalse("Value is present in logical scope.", logicalScope.contains(value));
 	}
 
+	/**
+	 * Verifies that calling {@link AbstractMetricGroup#getLogicalScope(CharacterFilter, char, int)} on {@link GenericValueMetricGroup}
+	 * should ignore value as well.
+	 */
+	@Test
+	public void testLogicalScopeShouldIgnoreValueGroupName() throws Exception {
+		Configuration config = new Configuration();
+		config.setString(ConfigConstants.METRICS_REPORTER_PREFIX + "test." + ConfigConstants.METRICS_REPORTER_CLASS_SUFFIX, TestReporter.class.getName());
+
+		MetricRegistryImpl registry = new MetricRegistryImpl(MetricRegistryConfiguration.fromConfiguration(config));
+		try {
+			GenericMetricGroup root = new GenericMetricGroup(registry, new DummyAbstractMetricGroup(registry), "root");
+
+			String key = "key";
+			String value = "value";
+
+			MetricGroup group = root.addGroup(key, value);
+
+			String logicalScope = ((AbstractMetricGroup) group)
+				.getLogicalScope(new DummyCharacterFilter(), registry.getDelimiter(), 0);
+			assertThat("Key is missing from logical scope.", logicalScope, containsString(key));
+			assertThat("Value is present in logical scope.", logicalScope, not(containsString(value)));
+		} finally {
+			registry.shutdown().get();
+		}
+	}
+
 	@Test
 	public void closedGroupDoesNotRegisterMetrics() {
 		GenericMetricGroup group = new GenericMetricGroup(