[
https://issues.apache.org/jira/browse/FLINK-7692?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16268861#comment-16268861
]
Chesnay Schepler commented on FLINK-7692:
-----------------------------------------
[~tonywei] Those are good points, luckily we can resolve them in a rather nice
way. We have to make one change to the original specification though: Create 2
groups for key-value pairs, instead of one.
For a KeyValue group, the {{name}} on it's own behaves the same way as a
generic group with the same name; it is part of the identifier and logical
scope. Only the value is treated differently, as it isn't part of the logical
scope.
I've create a scaffold implementation (that probably doesn't fully compile... )
here: https://github.com/zentol/flink/tree/7692
This still needs tests, documentation, integration into reporters and we have
to go through all usages of {{addGroup}} to find cases we can now model better.
We need a new class for representing the key group, that behaves pretty much
like a generic metric group:
{code}
public class GenericKeyMetricGroup extends GenericMetricGroup {
GenericKeyMetricGroup(MetricRegistry registry, AbstractMetricGroup
parent, String name) {
super(registry, parent, name);
}
@Override
protected GenericMetricGroup createChildGroup(String name) {
return new GenericValueMetricGroup(registry, this, name);
}
}
{code}
and a another class for representing the value, that a) modifies the variables
and b) is excluded from the logical scope
{code}
public class GenericValueMetricGroup extends GenericMetricGroup {
private String key;
private final String value;
GenericValueMetricGroup(MetricRegistry registry, GenericKeyMetricGroup
parent, String value) {
super(registry, parent, value);
this.key = parent.getGroupName(name -> name);
this.value = value;
}
//
------------------------------------------------------------------------
@Override
protected void putVariables(Map<String, String> variables) {
variables.put(ScopeFormat.asVariable(this.key), value);
}
@Override
public String getLogicalScope(CharacterFilter filter, char delimiter) {
return parent.getLogicalScope(filter, delimiter);
}
}
{code}
This works with (surprisingly) little changes to existing classes; just 2 hooks
to add more variables and to create a different kind of generic child group.
With this, the implementation of {{addGroup(key, value)}} is trivial:
{code}
public MetricGroup addGroup(String key, String value) {
return addGroup(key).addGroup(value);
}
{code}
Now, as to the issues you raised:.
When calling {{addGroup(name, value)}},
* if a group {{name}} already exists, we just continue with whatever group was
there. If the existing group is a key group we get the behavior we want,
otherwise we will add another {{GenericMetricGroup}}. This may result in the
value not being exposed as a proper key-value pair, but it doesn't lead to loss
of information (which we would have if we skipped registering the value). In
any case the metric identifier is the same.
* if a group {{value}} already exists we don't have a problem. Given that the
key group is never exposed to the outside this case can only occur upon
subsequent calls of {{addGroup(name, value)}}, making the latter calls a no-op,
as is the existing behavior.
> 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)
