+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
> >
>
>

Reply via email to