[ 
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)

Reply via email to