Hi all, I agree getScopeVariables is in line with the existing terminology but I’ve seen that the existing terminology is a bit confusing with regards to how users end up querying these metrics and building alerts/dashboards. I often get the question how do I add a tag or label to my Flink metric, and it would be more intuitive if labels and tags replaced group as a name, to be more in align with popular metric systems.
Best, Mason On Thu, Jul 20, 2023 at 1:41 AM Chesnay Schepler <ches...@apache.org> wrote: > environment variables is a very different though though, that'd just > confuse users. > It's also not a term we've used in the documentation. > getScopeVariables would I guess be most in line with existing terminology. > > On 19/07/2023 18:10, Matthias Pohl wrote: > >> We don't have a well-defined process for breaking behavioral changes. We > >> could consider adding a new method with a different name. > > Introducing a new API to make the behavioral change visible was also the > > suggestion in the deprecation ML thread [1]. getEnvironmentVariables (or > > even getEnvironment) might be a reasonable change. > > > > [1] https://lists.apache.org/thread/vmhzv8fcw2b33pqxp43486owrxbkd5x9 > > > > On Tue, Jul 18, 2023 at 1:10 PM Jing Ge <j...@ververica.com.invalid> > wrote: > > > >> +1 > >> > >> On Tue, Jul 18, 2023 at 12:24 PM Xintong Song <tonysong...@gmail.com> > >> wrote: > >> > >>> +1 > >>> > >>> Best, > >>> > >>> Xintong > >>> > >>> > >>> > >>> On Tue, Jul 18, 2023 at 5:02 PM Chesnay Schepler <ches...@apache.org> > >>> wrote: > >>> > >>>> The FLIP number was changed to 342. > >>>> > >>>> On 18/07/2023 10:56, Chesnay Schepler wrote: > >>>>> MetricGroup#getAllVariables returns all variables associated with the > >>>>> metric, for example: > >>>>> > >>>>> |<job_id> = abcde| > >>>>> |<subtask_index> = ||0| > >>>>> > >>>>> The keys are surrounded by brackets for no particular reason. > >>>>> > >>>>> In virtually every use-case for this method the user is stripping the > >>>>> brackets from keys, as done in: > >>>>> > >>>>> * our datadog reporter: > >>>>> > >> > https://github.com/apache/flink/blob/9c3c8afbd9325b5df8291bd831da2d9f8785b30a/flink-metrics/flink-metrics-datadog/src/main/java/org/apache/flink/metrics/datadog/DatadogHttpReporter.java#L244 > >>>>> < > >> > https://github.com/apache/flink/blob/9c3c8afbd9325b5df8291bd831da2d9f8785b30a/flink-metrics/flink-metrics-datadog/src/main/java/org/apache/flink/metrics/datadog/DatadogHttpReporter.java#L244 > >>>>> * our prometheus reporter (implicitly via a character filter): > >>>>> > >> > https://github.com/apache/flink/blob/9c3c8afbd9325b5df8291bd831da2d9f8785b30a/flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/AbstractPrometheusReporter.java#L236 > >>>>> < > >> > https://github.com/apache/flink/blob/9c3c8afbd9325b5df8291bd831da2d9f8785b30a/flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/AbstractPrometheusReporter.java#L236 > >>>>> * our JMX reporter: > >>>>> > >> > https://github.com/apache/flink/blob/9c3c8afbd9325b5df8291bd831da2d9f8785b30a/flink-metrics/flink-metrics-jmx/src/main/java/org/apache/flink/metrics/jmx/JMXReporter.java#L223 > >>>>> < > >> > https://github.com/apache/flink/blob/9c3c8afbd9325b5df8291bd831da2d9f8785b30a/flink-metrics/flink-metrics-jmx/src/main/java/org/apache/flink/metrics/jmx/JMXReporter.java#L223 > >>>>> > >>>>> I propose to change the method spec and implementation to remove the > >>>>> brackets around keys. > >>>>> > >>>>> For migration purposes it may make sense to add a new method with the > >>>>> new behavior (|getVariables()|) and deprecate the old method. > >>>>> > >>>>> > >>>>> > >> > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=263425202 > >>>> > >
