[jira] [Commented] (FLINK-8080) Remove need for "metrics.reporters"

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-8080?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16285814#comment-16285814
 ] 

ASF GitHub Bot commented on FLINK-8080:
---

Github user zentol commented on the issue:

https://github.com/apache/flink/pull/5099
  
@greghogan I've addressed your comments.


> Remove need for "metrics.reporters"
> ---
>
> Key: FLINK-8080
> URL: https://issues.apache.org/jira/browse/FLINK-8080
> Project: Flink
>  Issue Type: Improvement
>  Components: Configuration, Metrics
>Reporter: Chesnay Schepler
>Assignee: Chesnay Schepler
>Priority: Trivial
> Fix For: 1.5.0
>
>
> Currently, in order to use a reporter one must configure something like this:
> {code}
> metrics.reporters: jmx
> metrics.reporter.jmx.class: ...
> {code}
> It would be neat if users did not have to set {{metrics.reporters}}. We can 
> accomplish this by a scanning the configuration for configuration keys 
> starting with {{metrics.reporter.}} and using the next word as a reporter 
> name.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-8080) Remove need for "metrics.reporters"

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-8080?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16285812#comment-16285812
 ] 

ASF GitHub Bot commented on FLINK-8080:
---

Github user zentol commented on a diff in the pull request:

https://github.com/apache/flink/pull/5099#discussion_r156052191
  
--- Diff: docs/monitoring/metrics.md ---
@@ -329,11 +329,11 @@ or by assigning unique names to jobs and operators.
 Metrics can be exposed to an external system by configuring one or several 
reporters in `conf/flink-conf.yaml`. These
 reporters will be instantiated on each job and task manager when they are 
started.
 
-- `metrics.reporters`: The list of named reporters.
 - `metrics.reporter..`: Generic setting `` for the 
reporter named ``.
 - `metrics.reporter..class`: The reporter class to use for the 
reporter named ``.
 - `metrics.reporter..interval`: The reporter interval to use for the 
reporter named ``.
 - `metrics.reporter..scope.delimiter`: The delimiter to use for the 
identifier (default value use `metrics.scope.delimiter`) for the reporter named 
``.
+- `metrics.reporters`: (optional) An include list for reporters to 
instantiate. By default all configured reporters will be used.
--- End diff --

I guess it can't hurt to explicitly mention "comma-separated", but I don't 
have a strong preference, we have an example after all.


> Remove need for "metrics.reporters"
> ---
>
> Key: FLINK-8080
> URL: https://issues.apache.org/jira/browse/FLINK-8080
> Project: Flink
>  Issue Type: Improvement
>  Components: Configuration, Metrics
>Reporter: Chesnay Schepler
>Assignee: Chesnay Schepler
>Priority: Trivial
> Fix For: 1.5.0
>
>
> Currently, in order to use a reporter one must configure something like this:
> {code}
> metrics.reporters: jmx
> metrics.reporter.jmx.class: ...
> {code}
> It would be neat if users did not have to set {{metrics.reporters}}. We can 
> accomplish this by a scanning the configuration for configuration keys 
> starting with {{metrics.reporter.}} and using the next word as a reporter 
> name.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-8080) Remove need for "metrics.reporters"

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-8080?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16285809#comment-16285809
 ] 

ASF GitHub Bot commented on FLINK-8080:
---

Github user zentol commented on a diff in the pull request:

https://github.com/apache/flink/pull/5099#discussion_r156051769
  
--- Diff: 
flink-runtime/src/main/java/org/apache/flink/runtime/metrics/MetricRegistryConfiguration.java
 ---
@@ -44,7 +48,13 @@
private static volatile MetricRegistryConfiguration 
defaultConfiguration;
 
// regex pattern to split the defined reporters
-   private static final Pattern splitPattern = 
Pattern.compile("\\s*,\\s*");
+   private static final Pattern reporterListPattern = 
Pattern.compile("\\s*,\\s*");
+
+   // regex pattern to extract the name from reporter configuration keys, 
e.g. "rep" from "metrics.reporter.rep.class"
+   private static final Pattern reporterClassPattern = Pattern.compile(
+   Pattern.quote(ConfigConstants.METRICS_REPORTER_PREFIX) +
+   "([\\S&&[^.]]*)\\." +
--- End diff --

of course, lacking documentation is the one gripe I always have when i find 
a regex somewhere, and now i did that myself :(


> Remove need for "metrics.reporters"
> ---
>
> Key: FLINK-8080
> URL: https://issues.apache.org/jira/browse/FLINK-8080
> Project: Flink
>  Issue Type: Improvement
>  Components: Configuration, Metrics
>Reporter: Chesnay Schepler
>Assignee: Chesnay Schepler
>Priority: Trivial
> Fix For: 1.5.0
>
>
> Currently, in order to use a reporter one must configure something like this:
> {code}
> metrics.reporters: jmx
> metrics.reporter.jmx.class: ...
> {code}
> It would be neat if users did not have to set {{metrics.reporters}}. We can 
> accomplish this by a scanning the configuration for configuration keys 
> starting with {{metrics.reporter.}} and using the next word as a reporter 
> name.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-8080) Remove need for "metrics.reporters"

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-8080?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16282444#comment-16282444
 ] 

ASF GitHub Bot commented on FLINK-8080:
---

Github user greghogan commented on a diff in the pull request:

https://github.com/apache/flink/pull/5099#discussion_r155618187
  
--- Diff: docs/monitoring/metrics.md ---
@@ -329,11 +329,11 @@ or by assigning unique names to jobs and operators.
 Metrics can be exposed to an external system by configuring one or several 
reporters in `conf/flink-conf.yaml`. These
 reporters will be instantiated on each job and task manager when they are 
started.
 
-- `metrics.reporters`: The list of named reporters.
 - `metrics.reporter..`: Generic setting `` for the 
reporter named ``.
 - `metrics.reporter..class`: The reporter class to use for the 
reporter named ``.
 - `metrics.reporter..interval`: The reporter interval to use for the 
reporter named ``.
 - `metrics.reporter..scope.delimiter`: The delimiter to use for the 
identifier (default value use `metrics.scope.delimiter`) for the reporter named 
``.
+- `metrics.reporters`: (optional) An include list for reporters to 
instantiate. By default all configured reporters will be used.
--- End diff --

"An include list for reporters" -> "A comma-separated list of reporter 
names"? Not sure if we need to specify "comma-separated".


> Remove need for "metrics.reporters"
> ---
>
> Key: FLINK-8080
> URL: https://issues.apache.org/jira/browse/FLINK-8080
> Project: Flink
>  Issue Type: Improvement
>  Components: Configuration, Metrics
>Reporter: Chesnay Schepler
>Assignee: Chesnay Schepler
>Priority: Trivial
> Fix For: 1.5.0
>
>
> Currently, in order to use a reporter one must configure something like this:
> {code}
> metrics.reporters: jmx
> metrics.reporter.jmx.class: ...
> {code}
> It would be neat if users did not have to set {{metrics.reporters}}. We can 
> accomplish this by a scanning the configuration for configuration keys 
> starting with {{metrics.reporter.}} and using the next word as a reporter 
> name.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-8080) Remove need for "metrics.reporters"

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-8080?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16282443#comment-16282443
 ] 

ASF GitHub Bot commented on FLINK-8080:
---

Github user greghogan commented on a diff in the pull request:

https://github.com/apache/flink/pull/5099#discussion_r155623170
  
--- Diff: 
flink-runtime/src/main/java/org/apache/flink/runtime/metrics/MetricRegistryConfiguration.java
 ---
@@ -44,7 +48,13 @@
private static volatile MetricRegistryConfiguration 
defaultConfiguration;
 
// regex pattern to split the defined reporters
-   private static final Pattern splitPattern = 
Pattern.compile("\\s*,\\s*");
+   private static final Pattern reporterListPattern = 
Pattern.compile("\\s*,\\s*");
+
+   // regex pattern to extract the name from reporter configuration keys, 
e.g. "rep" from "metrics.reporter.rep.class"
+   private static final Pattern reporterClassPattern = Pattern.compile(
+   Pattern.quote(ConfigConstants.METRICS_REPORTER_PREFIX) +
+   "([\\S&&[^.]]*)\\." +
--- End diff --

It would be helpful to document that we are intersecting regex character 
classes.


> Remove need for "metrics.reporters"
> ---
>
> Key: FLINK-8080
> URL: https://issues.apache.org/jira/browse/FLINK-8080
> Project: Flink
>  Issue Type: Improvement
>  Components: Configuration, Metrics
>Reporter: Chesnay Schepler
>Assignee: Chesnay Schepler
>Priority: Trivial
> Fix For: 1.5.0
>
>
> Currently, in order to use a reporter one must configure something like this:
> {code}
> metrics.reporters: jmx
> metrics.reporter.jmx.class: ...
> {code}
> It would be neat if users did not have to set {{metrics.reporters}}. We can 
> accomplish this by a scanning the configuration for configuration keys 
> starting with {{metrics.reporter.}} and using the next word as a reporter 
> name.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-8080) Remove need for "metrics.reporters"

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-8080?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16282445#comment-16282445
 ] 

ASF GitHub Bot commented on FLINK-8080:
---

Github user greghogan commented on a diff in the pull request:

https://github.com/apache/flink/pull/5099#discussion_r155620220
  
--- Diff: 
flink-runtime/src/main/java/org/apache/flink/runtime/metrics/MetricRegistryConfiguration.java
 ---
@@ -108,15 +118,36 @@ public static MetricRegistryConfiguration 
fromConfiguration(Configuration config
delim = '.';
}
 
-   final String definedReporters = 
configuration.getString(MetricOptions.REPORTERS_LIST);
+   Set includedReporters = 
reporterListPattern.splitAsStream(configuration.getString(MetricOptions.REPORTERS_LIST,
 ""))
+   .collect(Collectors.toSet());
+
+   // use a TreeSet to make the reporter order deterministic, 
which is useful for testing
+   Set namedReporters = new TreeSet<>(String::compareTo);
+   // scan entire configuration for "metric.reporter" keys and 
parse individual reporter configurations
+   for (String key : configuration.keySet()) {
+   if 
(key.startsWith(ConfigConstants.METRICS_REPORTER_PREFIX)) {
+   Matcher matcher = 
reporterClassPattern.matcher(key);
+   if (matcher.matches()) {
+   String reporterName = matcher.group(1);
+   if (includedReporters.isEmpty() || 
includedReporters.contains(reporterName)) {
+   if 
(namedReporters.contains(reporterName)) {
+   LOG.warn("Duplicate 
class configuration detected for reporter {}.", reporterName);
+   } else {
+   
namedReporters.add(reporterName);
+   }
+   } else {
+   LOG.info("Excluding reporter 
{}.", reporterName);
--- End diff --

Log the reason for excluding the reporter (not in the reporters list)?


> Remove need for "metrics.reporters"
> ---
>
> Key: FLINK-8080
> URL: https://issues.apache.org/jira/browse/FLINK-8080
> Project: Flink
>  Issue Type: Improvement
>  Components: Configuration, Metrics
>Reporter: Chesnay Schepler
>Assignee: Chesnay Schepler
>Priority: Trivial
> Fix For: 1.5.0
>
>
> Currently, in order to use a reporter one must configure something like this:
> {code}
> metrics.reporters: jmx
> metrics.reporter.jmx.class: ...
> {code}
> It would be neat if users did not have to set {{metrics.reporters}}. We can 
> accomplish this by a scanning the configuration for configuration keys 
> starting with {{metrics.reporter.}} and using the next word as a reporter 
> name.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-8080) Remove need for "metrics.reporters"

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-8080?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16282446#comment-16282446
 ] 

ASF GitHub Bot commented on FLINK-8080:
---

Github user greghogan commented on a diff in the pull request:

https://github.com/apache/flink/pull/5099#discussion_r155625966
  
--- Diff: 
flink-runtime/src/test/java/org/apache/flink/runtime/metrics/MetricRegistryImplTest.java
 ---
@@ -76,8 +76,27 @@ public void testIsShutdown() {
public void testReporterInstantiation() {
Configuration config = new Configuration();
 
+   config.setString(ConfigConstants.METRICS_REPORTER_PREFIX + 
"test." + ConfigConstants.METRICS_REPORTER_CLASS_SUFFIX, 
TestReporter1.class.getName());
+
+   MetricRegistryImpl metricRegistry = new 
MetricRegistryImpl(MetricRegistryConfiguration.fromConfiguration(config));
+
+   assertTrue(metricRegistry.getReporters().size() == 1);
+
+   Assert.assertTrue(TestReporter1.wasOpened);
+
+   metricRegistry.shutdown();
+   }
+
+   /**
+* Verifies that the reporter name list is correctly used to determine 
which reporters should be instantiated.
+*/
+   @Test
+   public void testReporterInclusion() {
+   Configuration config = new Configuration();
+
config.setString(MetricOptions.REPORTERS_LIST, "test");
config.setString(ConfigConstants.METRICS_REPORTER_PREFIX + 
"test." + ConfigConstants.METRICS_REPORTER_CLASS_SUFFIX, 
TestReporter1.class.getName());
+   config.setString(ConfigConstants.METRICS_REPORTER_PREFIX + 
"test1." + ConfigConstants.METRICS_REPORTER_CLASS_SUFFIX, 
TestReporter1.class.getName());
--- End diff --

Use a `TestReporter2` and verify not opened? Do we need both 
`testReporterInstantiation` and `testReporterInclusion`?


> Remove need for "metrics.reporters"
> ---
>
> Key: FLINK-8080
> URL: https://issues.apache.org/jira/browse/FLINK-8080
> Project: Flink
>  Issue Type: Improvement
>  Components: Configuration, Metrics
>Reporter: Chesnay Schepler
>Assignee: Chesnay Schepler
>Priority: Trivial
> Fix For: 1.5.0
>
>
> Currently, in order to use a reporter one must configure something like this:
> {code}
> metrics.reporters: jmx
> metrics.reporter.jmx.class: ...
> {code}
> It would be neat if users did not have to set {{metrics.reporters}}. We can 
> accomplish this by a scanning the configuration for configuration keys 
> starting with {{metrics.reporter.}} and using the next word as a reporter 
> name.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-8080) Remove need for "metrics.reporters"

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-8080?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16276538#comment-16276538
 ] 

ASF GitHub Bot commented on FLINK-8080:
---

Github user zentol commented on a diff in the pull request:

https://github.com/apache/flink/pull/5099#discussion_r154598676
  
--- Diff: 
flink-runtime/src/main/java/org/apache/flink/runtime/metrics/MetricRegistryConfiguration.java
 ---
@@ -43,8 +46,8 @@
 
private static volatile MetricRegistryConfiguration 
defaultConfiguration;
 
-   // regex pattern to split the defined reporters
-   private static final Pattern splitPattern = 
Pattern.compile("\\s*,\\s*");
+   // regex pattern to extract the name from reporter configuration keys, 
e.g. "rep" from "metrics.reporter.rep.class"
+   private static final Pattern configurationPattern = 
Pattern.compile("metrics\\.reporter\\.([\\S&&[^.]]*)\\..*");
--- End diff --

yes


> Remove need for "metrics.reporters"
> ---
>
> Key: FLINK-8080
> URL: https://issues.apache.org/jira/browse/FLINK-8080
> Project: Flink
>  Issue Type: Improvement
>  Components: Configuration, Metrics
>Reporter: Chesnay Schepler
>Assignee: Chesnay Schepler
>Priority: Trivial
> Fix For: 1.5.0
>
>
> Currently, in order to use a reporter one must configure something like this:
> {code}
> metrics.reporters: jmx
> metrics.reporter.jmx.class: ...
> {code}
> It would be neat if users did not have to set {{metrics.reporters}}. We can 
> accomplish this by a scanning the configuration for configuration keys 
> starting with {{metrics.reporter.}} and using the next word as a reporter 
> name.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-8080) Remove need for "metrics.reporters"

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-8080?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16276476#comment-16276476
 ] 

ASF GitHub Bot commented on FLINK-8080:
---

Github user zentol commented on the issue:

https://github.com/apache/flink/pull/5099
  
We could also make `metrics.reporters` an optional list of reporters to 
enable; by default we include all reporters we can find. This would be more 
user-friendly since you wouldn't have to modify the configuration of a reporter.


> Remove need for "metrics.reporters"
> ---
>
> Key: FLINK-8080
> URL: https://issues.apache.org/jira/browse/FLINK-8080
> Project: Flink
>  Issue Type: Improvement
>  Components: Configuration, Metrics
>Reporter: Chesnay Schepler
>Assignee: Chesnay Schepler
>Priority: Trivial
> Fix For: 1.5.0
>
>
> Currently, in order to use a reporter one must configure something like this:
> {code}
> metrics.reporters: jmx
> metrics.reporter.jmx.class: ...
> {code}
> It would be neat if users did not have to set {{metrics.reporters}}. We can 
> accomplish this by a scanning the configuration for configuration keys 
> starting with {{metrics.reporter.}} and using the next word as a reporter 
> name.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-8080) Remove need for "metrics.reporters"

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-8080?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16276465#comment-16276465
 ] 

ASF GitHub Bot commented on FLINK-8080:
---

Github user zentol commented on the issue:

https://github.com/apache/flink/pull/5099
  
only checking for the class makes sense.


> Remove need for "metrics.reporters"
> ---
>
> Key: FLINK-8080
> URL: https://issues.apache.org/jira/browse/FLINK-8080
> Project: Flink
>  Issue Type: Improvement
>  Components: Configuration, Metrics
>Reporter: Chesnay Schepler
>Assignee: Chesnay Schepler
>Priority: Trivial
> Fix For: 1.5.0
>
>
> Currently, in order to use a reporter one must configure something like this:
> {code}
> metrics.reporters: jmx
> metrics.reporter.jmx.class: ...
> {code}
> It would be neat if users did not have to set {{metrics.reporters}}. We can 
> accomplish this by a scanning the configuration for configuration keys 
> starting with {{metrics.reporter.}} and using the next word as a reporter 
> name.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-8080) Remove need for "metrics.reporters"

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-8080?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16274227#comment-16274227
 ] 

ASF GitHub Bot commented on FLINK-8080:
---

Github user zentol commented on a diff in the pull request:

https://github.com/apache/flink/pull/5099#discussion_r154315071
  
--- Diff: 
flink-core/src/main/java/org/apache/flink/configuration/MetricOptions.java ---
@@ -25,20 +25,9 @@
 public class MetricOptions {
 
/**
-* The list of named reporters. Names are defined here and per-reporter 
configs
-* are given with the reporter config prefix and the reporter name.
-*
-* Example:
-* {@code
-* metrics.reporters = foo, bar
-*
-* metrics.reporter.foo.class = 
org.apache.flink.metrics.reporter.JMXReporter
-* metrics.reporter.foo.interval = 10
-*
-* metrics.reporter.bar.class = 
org.apache.flink.metrics.graphite.GraphiteReporter
-* metrics.reporter.bar.port = 1337
-* }
+* @deprecated This configuration key has no effect.
 */
+   @Deprecated
--- End diff --

This option should only be used by Flink in the setup of reporters, and 
tests/user-code to configure reporters. All these cases should continue to work 
even with the option being a no-op. The only case that may now fail if some 
test was configuring this option but no actual reporter, but well, that will 
fail regardless of whether we keep the option or not.

I'd rather keep it to not cause compilation failures.


> Remove need for "metrics.reporters"
> ---
>
> Key: FLINK-8080
> URL: https://issues.apache.org/jira/browse/FLINK-8080
> Project: Flink
>  Issue Type: Improvement
>  Components: Configuration, Metrics
>Reporter: Chesnay Schepler
>Assignee: Chesnay Schepler
>Priority: Trivial
> Fix For: 1.5.0
>
>
> Currently, in order to use a reporter one must configure something like this:
> {code}
> metrics.reporters: jmx
> metrics.reporter.jmx.class: ...
> {code}
> It would be neat if users did not have to set {{metrics.reporters}}. We can 
> accomplish this by a scanning the configuration for configuration keys 
> starting with {{metrics.reporter.}} and using the next word as a reporter 
> name.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-8080) Remove need for "metrics.reporters"

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-8080?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16272880#comment-16272880
 ] 

ASF GitHub Bot commented on FLINK-8080:
---

Github user greghogan commented on a diff in the pull request:

https://github.com/apache/flink/pull/5099#discussion_r154122694
  
--- Diff: 
flink-core/src/main/java/org/apache/flink/configuration/MetricOptions.java ---
@@ -25,20 +25,9 @@
 public class MetricOptions {
 
/**
-* The list of named reporters. Names are defined here and per-reporter 
configs
-* are given with the reporter config prefix and the reporter name.
-*
-* Example:
-* {@code
-* metrics.reporters = foo, bar
-*
-* metrics.reporter.foo.class = 
org.apache.flink.metrics.reporter.JMXReporter
-* metrics.reporter.foo.interval = 10
-*
-* metrics.reporter.bar.class = 
org.apache.flink.metrics.graphite.GraphiteReporter
-* metrics.reporter.bar.port = 1337
-* }
+* @deprecated This configuration key has no effect.
 */
+   @Deprecated
--- End diff --

Have we removed all usages of this key? Can we simply delete the option? 
The behavior of any reporters relying on this field will be broken so is it 
better to change the `@PublicEvolving` API?


> Remove need for "metrics.reporters"
> ---
>
> Key: FLINK-8080
> URL: https://issues.apache.org/jira/browse/FLINK-8080
> Project: Flink
>  Issue Type: Improvement
>  Components: Configuration, Metrics
>Reporter: Chesnay Schepler
>Assignee: Chesnay Schepler
>Priority: Trivial
> Fix For: 1.5.0
>
>
> Currently, in order to use a reporter one must configure something like this:
> {code}
> metrics.reporters: jmx
> metrics.reporter.jmx.class: ...
> {code}
> It would be neat if users did not have to set {{metrics.reporters}}. We can 
> accomplish this by a scanning the configuration for configuration keys 
> starting with {{metrics.reporter.}} and using the next word as a reporter 
> name.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-8080) Remove need for "metrics.reporters"

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-8080?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16272879#comment-16272879
 ] 

ASF GitHub Bot commented on FLINK-8080:
---

Github user greghogan commented on a diff in the pull request:

https://github.com/apache/flink/pull/5099#discussion_r154121821
  
--- Diff: 
flink-runtime/src/main/java/org/apache/flink/runtime/metrics/MetricRegistryConfiguration.java
 ---
@@ -43,8 +46,8 @@
 
private static volatile MetricRegistryConfiguration 
defaultConfiguration;
 
-   // regex pattern to split the defined reporters
-   private static final Pattern splitPattern = 
Pattern.compile("\\s*,\\s*");
+   // regex pattern to extract the name from reporter configuration keys, 
e.g. "rep" from "metrics.reporter.rep.class"
+   private static final Pattern configurationPattern = 
Pattern.compile("metrics\\.reporter\\.([\\S&&[^.]]*)\\..*");
--- End diff --

Can we `Pattern.quote(ConfigConstants.METRICS_REPORTER_PREFIX)`?


> Remove need for "metrics.reporters"
> ---
>
> Key: FLINK-8080
> URL: https://issues.apache.org/jira/browse/FLINK-8080
> Project: Flink
>  Issue Type: Improvement
>  Components: Configuration, Metrics
>Reporter: Chesnay Schepler
>Assignee: Chesnay Schepler
>Priority: Trivial
> Fix For: 1.5.0
>
>
> Currently, in order to use a reporter one must configure something like this:
> {code}
> metrics.reporters: jmx
> metrics.reporter.jmx.class: ...
> {code}
> It would be neat if users did not have to set {{metrics.reporters}}. We can 
> accomplish this by a scanning the configuration for configuration keys 
> starting with {{metrics.reporter.}} and using the next word as a reporter 
> name.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-8080) Remove need for "metrics.reporters"

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-8080?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16270649#comment-16270649
 ] 

ASF GitHub Bot commented on FLINK-8080:
---

GitHub user zentol opened a pull request:

https://github.com/apache/flink/pull/5099

[FLINK-8080][metrics] Remove need for "metrics.reporters" config key

## What is the purpose of the change

This PR simplifies the reporter configuration by no longer requiring users 
to explicitly set a list of reporter names via `metrics.reporters`. Instead, 
the names are automatically determined based on 
`metrics.reporter..` keys found in the configuration.

## Brief change log

* modify reporter configuration parsing to detect reporter names
* deprecate `MetricOptions.REPORTERS_LIST`
  * remove all usages
  * remove all references from documentation

## Verifying this change

This change is already covered by existing tests in the various reporter 
modules that configure a reporter.
See all tests where `MetricOptions.REPORTERS_LIST` was removed.

## Does this pull request potentially affect one of the following parts:

  - Dependencies (does it add or upgrade a dependency): (no)
  - The public API, i.e., is any changed class annotated with 
`@Public(Evolving)`: (no)
  - The serializers: (no)
  - The runtime per-record code paths (performance sensitive): (no)
  - Anything that affects deployment or recovery: JobManager (and its 
components), Checkpointing, Yarn/Mesos, ZooKeeper: (no)
  - The S3 file system connector: (no)

## Documentation

  - Does this pull request introduce a new feature? (yes)
  - If yes, how is the feature documented? (docs)


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/zentol/flink 8080

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/flink/pull/5099.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #5099


commit 9516d60ffa87ff2d3a42ca0379e95ada0f42b61d
Author: zentol 
Date:   2017-11-15T11:56:30Z

[FLINK-8080][metrics] Remove need for "metrics.reporters" config key




> Remove need for "metrics.reporters"
> ---
>
> Key: FLINK-8080
> URL: https://issues.apache.org/jira/browse/FLINK-8080
> Project: Flink
>  Issue Type: Improvement
>  Components: Configuration, Metrics
>Reporter: Chesnay Schepler
>Assignee: Chesnay Schepler
>Priority: Trivial
> Fix For: 1.5.0
>
>
> Currently, in order to use a reporter one must configure something like this:
> {code}
> metrics.reporters: jmx
> metrics.reporter.jmx.class: ...
> {code}
> It would be neat if users did not have to set {{metrics.reporters}}. We can 
> accomplish this by a scanning the configuration for configuration keys 
> starting with {{metrics.reporter.}} and using the next word as a reporter 
> name.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)