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

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

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

    https://github.com/apache/flink/pull/4586#discussion_r135764351
  
    --- Diff: 
flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusReporter.java
 ---
    @@ -84,21 +84,30 @@ static String replaceInvalidChars(final String input) {
     
        @Override
        public void open(MetricConfig config) {
    -           int port = config.getInteger(ARG_PORT, DEFAULT_PORT);
    -           LOG.info("Using port {}.", port);
    -           prometheusEndpoint = new PrometheusEndpoint(port);
    -           try {
    -                   prometheusEndpoint.start(NanoHTTPD.SOCKET_READ_TIMEOUT, 
true);
    -           } catch (IOException e) {
    -                   final String msg = "Could not start PrometheusEndpoint 
on port " + port;
    -                   LOG.warn(msg, e);
    -                   throw new RuntimeException(msg, e);
    +           String portsConfig = config.getString(ARG_PORT, DEFAULT_PORT);
    +
    +           if (portsConfig != null) {
    --- End diff --
    
    This check isn't needed. If we keep it we should also throw an exception if 
portsConfig is null as otherwise we're hitting an NPE later on since httpServer 
is still null.


> PrometheusReporter improvements
> -------------------------------
>
>                 Key: FLINK-7502
>                 URL: https://issues.apache.org/jira/browse/FLINK-7502
>             Project: Flink
>          Issue Type: Improvement
>          Components: Metrics
>    Affects Versions: 1.4.0
>            Reporter: Maximilian Bode
>            Assignee: Maximilian Bode
>            Priority: Minor
>
> * do not throw exceptions on metrics being registered for second time
> * allow port ranges for setups where multiple reporters are on same host 
> (e.g. one TaskManager and one JobManager)
> * do not use nanohttpd anymore, there is now a minimal http server included 
> in [Prometheus JVM client|https://github.com/prometheus/client_java]



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

Reply via email to