[GitHub] [flink] 1u0 commented on issue #9867: [FLINK-7958][metrics] Reporters provide default delimiter

[GitBox <gi...@apache.org>]

1u0 commented on issue #9867: [FLINK-7958][metrics] Reporters provide default delimiter
URL: https://github.com/apache/flink/pull/9867#issuecomment-542912934
 
 
   My point is that, in my opinion, this configuration (and also [the configuration for the default delimeter](https://github.com/apache/flink/pull/2219#discussion_r70475544)) is not in demand by users.
   Instead of exposing this parameter outside of Flink to operators of Flink cluster, you can still make it configurable but internally. So when metrics register themselves, they would define which delimeter to use. End users probably won't need to customize this.
   
   While in case of Prometheus reporter, `.` is [invalid character in metric name](https://prometheus.io/docs/concepts/data_model/#metric-names-and-labels). InfluxDB reporter just followed this convention (I don't remember exact reason why I didn't use `.`). It may worth to use `_` for all reporters (for unification, as some common ground), but I understand that this would be an incompatible change.
   
   But thinking more generally, I find that the `(CharacterFilter, delimeter)` pair is just a concrete implementation of more generic api (something like `MetricScopeFormatter` that can become more specific (and different) to some metric reporters.
   
   To summarise, my personal cons against this change are:
    * the configuration is not requested by users. Less configuration exposed by Flink - less things to maintain as public api and more freedom to change implementation later;
    * even if the config is exposed, actual correct values that would work in practice are limited (cannot be any character) and specific to actual metrics system;
    * for some reporters, the scope formatting can required to be more flexible (compared to current implementation based on `CharacterFilter` and `delimeter`).

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services