zentol commented on pull request #14510:
URL: https://github.com/apache/flink/pull/14510#issuecomment-751770942
well...this isn't quite the direction I was going for.
The problem I have with it is that we have to modify every single reporter.
This has a few implications:
- this behavior is now essentially opt-in, which ultimately means that
user-defined reporters are likely to behave differently, still suffering from
the problem at hand
- duplicate logic in reporters
- which you reduce via inheritance, but:
- it is now hard-wired to go through the `open()` method, which is
essentially legacy code (because new reporters should use factories and pass
everything via the constructor, but you can't force that in `AbstractReporter`
because legacy reporters may still be around)
- it is super easy to miss a call to `super#filterCharacters`
- forces reporters to now pass themselves as the `CharacterFilter`,
which is a pattern that we should have never introduced
- if the new filter actually modifies values then the caching mechanism in
the metric groups is rendered pointless (this was admittedly already the case
for _some_ reporters, but it is something we want to change in FLINK-14410)
I can just echo my previous statement in the JIRA; we should modify the
FrontMetricGroup instead to wrap the given filter (or introduce one where the
filter is null) doing the replacement, and add a ( #getAllVariables()) variant
that accepts a filter.
Then this behavior will be consistent for every reporter, without having to
touch any of them.
----------------------------------------------------------------
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]
