You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by lamber-ken <gi...@git.apache.org> on 2018/04/16 13:52:43 UTC

[GitHub] flink pull request #5857: [FLINK-9180][METRICS] add prometheus pushgateway r...

GitHub user lamber-ken opened a pull request:

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

    [FLINK-9180][METRICS] add prometheus pushgateway reporter

    ## What is the purpose of the change
    This pull request makes flink system can send metrics to prometheus via pushgateway. when using `yarn-cluster` model, it's useful.
    
    ## Brief change log
    
      - Add prometheus pushgateway repoter
      - Restructure the code of the promethues reporter part
    
    ## Verifying this change
    
    This change is already covered by existing tests. [prometheus test](https://github.com/apache/flink/tree/master/flink-metrics/flink-metrics-prometheus/src/test/java/org/apache/flink/metrics/prometheus)
    
    ## Does this pull request potentially affect one of the following parts:
    
      - Dependencies (does it add or upgrade a dependency): (yes)
      - 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? (JavaDocs)


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

    $ git pull https://github.com/lamber-ken/flink master

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

    https://github.com/apache/flink/pull/5857.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 #5857
    
----
commit a3503a5d08e4d02d6cf38d656e2697d3b1197cf1
Author: lamber-ken <!...@...>
Date:   2018-04-16T13:49:56Z

    add prometheus pushgateway reporter

----


---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by lamber-ken <gi...@git.apache.org>.
Github user lamber-ken commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5857#discussion_r191336445
  
    --- Diff: flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusPushGatewayReporter.java ---
    @@ -0,0 +1,79 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.metrics.prometheus;
    +
    +import org.apache.flink.annotation.PublicEvolving;
    +import org.apache.flink.metrics.Metric;
    +import org.apache.flink.metrics.MetricConfig;
    +import org.apache.flink.metrics.reporter.MetricReporter;
    +import org.apache.flink.metrics.reporter.Scheduled;
    +import org.apache.flink.util.AbstractID;
    +
    +import io.prometheus.client.CollectorRegistry;
    +import io.prometheus.client.exporter.PushGateway;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * /**
    + * {@link MetricReporter} that exports {@link Metric Metrics} via Prometheus Pushgateway.
    + */
    +@PublicEvolving
    +public class PrometheusPushGatewayReporter extends AbstractPrometheusReporter implements Scheduled {
    +	private static final Logger LOG = LoggerFactory.getLogger(PrometheusPushGatewayReporter.class);
    +
    +	public static final String ARG_HOST = "host";
    +	public static final String ARG_PORT = "port";
    +
    +	public static final char JOB_NAME_SEPARATOR = '-';
    +	public static final String JOB_NAME_PREFIX = "flink" + JOB_NAME_SEPARATOR;
    +
    +	private PushGateway pushGateway;
    +	private final String jobName;
    +
    +	public PrometheusPushGatewayReporter() {
    +		String random = new AbstractID().toString();
    +		jobName = JOB_NAME_PREFIX + random;
    --- End diff --
    
    so, it's better to make the prefix of jobname to distinguish different clusters, and each flink clusters use the same prefix


---

[GitHub] flink issue #5857: [FLINK-9187][METRICS] add prometheus pushgateway reporter

Posted by lamber-ken <gi...@git.apache.org>.
Github user lamber-ken commented on the issue:

    https://github.com/apache/flink/pull/5857
  
    @zentol, @tillrohrmann, cc
    
    I don't think there is a relationship between `FLINK-9543` and `FLINK-9187`.
    At present, the focus of our discussion is on the job name of pushgateway.
    My point is that there is no need to use JMID or TMID to form the job name of pushgateway, because the job name of pushgateway is just used to distinguish different services(like TM or JM and), and prevent the same metrics from being covered, it doesn't make sense for metrics data.
    
    A minimal reproducible example, two flink cluster, each cluster includes one JM and three TMs.
    ![image](https://user-images.githubusercontent.com/20113411/41583819-31d803bc-73d8-11e8-8d76-fecc9801638c.png)
    ![image](https://user-images.githubusercontent.com/20113411/41584075-e7a176e2-73d8-11e8-8174-dab68f4088ed.png)
    ![image](https://user-images.githubusercontent.com/20113411/41584093-f70c85e0-73d8-11e8-8aa8-46745db879aa.png)
    
    
    
    



---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5857#discussion_r191560994
  
    --- Diff: flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusPushGatewayReporter.java ---
    @@ -0,0 +1,79 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.metrics.prometheus;
    +
    +import org.apache.flink.annotation.PublicEvolving;
    +import org.apache.flink.metrics.Metric;
    +import org.apache.flink.metrics.MetricConfig;
    +import org.apache.flink.metrics.reporter.MetricReporter;
    +import org.apache.flink.metrics.reporter.Scheduled;
    +import org.apache.flink.util.AbstractID;
    +
    +import io.prometheus.client.CollectorRegistry;
    +import io.prometheus.client.exporter.PushGateway;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * /**
    + * {@link MetricReporter} that exports {@link Metric Metrics} via Prometheus Pushgateway.
    + */
    +@PublicEvolving
    +public class PrometheusPushGatewayReporter extends AbstractPrometheusReporter implements Scheduled {
    +	private static final Logger LOG = LoggerFactory.getLogger(PrometheusPushGatewayReporter.class);
    +
    +	public static final String ARG_HOST = "host";
    +	public static final String ARG_PORT = "port";
    +
    +	public static final char JOB_NAME_SEPARATOR = '-';
    +	public static final String JOB_NAME_PREFIX = "flink" + JOB_NAME_SEPARATOR;
    +
    +	private PushGateway pushGateway;
    +	private final String jobName;
    +
    +	public PrometheusPushGatewayReporter() {
    +		String random = new AbstractID().toString();
    +		jobName = JOB_NAME_PREFIX + random;
    --- End diff --
    
    What would happen if every taskmanager/jobmanager has it's own pushgateway?


---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by lamber-ken <gi...@git.apache.org>.
Github user lamber-ken commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5857#discussion_r192630255
  
    --- Diff: flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusPushGatewayReporter.java ---
    @@ -0,0 +1,79 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.metrics.prometheus;
    +
    +import org.apache.flink.annotation.PublicEvolving;
    +import org.apache.flink.metrics.Metric;
    +import org.apache.flink.metrics.MetricConfig;
    +import org.apache.flink.metrics.reporter.MetricReporter;
    +import org.apache.flink.metrics.reporter.Scheduled;
    +import org.apache.flink.util.AbstractID;
    +
    +import io.prometheus.client.CollectorRegistry;
    +import io.prometheus.client.exporter.PushGateway;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * /**
    + * {@link MetricReporter} that exports {@link Metric Metrics} via Prometheus Pushgateway.
    + */
    +@PublicEvolving
    +public class PrometheusPushGatewayReporter extends AbstractPrometheusReporter implements Scheduled {
    +	private static final Logger LOG = LoggerFactory.getLogger(PrometheusPushGatewayReporter.class);
    +
    +	public static final String ARG_HOST = "host";
    +	public static final String ARG_PORT = "port";
    +
    +	public static final char JOB_NAME_SEPARATOR = '-';
    +	public static final String JOB_NAME_PREFIX = "flink" + JOB_NAME_SEPARATOR;
    +
    +	private PushGateway pushGateway;
    +	private final String jobName;
    +
    +	public PrometheusPushGatewayReporter() {
    +		String random = new AbstractID().toString();
    +		jobName = JOB_NAME_PREFIX + random;
    --- End diff --
    
    @zentol, please cc, and what else do I need to do?  :+1: 


---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by lamber-ken <gi...@git.apache.org>.
Github user lamber-ken commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5857#discussion_r191351687
  
    --- Diff: flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusPushGatewayReporter.java ---
    @@ -0,0 +1,79 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.metrics.prometheus;
    +
    +import org.apache.flink.annotation.PublicEvolving;
    +import org.apache.flink.metrics.Metric;
    +import org.apache.flink.metrics.MetricConfig;
    +import org.apache.flink.metrics.reporter.MetricReporter;
    +import org.apache.flink.metrics.reporter.Scheduled;
    +import org.apache.flink.util.AbstractID;
    +
    +import io.prometheus.client.CollectorRegistry;
    +import io.prometheus.client.exporter.PushGateway;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * /**
    + * {@link MetricReporter} that exports {@link Metric Metrics} via Prometheus Pushgateway.
    + */
    +@PublicEvolving
    +public class PrometheusPushGatewayReporter extends AbstractPrometheusReporter implements Scheduled {
    +	private static final Logger LOG = LoggerFactory.getLogger(PrometheusPushGatewayReporter.class);
    +
    +	public static final String ARG_HOST = "host";
    +	public static final String ARG_PORT = "port";
    +
    +	public static final char JOB_NAME_SEPARATOR = '-';
    +	public static final String JOB_NAME_PREFIX = "flink" + JOB_NAME_SEPARATOR;
    +
    +	private PushGateway pushGateway;
    +	private final String jobName;
    +
    +	public PrometheusPushGatewayReporter() {
    +		String random = new AbstractID().toString();
    +		jobName = JOB_NAME_PREFIX + random;
    --- End diff --
    
    yes


---

[GitHub] flink issue #5857: [FLINK-9180][METRICS] add prometheus pushgateway reporter

Posted by lamber-ken <gi...@git.apache.org>.
Github user lamber-ken commented on the issue:

    https://github.com/apache/flink/pull/5857
  
    wrong place? can you point it out, I don't know. thank you


---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by lamber-ken <gi...@git.apache.org>.
Github user lamber-ken commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5857#discussion_r191176434
  
    --- Diff: flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusPushGatewayReporter.java ---
    @@ -0,0 +1,79 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.metrics.prometheus;
    +
    +import org.apache.flink.annotation.PublicEvolving;
    +import org.apache.flink.metrics.Metric;
    +import org.apache.flink.metrics.MetricConfig;
    +import org.apache.flink.metrics.reporter.MetricReporter;
    +import org.apache.flink.metrics.reporter.Scheduled;
    +import org.apache.flink.util.AbstractID;
    +
    +import io.prometheus.client.CollectorRegistry;
    +import io.prometheus.client.exporter.PushGateway;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * /**
    + * {@link MetricReporter} that exports {@link Metric Metrics} via Prometheus Pushgateway.
    + */
    +@PublicEvolving
    +public class PrometheusPushGatewayReporter extends AbstractPrometheusReporter implements Scheduled {
    --- End diff --
    
    ok, then I'll improve the doc


---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5857#discussion_r195643808
  
    --- Diff: flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusPushGatewayReporter.java ---
    @@ -0,0 +1,79 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.metrics.prometheus;
    +
    +import org.apache.flink.annotation.PublicEvolving;
    +import org.apache.flink.metrics.Metric;
    +import org.apache.flink.metrics.MetricConfig;
    +import org.apache.flink.metrics.reporter.MetricReporter;
    +import org.apache.flink.metrics.reporter.Scheduled;
    +import org.apache.flink.util.AbstractID;
    +
    +import io.prometheus.client.CollectorRegistry;
    +import io.prometheus.client.exporter.PushGateway;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * /**
    + * {@link MetricReporter} that exports {@link Metric Metrics} via Prometheus Pushgateway.
    + */
    +@PublicEvolving
    +public class PrometheusPushGatewayReporter extends AbstractPrometheusReporter implements Scheduled {
    +	private static final Logger LOG = LoggerFactory.getLogger(PrometheusPushGatewayReporter.class);
    +
    +	public static final String ARG_HOST = "host";
    +	public static final String ARG_PORT = "port";
    +
    +	public static final char JOB_NAME_SEPARATOR = '-';
    +	public static final String JOB_NAME_PREFIX = "flink" + JOB_NAME_SEPARATOR;
    +
    +	private PushGateway pushGateway;
    +	private final String jobName;
    +
    +	public PrometheusPushGatewayReporter() {
    +		String random = new AbstractID().toString();
    +		jobName = JOB_NAME_PREFIX + random;
    --- End diff --
    
    I just don't see a reason to rush this. There's a known issue we have to fix and the PR is not at risk of becoming outdated in the mean-time. If the argument is that "other people might start using it already" then we may just end up unnecessarily breaking their setup before the release.


---

[GitHub] flink issue #5857: [FLINK-9187][METRICS] add prometheus pushgateway reporter

Posted by lamber-ken <gi...@git.apache.org>.
Github user lamber-ken commented on the issue:

    https://github.com/apache/flink/pull/5857
  
    @zentol, I see. 
    I just want to update the forked project, but I'm not familiar with the process.
    I'm learning this process, so try to delete then forked again. :smile: 



---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5857#discussion_r191174704
  
    --- Diff: flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/AbstractPrometheusReporter.java ---
    @@ -0,0 +1,283 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.metrics.prometheus;
    +
    +import org.apache.flink.annotation.PublicEvolving;
    +import org.apache.flink.annotation.VisibleForTesting;
    +import org.apache.flink.metrics.CharacterFilter;
    +import org.apache.flink.metrics.Counter;
    +import org.apache.flink.metrics.Gauge;
    +import org.apache.flink.metrics.Histogram;
    +import org.apache.flink.metrics.Meter;
    +import org.apache.flink.metrics.Metric;
    +import org.apache.flink.metrics.MetricConfig;
    +import org.apache.flink.metrics.MetricGroup;
    +import org.apache.flink.metrics.reporter.MetricReporter;
    +import org.apache.flink.runtime.metrics.groups.AbstractMetricGroup;
    +import org.apache.flink.runtime.metrics.groups.FrontMetricGroup;
    +
    +import io.prometheus.client.Collector;
    +import io.prometheus.client.CollectorRegistry;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import java.util.AbstractMap;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.Collections;
    +import java.util.HashMap;
    +import java.util.LinkedList;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.regex.Pattern;
    +
    +
    +/**
    + * base prometheus reporter for prometheus metrics.
    + */
    +@PublicEvolving
    +public abstract class AbstractPrometheusReporter implements MetricReporter {
    +
    +	private static final Logger LOG = LoggerFactory.getLogger(AbstractPrometheusReporter.class);
    --- End diff --
    
    it is a bit icky to have a separate loggers in the base and sub classes. Define the logger as below instead and update all logger usages.
    ```
    protected final Logger log = LoggerFactory.getLogger(getClass());
    ```


---

[GitHub] flink issue #5857: [FLINK-9180][METRICS] add prometheus pushgateway reporter

Posted by lamber-ken <gi...@git.apache.org>.
Github user lamber-ken commented on the issue:

    https://github.com/apache/flink/pull/5857
  
    @zhangminglei ,can you cc [FLINK-9187](https://issues.apache.org/jira/browse/FLINK-9187)


---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5857#discussion_r191174014
  
    --- Diff: flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusPushGatewayReporter.java ---
    @@ -0,0 +1,79 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.metrics.prometheus;
    +
    +import org.apache.flink.annotation.PublicEvolving;
    +import org.apache.flink.metrics.Metric;
    +import org.apache.flink.metrics.MetricConfig;
    +import org.apache.flink.metrics.reporter.MetricReporter;
    +import org.apache.flink.metrics.reporter.Scheduled;
    +import org.apache.flink.util.AbstractID;
    +
    +import io.prometheus.client.CollectorRegistry;
    +import io.prometheus.client.exporter.PushGateway;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * /**
    + * {@link MetricReporter} that exports {@link Metric Metrics} via Prometheus Pushgateway.
    + */
    +@PublicEvolving
    +public class PrometheusPushGatewayReporter extends AbstractPrometheusReporter implements Scheduled {
    +	private static final Logger LOG = LoggerFactory.getLogger(PrometheusPushGatewayReporter.class);
    +
    +	public static final String ARG_HOST = "host";
    +	public static final String ARG_PORT = "port";
    +
    +	public static final char JOB_NAME_SEPARATOR = '-';
    +	public static final String JOB_NAME_PREFIX = "flink" + JOB_NAME_SEPARATOR;
    +
    +	private PushGateway pushGateway;
    +	private final String jobName;
    +
    +	public PrometheusPushGatewayReporter() {
    +		String random = new AbstractID().toString();
    +		jobName = JOB_NAME_PREFIX + random;
    --- End diff --
    
    The jobname should be configurable


---

[GitHub] flink issue #5857: [FLINK-9180][METRICS] add prometheus pushgateway reporter

Posted by lamber-ken <gi...@git.apache.org>.
Github user lamber-ken commented on the issue:

    https://github.com/apache/flink/pull/5857
  
    ok, I see. I'll close the PR.
    use `FLINK-9189`, ok?


---

[GitHub] flink issue #5857: [FLINK-9180][METRICS] add prometheus pushgateway reporter

Posted by lamber-ken <gi...@git.apache.org>.
Github user lamber-ken commented on the issue:

    https://github.com/apache/flink/pull/5857
  
    ok, I see, thanks a lot. 
    I contributed to [ClickHouse](https://github.com/yandex/ClickHouse) before, the submission process is different.


---

[GitHub] flink pull request #5857: [FLINK-9180][METRICS] add prometheus pushgateway r...

Posted by lamber-ken <gi...@git.apache.org>.
Github user lamber-ken closed the pull request at:

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


---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by lamber-ken <gi...@git.apache.org>.
Github user lamber-ken commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5857#discussion_r191647381
  
    --- Diff: flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusPushGatewayReporter.java ---
    @@ -0,0 +1,79 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.metrics.prometheus;
    +
    +import org.apache.flink.annotation.PublicEvolving;
    +import org.apache.flink.metrics.Metric;
    +import org.apache.flink.metrics.MetricConfig;
    +import org.apache.flink.metrics.reporter.MetricReporter;
    +import org.apache.flink.metrics.reporter.Scheduled;
    +import org.apache.flink.util.AbstractID;
    +
    +import io.prometheus.client.CollectorRegistry;
    +import io.prometheus.client.exporter.PushGateway;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * /**
    + * {@link MetricReporter} that exports {@link Metric Metrics} via Prometheus Pushgateway.
    + */
    +@PublicEvolving
    +public class PrometheusPushGatewayReporter extends AbstractPrometheusReporter implements Scheduled {
    +	private static final Logger LOG = LoggerFactory.getLogger(PrometheusPushGatewayReporter.class);
    +
    +	public static final String ARG_HOST = "host";
    +	public static final String ARG_PORT = "port";
    +
    +	public static final char JOB_NAME_SEPARATOR = '-';
    +	public static final String JOB_NAME_PREFIX = "flink" + JOB_NAME_SEPARATOR;
    +
    +	private PushGateway pushGateway;
    +	private final String jobName;
    +
    +	public PrometheusPushGatewayReporter() {
    +		String random = new AbstractID().toString();
    +		jobName = JOB_NAME_PREFIX + random;
    --- End diff --
    
    ![image](https://user-images.githubusercontent.com/20113411/40701111-3b47089a-640f-11e8-8cd3-bc5684c07228.png)



---

[GitHub] flink issue #5857: [FLINK-9180][METRICS] add prometheus pushgateway reporter

Posted by zhangminglei <gi...@git.apache.org>.
Github user zhangminglei commented on the issue:

    https://github.com/apache/flink/pull/5857
  
    Hi, You push to the wrong place.


---

[GitHub] flink issue #5857: [FLINK-9180][METRICS] add prometheus pushgateway reporter

Posted by zhangminglei <gi...@git.apache.org>.
Github user zhangminglei commented on the issue:

    https://github.com/apache/flink/pull/5857
  
    Not ok. 9189 seems does not exist since you can not access that. You can check it.


---

[GitHub] flink issue #5857: [FLINK-9180][METRICS] add prometheus pushgateway reporter

Posted by zhangminglei <gi...@git.apache.org>.
Github user zhangminglei commented on the issue:

    https://github.com/apache/flink/pull/5857
  
    And I will delete the incorrect link from the flink-9180 jira. Let you know.


---

[GitHub] flink issue #5857: [FLINK-9187][METRICS] add prometheus pushgateway reporter

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:

    https://github.com/apache/flink/pull/5857
  
    I think it's fine to be in the same project.


---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5857#discussion_r191578999
  
    --- Diff: flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusPushGatewayReporter.java ---
    @@ -0,0 +1,79 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.metrics.prometheus;
    +
    +import org.apache.flink.annotation.PublicEvolving;
    +import org.apache.flink.metrics.Metric;
    +import org.apache.flink.metrics.MetricConfig;
    +import org.apache.flink.metrics.reporter.MetricReporter;
    +import org.apache.flink.metrics.reporter.Scheduled;
    +import org.apache.flink.util.AbstractID;
    +
    +import io.prometheus.client.CollectorRegistry;
    +import io.prometheus.client.exporter.PushGateway;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * /**
    + * {@link MetricReporter} that exports {@link Metric Metrics} via Prometheus Pushgateway.
    + */
    +@PublicEvolving
    +public class PrometheusPushGatewayReporter extends AbstractPrometheusReporter implements Scheduled {
    +	private static final Logger LOG = LoggerFactory.getLogger(PrometheusPushGatewayReporter.class);
    +
    +	public static final String ARG_HOST = "host";
    +	public static final String ARG_PORT = "port";
    +
    +	public static final char JOB_NAME_SEPARATOR = '-';
    +	public static final String JOB_NAME_PREFIX = "flink" + JOB_NAME_SEPARATOR;
    +
    +	private PushGateway pushGateway;
    +	private final String jobName;
    +
    +	public PrometheusPushGatewayReporter() {
    +		String random = new AbstractID().toString();
    +		jobName = JOB_NAME_PREFIX + random;
    --- End diff --
    
    It is very possible that I'm overthinking this, and I've come up with a compromise.
    
    If there is one thing we learned in regards to the metric system it is that users despise random IDs, especially so if they can't connect them with anything else. The random ID that you're suggesting is exactly that; a random piece of data, that effectively is just a workaround for the questionable design of the PushGateway.
    For the sake of analyzing metrics this ID is irrelevant, it just eats up space.
    The randomness is especially problematic since this ID is used for deleting metrics (which one has to do at some point), making this arbitrary value _really_ important.
    
    For our intents however we just need _unique_ value for each container, i.e. dispatcher/taskmanager etc., not necessarily random .
    Every distributed component already has such an ID, most notable the TaskManager ID that is already exposed to the metric system. JobManager IDs are currently not exposed, but it was only a matter of time until this becomes necessary.
    While technically still a random value it at least does not an an entirely not label/dimension, but merely copies an existing one.


---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by lamber-ken <gi...@git.apache.org>.
Github user lamber-ken commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5857#discussion_r191306509
  
    --- Diff: flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusPushGatewayReporter.java ---
    @@ -0,0 +1,79 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.metrics.prometheus;
    +
    +import org.apache.flink.annotation.PublicEvolving;
    +import org.apache.flink.metrics.Metric;
    +import org.apache.flink.metrics.MetricConfig;
    +import org.apache.flink.metrics.reporter.MetricReporter;
    +import org.apache.flink.metrics.reporter.Scheduled;
    +import org.apache.flink.util.AbstractID;
    +
    +import io.prometheus.client.CollectorRegistry;
    +import io.prometheus.client.exporter.PushGateway;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * /**
    + * {@link MetricReporter} that exports {@link Metric Metrics} via Prometheus Pushgateway.
    + */
    +@PublicEvolving
    +public class PrometheusPushGatewayReporter extends AbstractPrometheusReporter implements Scheduled {
    +	private static final Logger LOG = LoggerFactory.getLogger(PrometheusPushGatewayReporter.class);
    +
    +	public static final String ARG_HOST = "host";
    +	public static final String ARG_PORT = "port";
    +
    +	public static final char JOB_NAME_SEPARATOR = '-';
    +	public static final String JOB_NAME_PREFIX = "flink" + JOB_NAME_SEPARATOR;
    +
    +	private PushGateway pushGateway;
    +	private final String jobName;
    +
    +	public PrometheusPushGatewayReporter() {
    +		String random = new AbstractID().toString();
    +		jobName = JOB_NAME_PREFIX + random;
    --- End diff --
    
    the prefix of jobName can be configurable, ok?


---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by lamber-ken <gi...@git.apache.org>.
Github user lamber-ken commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5857#discussion_r195281287
  
    --- Diff: flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusPushGatewayReporter.java ---
    @@ -0,0 +1,79 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.metrics.prometheus;
    +
    +import org.apache.flink.annotation.PublicEvolving;
    +import org.apache.flink.metrics.Metric;
    +import org.apache.flink.metrics.MetricConfig;
    +import org.apache.flink.metrics.reporter.MetricReporter;
    +import org.apache.flink.metrics.reporter.Scheduled;
    +import org.apache.flink.util.AbstractID;
    +
    +import io.prometheus.client.CollectorRegistry;
    +import io.prometheus.client.exporter.PushGateway;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * /**
    + * {@link MetricReporter} that exports {@link Metric Metrics} via Prometheus Pushgateway.
    + */
    +@PublicEvolving
    +public class PrometheusPushGatewayReporter extends AbstractPrometheusReporter implements Scheduled {
    +	private static final Logger LOG = LoggerFactory.getLogger(PrometheusPushGatewayReporter.class);
    +
    +	public static final String ARG_HOST = "host";
    +	public static final String ARG_PORT = "port";
    +
    +	public static final char JOB_NAME_SEPARATOR = '-';
    +	public static final String JOB_NAME_PREFIX = "flink" + JOB_NAME_SEPARATOR;
    +
    +	private PushGateway pushGateway;
    +	private final String jobName;
    +
    +	public PrometheusPushGatewayReporter() {
    +		String random = new AbstractID().toString();
    +		jobName = JOB_NAME_PREFIX + random;
    --- End diff --
    
    ok,I see


---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by lamber-ken <gi...@git.apache.org>.
Github user lamber-ken closed the pull request at:

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


---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5857#discussion_r195644101
  
    --- Diff: flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusPushGatewayReporter.java ---
    @@ -0,0 +1,79 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.metrics.prometheus;
    +
    +import org.apache.flink.annotation.PublicEvolving;
    +import org.apache.flink.metrics.Metric;
    +import org.apache.flink.metrics.MetricConfig;
    +import org.apache.flink.metrics.reporter.MetricReporter;
    +import org.apache.flink.metrics.reporter.Scheduled;
    +import org.apache.flink.util.AbstractID;
    +
    +import io.prometheus.client.CollectorRegistry;
    +import io.prometheus.client.exporter.PushGateway;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * /**
    + * {@link MetricReporter} that exports {@link Metric Metrics} via Prometheus Pushgateway.
    + */
    +@PublicEvolving
    +public class PrometheusPushGatewayReporter extends AbstractPrometheusReporter implements Scheduled {
    +	private static final Logger LOG = LoggerFactory.getLogger(PrometheusPushGatewayReporter.class);
    +
    +	public static final String ARG_HOST = "host";
    +	public static final String ARG_PORT = "port";
    +
    +	public static final char JOB_NAME_SEPARATOR = '-';
    +	public static final String JOB_NAME_PREFIX = "flink" + JOB_NAME_SEPARATOR;
    +
    +	private PushGateway pushGateway;
    +	private final String jobName;
    +
    +	public PrometheusPushGatewayReporter() {
    +		String random = new AbstractID().toString();
    +		jobName = JOB_NAME_PREFIX + random;
    --- End diff --
    
    At the _very least_ we should already be using TM IDs.


---

[GitHub] flink issue #5857: [FLINK-9187][METRICS] add prometheus pushgateway reporter

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:

    https://github.com/apache/flink/pull/5857
  
    If you created another fork and are no longer able to access the previous one then yes, you will need a new PR.


---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5857#discussion_r191325180
  
    --- Diff: flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusPushGatewayReporter.java ---
    @@ -0,0 +1,79 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.metrics.prometheus;
    +
    +import org.apache.flink.annotation.PublicEvolving;
    +import org.apache.flink.metrics.Metric;
    +import org.apache.flink.metrics.MetricConfig;
    +import org.apache.flink.metrics.reporter.MetricReporter;
    +import org.apache.flink.metrics.reporter.Scheduled;
    +import org.apache.flink.util.AbstractID;
    +
    +import io.prometheus.client.CollectorRegistry;
    +import io.prometheus.client.exporter.PushGateway;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * /**
    + * {@link MetricReporter} that exports {@link Metric Metrics} via Prometheus Pushgateway.
    + */
    +@PublicEvolving
    +public class PrometheusPushGatewayReporter extends AbstractPrometheusReporter implements Scheduled {
    +	private static final Logger LOG = LoggerFactory.getLogger(PrometheusPushGatewayReporter.class);
    +
    +	public static final String ARG_HOST = "host";
    +	public static final String ARG_PORT = "port";
    +
    +	public static final char JOB_NAME_SEPARATOR = '-';
    +	public static final String JOB_NAME_PREFIX = "flink" + JOB_NAME_SEPARATOR;
    +
    +	private PushGateway pushGateway;
    +	private final String jobName;
    +
    +	public PrometheusPushGatewayReporter() {
    +		String random = new AbstractID().toString();
    +		jobName = JOB_NAME_PREFIX + random;
    --- End diff --
    
    That can only happen if 2 metrics have the exact same name and set of labels. Due to how the reporter is implemented this generally cannot happen.


---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5857#discussion_r195229711
  
    --- Diff: flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusPushGatewayReporter.java ---
    @@ -0,0 +1,79 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.metrics.prometheus;
    +
    +import org.apache.flink.annotation.PublicEvolving;
    +import org.apache.flink.metrics.Metric;
    +import org.apache.flink.metrics.MetricConfig;
    +import org.apache.flink.metrics.reporter.MetricReporter;
    +import org.apache.flink.metrics.reporter.Scheduled;
    +import org.apache.flink.util.AbstractID;
    +
    +import io.prometheus.client.CollectorRegistry;
    +import io.prometheus.client.exporter.PushGateway;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * /**
    + * {@link MetricReporter} that exports {@link Metric Metrics} via Prometheus Pushgateway.
    + */
    +@PublicEvolving
    +public class PrometheusPushGatewayReporter extends AbstractPrometheusReporter implements Scheduled {
    +	private static final Logger LOG = LoggerFactory.getLogger(PrometheusPushGatewayReporter.class);
    +
    +	public static final String ARG_HOST = "host";
    +	public static final String ARG_PORT = "port";
    +
    +	public static final char JOB_NAME_SEPARATOR = '-';
    +	public static final String JOB_NAME_PREFIX = "flink" + JOB_NAME_SEPARATOR;
    +
    +	private PushGateway pushGateway;
    +	private final String jobName;
    +
    +	public PrometheusPushGatewayReporter() {
    +		String random = new AbstractID().toString();
    +		jobName = JOB_NAME_PREFIX + random;
    --- End diff --
    
    I'm inclined to block the PR on the JobManager ID exposure. ([FLINK-9543](https://issues.apache.org/jira/browse/FLINK-9543))
    
    The PR is not at risk at becoming outdated, so keeping it open for a while isn't a problem.


---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5857#discussion_r191175257
  
    --- Diff: flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusReporter.java ---
    @@ -120,199 +83,4 @@ public void close() {
     		CollectorRegistry.defaultRegistry.clear();
    --- End diff --
    
    replace with `super.close()`


---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by lamber-ken <gi...@git.apache.org>.
Github user lamber-ken commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5857#discussion_r191628488
  
    --- Diff: flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusPushGatewayReporter.java ---
    @@ -0,0 +1,79 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.metrics.prometheus;
    +
    +import org.apache.flink.annotation.PublicEvolving;
    +import org.apache.flink.metrics.Metric;
    +import org.apache.flink.metrics.MetricConfig;
    +import org.apache.flink.metrics.reporter.MetricReporter;
    +import org.apache.flink.metrics.reporter.Scheduled;
    +import org.apache.flink.util.AbstractID;
    +
    +import io.prometheus.client.CollectorRegistry;
    +import io.prometheus.client.exporter.PushGateway;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * /**
    + * {@link MetricReporter} that exports {@link Metric Metrics} via Prometheus Pushgateway.
    + */
    +@PublicEvolving
    +public class PrometheusPushGatewayReporter extends AbstractPrometheusReporter implements Scheduled {
    +	private static final Logger LOG = LoggerFactory.getLogger(PrometheusPushGatewayReporter.class);
    +
    +	public static final String ARG_HOST = "host";
    +	public static final String ARG_PORT = "port";
    +
    +	public static final char JOB_NAME_SEPARATOR = '-';
    +	public static final String JOB_NAME_PREFIX = "flink" + JOB_NAME_SEPARATOR;
    +
    +	private PushGateway pushGateway;
    +	private final String jobName;
    +
    +	public PrometheusPushGatewayReporter() {
    +		String random = new AbstractID().toString();
    +		jobName = JOB_NAME_PREFIX + random;
    --- End diff --
    
    :+1: right
    it's better to use JobManager ID / TaskManager ID to compose the jobName, then jobName is `JM ID` / `TM ID`  
    or combined with the specified prefix like `prefix + JM ID` / `prefix + TM ID`
    
    but for now,  JM IDs are currently not exposed, so use random strings instead of JM/TM ID


---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5857#discussion_r191561341
  
    --- Diff: flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusPushGatewayReporter.java ---
    @@ -0,0 +1,79 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.metrics.prometheus;
    +
    +import org.apache.flink.annotation.PublicEvolving;
    +import org.apache.flink.metrics.Metric;
    +import org.apache.flink.metrics.MetricConfig;
    +import org.apache.flink.metrics.reporter.MetricReporter;
    +import org.apache.flink.metrics.reporter.Scheduled;
    +import org.apache.flink.util.AbstractID;
    +
    +import io.prometheus.client.CollectorRegistry;
    +import io.prometheus.client.exporter.PushGateway;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * /**
    + * {@link MetricReporter} that exports {@link Metric Metrics} via Prometheus Pushgateway.
    + */
    +@PublicEvolving
    +public class PrometheusPushGatewayReporter extends AbstractPrometheusReporter implements Scheduled {
    +	private static final Logger LOG = LoggerFactory.getLogger(PrometheusPushGatewayReporter.class);
    +
    +	public static final String ARG_HOST = "host";
    +	public static final String ARG_PORT = "port";
    +
    +	public static final char JOB_NAME_SEPARATOR = '-';
    +	public static final String JOB_NAME_PREFIX = "flink" + JOB_NAME_SEPARATOR;
    +
    +	private PushGateway pushGateway;
    +	private final String jobName;
    +
    +	public PrometheusPushGatewayReporter() {
    +		String random = new AbstractID().toString();
    +		jobName = JOB_NAME_PREFIX + random;
    --- End diff --
    
    eh forget it, that's not a viable option for containerized environments that this issue targets anyway...


---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5857#discussion_r191576816
  
    --- Diff: docs/monitoring/metrics.md ---
    @@ -699,6 +699,39 @@ Flink metric types are mapped to Prometheus metric types as follows:
     
     All Flink metrics variables (see [List of all Variables](#list-of-all-variables)) are exported to Prometheus as labels. 
     
    +### PrometheusPushGateway (org.apache.flink.metrics.prometheus.PrometheusPushGatewayReporter)
    --- End diff --
    
    Please add a section highlighting the differences and use-cases compared to the existing reporter.
    
    In particular we should mention that this reporter, like the existing reporter, is not suited for short-lived jobs.


---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by lamber-ken <gi...@git.apache.org>.
Github user lamber-ken commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5857#discussion_r191428603
  
    --- Diff: flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusPushGatewayReporter.java ---
    @@ -0,0 +1,79 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.metrics.prometheus;
    +
    +import org.apache.flink.annotation.PublicEvolving;
    +import org.apache.flink.metrics.Metric;
    +import org.apache.flink.metrics.MetricConfig;
    +import org.apache.flink.metrics.reporter.MetricReporter;
    +import org.apache.flink.metrics.reporter.Scheduled;
    +import org.apache.flink.util.AbstractID;
    +
    +import io.prometheus.client.CollectorRegistry;
    +import io.prometheus.client.exporter.PushGateway;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * /**
    + * {@link MetricReporter} that exports {@link Metric Metrics} via Prometheus Pushgateway.
    + */
    +@PublicEvolving
    +public class PrometheusPushGatewayReporter extends AbstractPrometheusReporter implements Scheduled {
    +	private static final Logger LOG = LoggerFactory.getLogger(PrometheusPushGatewayReporter.class);
    +
    +	public static final String ARG_HOST = "host";
    +	public static final String ARG_PORT = "port";
    +
    +	public static final char JOB_NAME_SEPARATOR = '-';
    +	public static final String JOB_NAME_PREFIX = "flink" + JOB_NAME_SEPARATOR;
    +
    +	private PushGateway pushGateway;
    +	private final String jobName;
    +
    +	public PrometheusPushGatewayReporter() {
    +		String random = new AbstractID().toString();
    +		jobName = JOB_NAME_PREFIX + random;
    --- End diff --
    
    May I think your thinking is complicated, here's my idea
    Using different name prefixes to distinguish different flink clusters, and each taskmanager or jobmanager in the same flink cluster uses the same prefix, it has met our needs.
    We just need to make sure that different clusters use different jobName
    
    
    
    
    if so, the metrics of tm(A) may be covered with the metrics of tm(B), etc


---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by lamber-ken <gi...@git.apache.org>.
Github user lamber-ken commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5857#discussion_r191335598
  
    --- Diff: flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusPushGatewayReporter.java ---
    @@ -0,0 +1,79 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.metrics.prometheus;
    +
    +import org.apache.flink.annotation.PublicEvolving;
    +import org.apache.flink.metrics.Metric;
    +import org.apache.flink.metrics.MetricConfig;
    +import org.apache.flink.metrics.reporter.MetricReporter;
    +import org.apache.flink.metrics.reporter.Scheduled;
    +import org.apache.flink.util.AbstractID;
    +
    +import io.prometheus.client.CollectorRegistry;
    +import io.prometheus.client.exporter.PushGateway;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * /**
    + * {@link MetricReporter} that exports {@link Metric Metrics} via Prometheus Pushgateway.
    + */
    +@PublicEvolving
    +public class PrometheusPushGatewayReporter extends AbstractPrometheusReporter implements Scheduled {
    +	private static final Logger LOG = LoggerFactory.getLogger(PrometheusPushGatewayReporter.class);
    +
    +	public static final String ARG_HOST = "host";
    +	public static final String ARG_PORT = "port";
    +
    +	public static final char JOB_NAME_SEPARATOR = '-';
    +	public static final String JOB_NAME_PREFIX = "flink" + JOB_NAME_SEPARATOR;
    +
    +	private PushGateway pushGateway;
    +	private final String jobName;
    +
    +	public PrometheusPushGatewayReporter() {
    +		String random = new AbstractID().toString();
    +		jobName = JOB_NAME_PREFIX + random;
    --- End diff --
    
    Hi, I did an experment, one jm with two tm, then send metrics to gateway, here's result
    ![image](https://user-images.githubusercontent.com/20113411/40646016-a3b2f188-635a-11e8-9e07-283f167179e3.png)



---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5857#discussion_r191174912
  
    --- Diff: flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/AbstractPrometheusReporter.java ---
    @@ -0,0 +1,283 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.metrics.prometheus;
    +
    +import org.apache.flink.annotation.PublicEvolving;
    +import org.apache.flink.annotation.VisibleForTesting;
    +import org.apache.flink.metrics.CharacterFilter;
    +import org.apache.flink.metrics.Counter;
    +import org.apache.flink.metrics.Gauge;
    +import org.apache.flink.metrics.Histogram;
    +import org.apache.flink.metrics.Meter;
    +import org.apache.flink.metrics.Metric;
    +import org.apache.flink.metrics.MetricConfig;
    +import org.apache.flink.metrics.MetricGroup;
    +import org.apache.flink.metrics.reporter.MetricReporter;
    +import org.apache.flink.runtime.metrics.groups.AbstractMetricGroup;
    +import org.apache.flink.runtime.metrics.groups.FrontMetricGroup;
    +
    +import io.prometheus.client.Collector;
    +import io.prometheus.client.CollectorRegistry;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import java.util.AbstractMap;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.Collections;
    +import java.util.HashMap;
    +import java.util.LinkedList;
    +import java.util.List;
    +import java.util.Map;
    +import java.util.regex.Pattern;
    +
    +
    +/**
    + * base prometheus reporter for prometheus metrics.
    + */
    +@PublicEvolving
    +public abstract class AbstractPrometheusReporter implements MetricReporter {
    +
    +	private static final Logger LOG = LoggerFactory.getLogger(AbstractPrometheusReporter.class);
    +
    +	private static final Pattern UNALLOWED_CHAR_PATTERN = Pattern.compile("[^a-zA-Z0-9:_]");
    +	private static final CharacterFilter CHARACTER_FILTER = new CharacterFilter() {
    +		@Override
    +		public String filterCharacters(String input) {
    +			return replaceInvalidChars(input);
    +		}
    +	};
    +
    +	private static final char SCOPE_SEPARATOR = '_';
    +	private static final String SCOPE_PREFIX = "flink" + SCOPE_SEPARATOR;
    +
    +	private final Map<String, AbstractMap.SimpleImmutableEntry<Collector, Integer>> collectorsWithCountByMetricName = new HashMap<>();
    +
    +	@VisibleForTesting
    +	static String replaceInvalidChars(final String input) {
    +		// https://prometheus.io/docs/instrumenting/writing_exporters/
    +		// Only [a-zA-Z0-9:_] are valid in metric names, any other characters should be sanitized to an underscore.
    +		return UNALLOWED_CHAR_PATTERN.matcher(input).replaceAll("_");
    +	}
    +
    +	@Override
    +	public abstract void open(MetricConfig config);
    --- End diff --
    
    This override is unnecessary.


---

[GitHub] flink issue #5857: [FLINK-9180][METRICS] add prometheus pushgateway reporter

Posted by lamber-ken <gi...@git.apache.org>.
Github user lamber-ken commented on the issue:

    https://github.com/apache/flink/pull/5857
  
    ok, I see. I'll close the PR.
    by the way, need to create jira first, and then PR?



---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5857#discussion_r191350958
  
    --- Diff: flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusPushGatewayReporter.java ---
    @@ -0,0 +1,79 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.metrics.prometheus;
    +
    +import org.apache.flink.annotation.PublicEvolving;
    +import org.apache.flink.metrics.Metric;
    +import org.apache.flink.metrics.MetricConfig;
    +import org.apache.flink.metrics.reporter.MetricReporter;
    +import org.apache.flink.metrics.reporter.Scheduled;
    +import org.apache.flink.util.AbstractID;
    +
    +import io.prometheus.client.CollectorRegistry;
    +import io.prometheus.client.exporter.PushGateway;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * /**
    + * {@link MetricReporter} that exports {@link Metric Metrics} via Prometheus Pushgateway.
    + */
    +@PublicEvolving
    +public class PrometheusPushGatewayReporter extends AbstractPrometheusReporter implements Scheduled {
    +	private static final Logger LOG = LoggerFactory.getLogger(PrometheusPushGatewayReporter.class);
    +
    +	public static final String ARG_HOST = "host";
    +	public static final String ARG_PORT = "port";
    +
    +	public static final char JOB_NAME_SEPARATOR = '-';
    +	public static final String JOB_NAME_PREFIX = "flink" + JOB_NAME_SEPARATOR;
    +
    +	private PushGateway pushGateway;
    +	private final String jobName;
    +
    +	public PrometheusPushGatewayReporter() {
    +		String random = new AbstractID().toString();
    +		jobName = JOB_NAME_PREFIX + random;
    --- End diff --
    
    Oh i see, the problem is that we're only using the generated job name as the grouping key.
    ```
    pushGateway.push(CollectorRegistry.defaultRegistry, jobName);
    ```
    What we actually want is to separate this push into multiple pushes, one for each grouping key combination.


---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5857#discussion_r191173964
  
    --- Diff: flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusPushGatewayReporter.java ---
    @@ -0,0 +1,79 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.metrics.prometheus;
    +
    +import org.apache.flink.annotation.PublicEvolving;
    +import org.apache.flink.metrics.Metric;
    +import org.apache.flink.metrics.MetricConfig;
    +import org.apache.flink.metrics.reporter.MetricReporter;
    +import org.apache.flink.metrics.reporter.Scheduled;
    +import org.apache.flink.util.AbstractID;
    +
    +import io.prometheus.client.CollectorRegistry;
    +import io.prometheus.client.exporter.PushGateway;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * /**
    + * {@link MetricReporter} that exports {@link Metric Metrics} via Prometheus Pushgateway.
    + */
    +@PublicEvolving
    +public class PrometheusPushGatewayReporter extends AbstractPrometheusReporter implements Scheduled {
    --- End diff --
    
    the reporter must be documented in https://github.com/apache/flink/blob/master/docs/monitoring/metrics.md


---

[GitHub] flink issue #5857: [FLINK-9180][METRICS] add prometheus pushgateway reporter

Posted by zhangminglei <gi...@git.apache.org>.
Github user zhangminglei commented on the issue:

    https://github.com/apache/flink/pull/5857
  
    Yes. You should create the JIRA first. Then, push a PR to the corresponding jira number. 


---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5857#discussion_r195650428
  
    --- Diff: flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusPushGatewayReporter.java ---
    @@ -0,0 +1,79 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.metrics.prometheus;
    +
    +import org.apache.flink.annotation.PublicEvolving;
    +import org.apache.flink.metrics.Metric;
    +import org.apache.flink.metrics.MetricConfig;
    +import org.apache.flink.metrics.reporter.MetricReporter;
    +import org.apache.flink.metrics.reporter.Scheduled;
    +import org.apache.flink.util.AbstractID;
    +
    +import io.prometheus.client.CollectorRegistry;
    +import io.prometheus.client.exporter.PushGateway;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * /**
    + * {@link MetricReporter} that exports {@link Metric Metrics} via Prometheus Pushgateway.
    + */
    +@PublicEvolving
    +public class PrometheusPushGatewayReporter extends AbstractPrometheusReporter implements Scheduled {
    +	private static final Logger LOG = LoggerFactory.getLogger(PrometheusPushGatewayReporter.class);
    +
    +	public static final String ARG_HOST = "host";
    +	public static final String ARG_PORT = "port";
    +
    +	public static final char JOB_NAME_SEPARATOR = '-';
    +	public static final String JOB_NAME_PREFIX = "flink" + JOB_NAME_SEPARATOR;
    +
    +	private PushGateway pushGateway;
    +	private final String jobName;
    +
    +	public PrometheusPushGatewayReporter() {
    +		String random = new AbstractID().toString();
    +		jobName = JOB_NAME_PREFIX + random;
    --- End diff --
    
    I'm not sure whether changing how the IDs are generated will break anyone's setup. My worry is just that this PR might fall through the cracks even after FLINK-9543 is being fixed. That would be a pity since it is a really nice feature from which users benefit.
    
    Agreed that we should use the TM IDs for the TaskManager metrics.


---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by lamber-ken <gi...@git.apache.org>.
Github user lamber-ken commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5857#discussion_r191297773
  
    --- Diff: flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusPushGatewayReporter.java ---
    @@ -0,0 +1,79 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.metrics.prometheus;
    +
    +import org.apache.flink.annotation.PublicEvolving;
    +import org.apache.flink.metrics.Metric;
    +import org.apache.flink.metrics.MetricConfig;
    +import org.apache.flink.metrics.reporter.MetricReporter;
    +import org.apache.flink.metrics.reporter.Scheduled;
    +import org.apache.flink.util.AbstractID;
    +
    +import io.prometheus.client.CollectorRegistry;
    +import io.prometheus.client.exporter.PushGateway;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * /**
    + * {@link MetricReporter} that exports {@link Metric Metrics} via Prometheus Pushgateway.
    + */
    +@PublicEvolving
    +public class PrometheusPushGatewayReporter extends AbstractPrometheusReporter implements Scheduled {
    +	private static final Logger LOG = LoggerFactory.getLogger(PrometheusPushGatewayReporter.class);
    +
    +	public static final String ARG_HOST = "host";
    +	public static final String ARG_PORT = "port";
    +
    +	public static final char JOB_NAME_SEPARATOR = '-';
    +	public static final String JOB_NAME_PREFIX = "flink" + JOB_NAME_SEPARATOR;
    +
    +	private PushGateway pushGateway;
    +	private final String jobName;
    +
    +	public PrometheusPushGatewayReporter() {
    +		String random = new AbstractID().toString();
    +		jobName = JOB_NAME_PREFIX + random;
    --- End diff --
    
    if the jobname is configurable, it means each taskmanager may use same jobname. 
    if so, the metrics of tm(A) may be covered with the metrics of tm(B), etc
    
    ### for example, tm1, tm2 push the metrics at the same time
    ```
    <dependency>
        <groupId>io.prometheus</groupId>
        <artifactId>simpleclient</artifactId>
        <version>0.0.26</version>
    </dependency>
    
    <dependency>
        <groupId>io.prometheus</groupId>
        <artifactId>simpleclient_pushgateway</artifactId>
        <version>0.0.26</version>
    </dependency>
    
    
    
    
    CollectorRegistry registry = new CollectorRegistry();
    String sameJobName = "flink-job";
    
    // taskmanager A
    Gauge tm1 = Gauge.build().name("flink_taskmanager_Status_JVM_CPU_Time").help("tm jvm cpu").register(registry);
    tm1.set(41);
    
    PushGateway pg1 = new PushGateway("localhost:9091");
    pg1.push(registry, sameJobName);
    
    
    // taskmanager B
    registry.clear();
    Gauge tm2 = Gauge.build().name("flink_taskmanager_Status_JVM_CPU_Time").help("tm jvm cpu").register(registry);
    tm2.set(42);
    
    PushGateway pg2 = new PushGateway("localhost:9091");
    pg2.push(registry, sameJobName);
    
    ```
    
    ### result, the metrics of tmA is covered with tmB
    ![image](https://user-images.githubusercontent.com/20113411/40636865-269828ce-6334-11e8-92e7-b222e6cfe6c0.png)



---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5857#discussion_r191348415
  
    --- Diff: flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusPushGatewayReporter.java ---
    @@ -0,0 +1,79 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.metrics.prometheus;
    +
    +import org.apache.flink.annotation.PublicEvolving;
    +import org.apache.flink.metrics.Metric;
    +import org.apache.flink.metrics.MetricConfig;
    +import org.apache.flink.metrics.reporter.MetricReporter;
    +import org.apache.flink.metrics.reporter.Scheduled;
    +import org.apache.flink.util.AbstractID;
    +
    +import io.prometheus.client.CollectorRegistry;
    +import io.prometheus.client.exporter.PushGateway;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * /**
    + * {@link MetricReporter} that exports {@link Metric Metrics} via Prometheus Pushgateway.
    + */
    +@PublicEvolving
    +public class PrometheusPushGatewayReporter extends AbstractPrometheusReporter implements Scheduled {
    +	private static final Logger LOG = LoggerFactory.getLogger(PrometheusPushGatewayReporter.class);
    +
    +	public static final String ARG_HOST = "host";
    +	public static final String ARG_PORT = "port";
    +
    +	public static final char JOB_NAME_SEPARATOR = '-';
    +	public static final String JOB_NAME_PREFIX = "flink" + JOB_NAME_SEPARATOR;
    +
    +	private PushGateway pushGateway;
    +	private final String jobName;
    +
    +	public PrometheusPushGatewayReporter() {
    +		String random = new AbstractID().toString();
    +		jobName = JOB_NAME_PREFIX + random;
    --- End diff --
    
    well, I forgot to remove the random suffix in my test. With the suffix removed indeed only metrics from one TM shows up.
    
    I woudl consider this a bug in the pushgateway; isn't the whole point of labels to differentiate between different instances?


---

[GitHub] flink issue #5857: [FLINK-9187][METRICS] add prometheus pushgateway reporter

Posted by lamber-ken <gi...@git.apache.org>.
Github user lamber-ken commented on the issue:

    https://github.com/apache/flink/pull/5857
  
    sorry, I reforked `flink` project, do I need to start a new PR?
    ![image](https://user-images.githubusercontent.com/20113411/41635822-76271486-747d-11e8-9ad3-c6447c1b930c.png)



---

[GitHub] flink issue #5857: [FLINK-9187][METRICS] add prometheus pushgateway reporter

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:

    https://github.com/apache/flink/pull/5857
  
    I I see the value of a `cluster_id` but you're mixing concerns here. It is not the responsibility of a `reporter` to introduce a `cluster_id` tag. Reporters are to faithfully report the metrics and their associated variables, not add more. Instead we may want to think about adding a configurable `cluster_id` value.
    
    With that out of the way we have arrived at my previous question, random ID vs actual ID. That the `job_name` will be equal to `tm/jm_id` is precisely why i prefer this approach, it doesn't introduce additional noise in the tags.
    
    I've already started working on `FLINK-9543` so we can proceed with this PR; the issue must be extended to also expose an ID for Dispatchers and register the JVM metrics for each JM. as to your question, `ResourceID`s are exactly what we're looking for; this is also what we're using for `TaskExecutor`.


---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5857#discussion_r191346124
  
    --- Diff: flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusPushGatewayReporter.java ---
    @@ -0,0 +1,79 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.metrics.prometheus;
    +
    +import org.apache.flink.annotation.PublicEvolving;
    +import org.apache.flink.metrics.Metric;
    +import org.apache.flink.metrics.MetricConfig;
    +import org.apache.flink.metrics.reporter.MetricReporter;
    +import org.apache.flink.metrics.reporter.Scheduled;
    +import org.apache.flink.util.AbstractID;
    +
    +import io.prometheus.client.CollectorRegistry;
    +import io.prometheus.client.exporter.PushGateway;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * /**
    + * {@link MetricReporter} that exports {@link Metric Metrics} via Prometheus Pushgateway.
    + */
    +@PublicEvolving
    +public class PrometheusPushGatewayReporter extends AbstractPrometheusReporter implements Scheduled {
    +	private static final Logger LOG = LoggerFactory.getLogger(PrometheusPushGatewayReporter.class);
    +
    +	public static final String ARG_HOST = "host";
    +	public static final String ARG_PORT = "port";
    +
    +	public static final char JOB_NAME_SEPARATOR = '-';
    +	public static final String JOB_NAME_PREFIX = "flink" + JOB_NAME_SEPARATOR;
    +
    +	private PushGateway pushGateway;
    +	private final String jobName;
    +
    +	public PrometheusPushGatewayReporter() {
    +		String random = new AbstractID().toString();
    +		jobName = JOB_NAME_PREFIX + random;
    --- End diff --
    
    wait a second...


---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by lamber-ken <gi...@git.apache.org>.
Github user lamber-ken commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5857#discussion_r191628800
  
    --- Diff: docs/monitoring/metrics.md ---
    @@ -699,6 +699,39 @@ Flink metric types are mapped to Prometheus metric types as follows:
     
     All Flink metrics variables (see [List of all Variables](#list-of-all-variables)) are exported to Prometheus as labels. 
     
    +### PrometheusPushGateway (org.apache.flink.metrics.prometheus.PrometheusPushGatewayReporter)
    --- End diff --
    
    thanks for review, I will improve the doc


---

[GitHub] flink issue #5857: [FLINK-9187][METRICS] add prometheus pushgateway reporter

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:

    https://github.com/apache/flink/pull/5857
  
    Why do you prefer random ID's to differentiate between JM/TM than their actual unique ID?


---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5857#discussion_r195346637
  
    --- Diff: flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusPushGatewayReporter.java ---
    @@ -0,0 +1,79 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.metrics.prometheus;
    +
    +import org.apache.flink.annotation.PublicEvolving;
    +import org.apache.flink.metrics.Metric;
    +import org.apache.flink.metrics.MetricConfig;
    +import org.apache.flink.metrics.reporter.MetricReporter;
    +import org.apache.flink.metrics.reporter.Scheduled;
    +import org.apache.flink.util.AbstractID;
    +
    +import io.prometheus.client.CollectorRegistry;
    +import io.prometheus.client.exporter.PushGateway;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * /**
    + * {@link MetricReporter} that exports {@link Metric Metrics} via Prometheus Pushgateway.
    + */
    +@PublicEvolving
    +public class PrometheusPushGatewayReporter extends AbstractPrometheusReporter implements Scheduled {
    +	private static final Logger LOG = LoggerFactory.getLogger(PrometheusPushGatewayReporter.class);
    +
    +	public static final String ARG_HOST = "host";
    +	public static final String ARG_PORT = "port";
    +
    +	public static final char JOB_NAME_SEPARATOR = '-';
    +	public static final String JOB_NAME_PREFIX = "flink" + JOB_NAME_SEPARATOR;
    +
    +	private PushGateway pushGateway;
    +	private final String jobName;
    +
    +	public PrometheusPushGatewayReporter() {
    +		String random = new AbstractID().toString();
    +		jobName = JOB_NAME_PREFIX + random;
    --- End diff --
    
    Theoretically, we could also merge this PR first and then implement [FLINK-9543](https://issues.apache.org/jira/browse/FLINK-9543) as a follow up if we say that in the first iteration we assign a unique (random) id to the metric name. That would make it work for @lamber-ken. Once we have implemented FLINK-9543, then we could replace the random part by the `JobManagerId`. Would that be a big problem @zentol?


---

[GitHub] flink issue #5857: [FLINK-9180][METRICS] add prometheus pushgateway reporter

Posted by lamber-ken <gi...@git.apache.org>.
Github user lamber-ken commented on the issue:

    https://github.com/apache/flink/pull/5857
  
    ok


---

[GitHub] flink issue #5857: [FLINK-9187][METRICS] add prometheus pushgateway reporter

Posted by lamber-ken <gi...@git.apache.org>.
Github user lamber-ken commented on the issue:

    https://github.com/apache/flink/pull/5857
  
    for example, 
    #### Environment
    - deploy flink job on hadoop yarn using `yarn-cluster` model
    - prometheus collect metrics 
      because we use prometheus in production, so don't want to use graphite or ganglia currently etc
    
    #### if not use pushgateway
    because using yarn-cluster model, so we can't know where the jobmanager on which machine and 
    we can't to define the prometheus targets.
    ![image](https://user-images.githubusercontent.com/20113411/38981072-1575909c-4384-11e8-8308-9620e06808fb.png)
    
    #### use pushgateway
    ![image](https://user-images.githubusercontent.com/20113411/38981092-1f94e2d0-4384-11e8-998c-902f8951b727.png)
    
    #### prometheus pushgateway
    although pushgateway has some [limits](https://prometheus.io/docs/practices/pushing/) currently,
    but it will meet our needs in the future
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    
    



---

[GitHub] flink issue #5857: [FLINK-9180][METRICS] add prometheus pushgateway reporter

Posted by zhangminglei <gi...@git.apache.org>.
Github user zhangminglei commented on the issue:

    https://github.com/apache/flink/pull/5857
  
    Yes. Apache hadoop is also different from apache flink. we should obey the rules.


---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5857#discussion_r191376001
  
    --- Diff: flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusPushGatewayReporter.java ---
    @@ -0,0 +1,79 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.metrics.prometheus;
    +
    +import org.apache.flink.annotation.PublicEvolving;
    +import org.apache.flink.metrics.Metric;
    +import org.apache.flink.metrics.MetricConfig;
    +import org.apache.flink.metrics.reporter.MetricReporter;
    +import org.apache.flink.metrics.reporter.Scheduled;
    +import org.apache.flink.util.AbstractID;
    +
    +import io.prometheus.client.CollectorRegistry;
    +import io.prometheus.client.exporter.PushGateway;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * /**
    + * {@link MetricReporter} that exports {@link Metric Metrics} via Prometheus Pushgateway.
    + */
    +@PublicEvolving
    +public class PrometheusPushGatewayReporter extends AbstractPrometheusReporter implements Scheduled {
    +	private static final Logger LOG = LoggerFactory.getLogger(PrometheusPushGatewayReporter.class);
    +
    +	public static final String ARG_HOST = "host";
    +	public static final String ARG_PORT = "port";
    +
    +	public static final char JOB_NAME_SEPARATOR = '-';
    +	public static final String JOB_NAME_PREFIX = "flink" + JOB_NAME_SEPARATOR;
    +
    +	private PushGateway pushGateway;
    +	private final String jobName;
    +
    +	public PrometheusPushGatewayReporter() {
    +		String random = new AbstractID().toString();
    +		jobName = JOB_NAME_PREFIX + random;
    --- End diff --
    
    To implement this we will have to change the design of the new reporter significantly. This will entail moving more logic from the abstract class to the concrete `PrometheusReporter` class as it will no longer be shared. Ideally the abstract class will only contain shared utility methods, and does not dictate the actual report/notification logic.
    
    The pushgateway reporter will create a separate collector for each grouping key (i.e. variable names/values, retrieved via `MetricGroup#getAllVariables()`). The collectors are not registered with the registry as this is only necessary in the pull model. The grouping by labels should ensure that we don't send to many requests; we may want to introduce a delay parameter to be safe.
    A new metric will be assigned to a collector based on the hash of the variables map; we may not use the variables map or MetricGroup direclty as a mapping key as the map may change over time.
    Similar to the existing vanilla reporter we will have to rely on ref-counting to cleanup existing collectors.
    
    This will allow users to freely switch between both reporters without having to worry about any additional detail but the port configuration.


---

[GitHub] flink issue #5857: [FLINK-9180][METRICS] add prometheus pushgateway reporter

Posted by zhangminglei <gi...@git.apache.org>.
Github user zhangminglei commented on the issue:

    https://github.com/apache/flink/pull/5857
  
    @lamber-ken You push your code to the incorrect jira number, flink-9180. But it is not relevant to your issue. you can check this out : https://issues.apache.org/jira/browse/FLINK-9180. 


---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/5857#discussion_r191345794
  
    --- Diff: flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusPushGatewayReporter.java ---
    @@ -0,0 +1,79 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.metrics.prometheus;
    +
    +import org.apache.flink.annotation.PublicEvolving;
    +import org.apache.flink.metrics.Metric;
    +import org.apache.flink.metrics.MetricConfig;
    +import org.apache.flink.metrics.reporter.MetricReporter;
    +import org.apache.flink.metrics.reporter.Scheduled;
    +import org.apache.flink.util.AbstractID;
    +
    +import io.prometheus.client.CollectorRegistry;
    +import io.prometheus.client.exporter.PushGateway;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * /**
    + * {@link MetricReporter} that exports {@link Metric Metrics} via Prometheus Pushgateway.
    + */
    +@PublicEvolving
    +public class PrometheusPushGatewayReporter extends AbstractPrometheusReporter implements Scheduled {
    +	private static final Logger LOG = LoggerFactory.getLogger(PrometheusPushGatewayReporter.class);
    +
    +	public static final String ARG_HOST = "host";
    +	public static final String ARG_PORT = "port";
    +
    +	public static final char JOB_NAME_SEPARATOR = '-';
    +	public static final String JOB_NAME_PREFIX = "flink" + JOB_NAME_SEPARATOR;
    +
    +	private PushGateway pushGateway;
    +	private final String jobName;
    +
    +	public PrometheusPushGatewayReporter() {
    +		String random = new AbstractID().toString();
    +		jobName = JOB_NAME_PREFIX + random;
    --- End diff --
    
    The PushGateway UI is irrelevant though, the Prometheus UI is what matters.
    
    ![capture](https://user-images.githubusercontent.com/5725237/40647873-4355e0de-632d-11e8-8fb1-9beb811410ad.PNG)



---

[GitHub] flink issue #5857: [FLINK-9187][METRICS] add prometheus pushgateway reporter

Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:

    https://github.com/apache/flink/pull/5857
  
    I think this is a nice addition. Basically turns the prometheus "pull model" into a "push model".
    
    @lamber-ken Can you check that the new dependency is correctly shaded?
    
    @zentol Do you think this is good in the same project as the prometheus reporter, or should this be in a separate project?


---

[GitHub] flink issue #5857: [FLINK-9187][METRICS] add prometheus pushgateway reporter

Posted by lamber-ken <gi...@git.apache.org>.
Github user lamber-ken commented on the issue:

    https://github.com/apache/flink/pull/5857
  
    @zentol , thanks for review.
    first, there is a small point that the IDs may be duplicated when use JM/TM actual unique ID to compose the job name of pushgateway, like the picture below.
    ![image](https://user-images.githubusercontent.com/20113411/41587417-c7367d86-73e1-11e8-95fc-ae54dbd63809.png)
    
    second, when config the grafana dashboard, the prefix of the pushgateway job name is useful, the uniq ID may not useful, because our metric data contains (`jm_id`, `tm_id`).
    
    so, for pushgateway, the jobName is used to distinguish different services.
    for grafana, the prefix of jobName can use to distinguish different clusters, we also can just use JM/TM ID and ignore the jobName.
    
    `FLINK-9543` is useful and important. I tried to finished it. 
    Is't ok to use `resourceId` to represent the JM‘ID?
    ```java
    public abstract class ResourceManager ...{
    	public static final String RESOURCE_MANAGER_NAME = "resourcemanager";
    
    	/** Unique id of the resource manager. */
    	private final ResourceID resourceId;
    
    ```


---

[GitHub] flink pull request #5857: [FLINK-9187][METRICS] add prometheus pushgateway r...

Posted by lamber-ken <gi...@git.apache.org>.
GitHub user lamber-ken reopened a pull request:

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

    [FLINK-9187][METRICS] add prometheus pushgateway reporter

    ## What is the purpose of the change
    This pull request makes flink system can send metrics to prometheus via pushgateway. when using `yarn-cluster` model, it's useful.
    
    ## Brief change log
    
      - Add prometheus pushgateway repoter
      - Restructure the code of the promethues reporter part
    
    ## Verifying this change
    
    This change is already covered by existing tests. [prometheus test](https://github.com/apache/flink/tree/master/flink-metrics/flink-metrics-prometheus/src/test/java/org/apache/flink/metrics/prometheus)
    
    ## Does this pull request potentially affect one of the following parts:
    
      - Dependencies (does it add or upgrade a dependency): (yes)
      - 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? (JavaDocs)


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

    $ git pull https://github.com/lamber-ken/flink master

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

    https://github.com/apache/flink/pull/5857.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 #5857
    
----
commit a3503a5d08e4d02d6cf38d656e2697d3b1197cf1
Author: lamber-ken <!...@...>
Date:   2018-04-16T13:49:56Z

    add prometheus pushgateway reporter

----


---

[GitHub] flink issue #5857: [FLINK-9187][METRICS] add prometheus pushgateway reporter

Posted by lamber-ken <gi...@git.apache.org>.
Github user lamber-ken commented on the issue:

    https://github.com/apache/flink/pull/5857
  
    cc @tillrohrmann : If you have time, please review this PR, thanks.


---

[GitHub] flink issue #5857: [FLINK-9187][METRICS] add prometheus pushgateway reporter

Posted by lamber-ken <gi...@git.apache.org>.
Github user lamber-ken commented on the issue:

    https://github.com/apache/flink/pull/5857
  
    @StephanEwen @zentol , thanks for review this pr
    
    yes, I had check the dependency. 
    
    by the way, in order to distinguish between different prometheus tasks, I defined different jobname for each flink job, so it can collect the metrics of separate project.
    
    although pushgateway has some limits currently, it may be useful in some cases, and pushgateway can be upgraded.


---