[ https://issues.apache.org/jira/browse/FLINK-7692?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16278449#comment-16278449 ]
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_r154924645 --- Diff: flink-runtime/src/test/java/org/apache/flink/runtime/metrics/groups/MetricGroupTest.java --- @@ -78,6 +81,134 @@ 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); + assertTrue(subgroup1 instanceof GenericValueMetricGroup); + assertTrue(((AbstractMetricGroup) subgroup1).parent instanceof GenericKeyMetricGroup); + 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); + assertTrue(subgroup2 instanceof GenericValueMetricGroup); + 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); + + assertTrue(((AbstractMetricGroup) subgroup).parent instanceof GenericMetricGroup); + assertTrue(subgroup instanceof GenericMetricGroup); + } + + @Test + public void forbidToCreateGenericValueMetricGroupAfterGenericMetricGroup() { --- End diff -- @zentol This is the case that will create `GenericKeyMetricGroup` in the second group if I always call `addGroup(key, true).addGroup(value, true)` in `AbstractMetricGroup#addGroup(String key, String value)`. So does it happen in [tony810430/flink `@6bec109`].(https://github.com/tony810430/flink/commit/6bec109fc52b7f49727e9440b1ce4170f71354d3) There is also a bug that doesn't verify the `subgroup` should not be `GenericMetricGroup`'s child group. Sorry. > 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)