StephanEwen commented on pull request #13920: URL: https://github.com/apache/flink/pull/13920#issuecomment-723207966
Thanks for implementing this. At a first glance, I think moving the `MetricNames` to `flink-core` does not work too well. The metric names don't have a meaning by themselves. They depend on the Metric Scope (think of it like a package/namespace) to build the full name. Without that, they are like a classname without package. Not unique, not instantiable, not queryable, etc. Just an incomplete fragment. They cannot be used in islolation. That was more coherent in the original place, the runtime metrics package, next to the runtime metric groups, which define the scope and build the full name. This looks like "premature consolidation". We could just keep the runtime metric names as they are, and have a separate class for source metric names. A bit like the config options, where the connector base also defines it's options in it's own module, rather than in a shared core class. The argument for metrics names is even stronger, because of the aforementioned issue that the names alone are incomplete. ---------------------------------------------------------------- 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]
