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


Reply via email to