You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2019/01/02 10:27:19 UTC

[GitHub] tony810430 closed pull request #7397: [FLINK-10857] [metrics] GenericValueMetricGroup should always ignore its group name in logical scope

tony810430 closed pull request #7397: [FLINK-10857] [metrics] GenericValueMetricGroup should always ignore its group name in logical scope
URL: https://github.com/apache/flink/pull/7397
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

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 4400b14a10d..4b70986a00f 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
@@ -176,7 +176,7 @@ String getLogicalScope(CharacterFilter filter, char delimiter, int reporterIndex
 		}
 	}
 
-	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 ef8e6e8bee8..41e9a91d386 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 @@ protected void putVariables(Map<String, String> variables) {
 	}
 
 	@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 71ae7f18ecb..9daa72d3610 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.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;
 
@@ -216,6 +219,28 @@ public void testNameCollisionAfterKeyValueGroup() {
 		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() {
+		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));
+		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);
+		assertTrue("Key is missing from logical scope.", logicalScope.contains(key));
+		assertFalse("Value is present in logical scope.", logicalScope.contains(value));
+	}
+
 	@Test
 	public void closedGroupDoesNotRegisterMetrics() {
 		GenericMetricGroup group = new GenericMetricGroup(


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services