zentol commented on a change in pull request #14510:
URL: https://github.com/apache/flink/pull/14510#discussion_r555126591
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/metrics/groups/AbstractMetricGroup.java
##########
@@ -137,11 +156,18 @@ public AbstractMetricGroup(MetricRegistry registry,
String[] scope, A parent) {
// if no variables are excluded (which is the default!) we re-use the
general variables map
// to save space
return internalGetAllVariables(
- excludedVariables.isEmpty() ? 0 : reporterIndex,
excludedVariables);
+ filter,
+ excludedVariables.isEmpty() ? 0 : reporterIndex,
+ excludedVariables,
+ delimiter);
}
private Map<String, String> internalGetAllVariables(
- int cachingIndex, Set<String> excludedVariables) {
+ CharacterFilter filter,
+ int cachingIndex,
+ Set<String> excludedVariables,
+ char delimiter) {
+ CharacterFilter newFilter = wrapWithDefaultFilter(filter, delimiter);
Review comment:
It shouldn't be a problem for `getMetricIdentifier()`; this method only
relies on `this.scopeComponents` which is constant and does not fetch anything
from parent groups.
Maybe I haven't explained this well.
Let's say the delimiter is `-`, the global default delimiter (returned by
`registry.getDelimiter()`) is `.`, and we have a variable defined in a parent
group called `-aaa.`.
The result we want is `_aaa.`.
The problem is you retrieved this variable in the first place via
`parent.getAllVariables()`. This one doesn't know about your filter, and uses
the delimiter returned by `registry.getDelimiter()`. It will apply this filter,
resulting in `-aaa_`, and when your filter is applied we end up with `_aaa_`,
which is not what we want.
The easiest way I see to solve is to only wrap the filter if the
cachingIndexis greater than 0.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
