[ https://issues.apache.org/jira/browse/FLINK-7692?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16279910#comment-16279910 ]
ASF GitHub Bot commented on FLINK-7692: --------------------------------------- Github user tony810430 commented on a diff in the pull request: https://github.com/apache/flink/pull/5115#discussion_r155183669 --- Diff: flink-runtime/src/test/java/org/apache/flink/runtime/metrics/groups/MetricGroupTest.java --- @@ -78,6 +81,138 @@ public void sameGroupOnNameCollision() { assertTrue(subgroup1 == subgroup2); } + @Test + public void createGroupWithUserDefinedVariables() { + GenericMetricGroup group = new GenericMetricGroup( + registry, new DummyAbstractMetricGroup(registry), "somegroup"); + + String keyName = "sometestkey"; + String valueName1 = "sometestvalue1"; + MetricGroup subgroup1 = group.addGroup(keyName, valueName1); + Map<String, String> variables1 = subgroup1.getAllVariables(); + + assertNotNull(subgroup1); + assertEquals(GenericValueMetricGroup.class, subgroup1.getClass()); + assertEquals(GenericKeyMetricGroup.class, ((AbstractMetricGroup) subgroup1).parent.getClass()); + assertTrue(variables1.containsKey(ScopeFormat.asVariable(keyName))); + assertEquals(valueName1, variables1.get(ScopeFormat.asVariable(keyName))); + + String valueName2 = "sometestvalue2"; + MetricGroup subgroup2 = group.addGroup(keyName, valueName2); + Map<String, String> variables2 = subgroup2.getAllVariables(); + + assertNotNull(subgroup2); + assertEquals(GenericValueMetricGroup.class, subgroup2.getClass()); + assertEquals(((AbstractMetricGroup) subgroup1).parent, ((AbstractMetricGroup) subgroup2).parent); + assertTrue(variables2.containsKey(ScopeFormat.asVariable(keyName))); + assertEquals(valueName2, variables2.get(ScopeFormat.asVariable(keyName))); + } + + @Test + public void forbidToCreateGenericKeyMetricGroupAfterGenericKeyMetricGroup() { + GenericMetricGroup group = new GenericMetricGroup( + registry, new DummyAbstractMetricGroup(registry), "somegroup"); + + String keyName = "somekeyname"; + String valueName = "somevaluename"; + group.addGroup(keyName, valueName); + + String keyName2 = "somekeyname2"; + String valueName2 = "somevaluename2"; + MetricGroup subgroup = group.addGroup(keyName).addGroup(keyName2, valueName2); + + // Is is illegal to call `MetricGroup#addGroup(String key, String value)` after `GenericKeyMetricGroup`. + // The behavior will fall back to `group.addGroup(key).addGroup(value)`. + assertEquals(GenericMetricGroup.class, ((AbstractMetricGroup) subgroup).parent.getClass()); --- End diff -- Do you mean it is preferable to replace this with verifying `group.getLogicalScope(ilter, delimiter)` and `group.getAllVariables()`? > Support user-defined variables in Metrics > ----------------------------------------- > > Key: FLINK-7692 > URL: https://issues.apache.org/jira/browse/FLINK-7692 > Project: Flink > Issue Type: Improvement > Components: Metrics > Affects Versions: 1.4.0 > Reporter: Chesnay Schepler > Assignee: Wei-Che Wei > Priority: Minor > Fix For: 1.5.0 > > > Reporters that identify metrics with a set of key-value pairs are currently > limited to the variables defined by Flink, like the taskmanager ID, with > users not being able to supply their own. > This is inconsistent with reporters that use metric identifiers that freely > include user-defined groups constructted via {{MetricGroup#addGroup(String > name)}}. > I propose adding a new method {{MetricGroup#addGroup(String key, String > name)}} that adds a new key-value pair to the {{variables}} map in it's > constructor. When constructing the metric identifier the key should be > included as well, resulting in the same result as when constructing the > metric groups tree via {{group.addGroup(key).addGroup(value)}}. > For this a new {{KeyedGenericMetricGroup}} should be created that resembles > the unkeyed version, with slight modifications to the constructor and > {{getScopeComponents}} method. -- This message was sent by Atlassian JIRA (v6.4.14#64029)