Github user zentol commented on a diff in the pull request: https://github.com/apache/flink/pull/5115#discussion_r155185291 --- 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 -- exactly. ð
---