You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/06/03 08:16:21 UTC

[GitHub] [pulsar] hangc0276 opened a new pull request #10803: use zookeeper prometheus metric provider to export zookeeper metrics

hangc0276 opened a new pull request #10803:
URL: https://github.com/apache/pulsar/pull/10803


   ### Motivation
   After upgrade zookeeper version to 3.6.2 in #8590 and removed AspectJ based metrics for ZooKeeper in #10533, the zookeeper's prometheus metric has lost if we start zookeeper by `bin/puldar-daemon start zookeeper`.
   
   Due to zookeeper 3.6.0+ has add internal prometheus metric provider, so we can turn on by default in pulsar.
   
   ### Modification
   1. turn on zookeeper prometheus metric provider by default in `conf/zookeeper.conf` and use 8000 as default port sync with old zookeeper metric port
   2. add grafana panel for new zookeeper metrics
   3. remove old prometheus metric provider in `ZooKeeperStarter` and `ConfigurationStoreStarter`.
   4. update reference-metric.md doc


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] merlimat commented on a change in pull request #10803: use zookeeper prometheus metric provider to export zookeeper metrics

Posted by GitBox <gi...@apache.org>.
merlimat commented on a change in pull request #10803:
URL: https://github.com/apache/pulsar/pull/10803#discussion_r644817992



##########
File path: pulsar-zookeeper/src/main/java/org/apache/pulsar/zookeeper/ZooKeeperStarter.java
##########
@@ -18,49 +18,11 @@
  */
 package org.apache.pulsar.zookeeper;
 
-import java.net.InetSocketAddress;
-
 import org.apache.zookeeper.server.quorum.QuorumPeerMain;
-import org.eclipse.jetty.server.Server;
-import org.eclipse.jetty.servlet.ServletContextHandler;
-import org.eclipse.jetty.servlet.ServletHolder;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import io.prometheus.client.exporter.MetricsServlet;
-import io.prometheus.client.hotspot.DefaultExports;
 
 public class ZooKeeperStarter {
     public static void main(String[] args) throws Exception {
-        start(args, "8000");
-    }
-
-    protected static void start(String[] args, String defaultStatsPort) throws Exception {
-        // Register basic JVM metrics
-        DefaultExports.initialize();
-
-        // Start Jetty to serve stats
-        int port = Integer.parseInt(System.getProperties().getProperty("stats_server_port", defaultStatsPort));
-
-        log.info("Starting ZK stats HTTP server at port {}", port);
-        InetSocketAddress httpEndpoint = InetSocketAddress.createUnresolved("0.0.0.0", port);
-
-        Server server = new Server(httpEndpoint);
-        ServletContextHandler context = new ServletContextHandler();
-        context.setContextPath("/");
-        server.setHandler(context);
-        context.addServlet(new ServletHolder(new MetricsServlet()), "/metrics");
-        try {
-            server.start();
-        } catch (Exception e) {
-            log.error("Failed to start HTTP server at port {}. Use \"-Dstats_server_port=1234\" to change port number",
-                    port, e);
-            throw e;
-        }
-
         // Start the regular ZooKeeper server
         QuorumPeerMain.main(args);

Review comment:
       Since there is nothing left in this class, we can remove it and just call QuorumMain from the CLI scripts 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] merlimat commented on a change in pull request #10803: use zookeeper prometheus metric provider to export zookeeper metrics

Posted by GitBox <gi...@apache.org>.
merlimat commented on a change in pull request #10803:
URL: https://github.com/apache/pulsar/pull/10803#discussion_r644817992



##########
File path: pulsar-zookeeper/src/main/java/org/apache/pulsar/zookeeper/ZooKeeperStarter.java
##########
@@ -18,49 +18,11 @@
  */
 package org.apache.pulsar.zookeeper;
 
-import java.net.InetSocketAddress;
-
 import org.apache.zookeeper.server.quorum.QuorumPeerMain;
-import org.eclipse.jetty.server.Server;
-import org.eclipse.jetty.servlet.ServletContextHandler;
-import org.eclipse.jetty.servlet.ServletHolder;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import io.prometheus.client.exporter.MetricsServlet;
-import io.prometheus.client.hotspot.DefaultExports;
 
 public class ZooKeeperStarter {
     public static void main(String[] args) throws Exception {
-        start(args, "8000");
-    }
-
-    protected static void start(String[] args, String defaultStatsPort) throws Exception {
-        // Register basic JVM metrics
-        DefaultExports.initialize();
-
-        // Start Jetty to serve stats
-        int port = Integer.parseInt(System.getProperties().getProperty("stats_server_port", defaultStatsPort));
-
-        log.info("Starting ZK stats HTTP server at port {}", port);
-        InetSocketAddress httpEndpoint = InetSocketAddress.createUnresolved("0.0.0.0", port);
-
-        Server server = new Server(httpEndpoint);
-        ServletContextHandler context = new ServletContextHandler();
-        context.setContextPath("/");
-        server.setHandler(context);
-        context.addServlet(new ServletHolder(new MetricsServlet()), "/metrics");
-        try {
-            server.start();
-        } catch (Exception e) {
-            log.error("Failed to start HTTP server at port {}. Use \"-Dstats_server_port=1234\" to change port number",
-                    port, e);
-            throw e;
-        }
-
         // Start the regular ZooKeeper server
         QuorumPeerMain.main(args);

Review comment:
       Since there is nothing left in this class, we can remove it and just call QuorumMain from the CLI scripts 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] hangc0276 commented on a change in pull request #10803: use zookeeper prometheus metric provider to export zookeeper metrics

Posted by GitBox <gi...@apache.org>.
hangc0276 commented on a change in pull request #10803:
URL: https://github.com/apache/pulsar/pull/10803#discussion_r644820249



##########
File path: pulsar-zookeeper/src/main/java/org/apache/pulsar/zookeeper/ZooKeeperStarter.java
##########
@@ -18,49 +18,11 @@
  */
 package org.apache.pulsar.zookeeper;
 
-import java.net.InetSocketAddress;
-
 import org.apache.zookeeper.server.quorum.QuorumPeerMain;
-import org.eclipse.jetty.server.Server;
-import org.eclipse.jetty.servlet.ServletContextHandler;
-import org.eclipse.jetty.servlet.ServletHolder;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import io.prometheus.client.exporter.MetricsServlet;
-import io.prometheus.client.hotspot.DefaultExports;
 
 public class ZooKeeperStarter {
     public static void main(String[] args) throws Exception {
-        start(args, "8000");
-    }
-
-    protected static void start(String[] args, String defaultStatsPort) throws Exception {
-        // Register basic JVM metrics
-        DefaultExports.initialize();
-
-        // Start Jetty to serve stats
-        int port = Integer.parseInt(System.getProperties().getProperty("stats_server_port", defaultStatsPort));
-
-        log.info("Starting ZK stats HTTP server at port {}", port);
-        InetSocketAddress httpEndpoint = InetSocketAddress.createUnresolved("0.0.0.0", port);
-
-        Server server = new Server(httpEndpoint);
-        ServletContextHandler context = new ServletContextHandler();
-        context.setContextPath("/");
-        server.setHandler(context);
-        context.addServlet(new ServletHolder(new MetricsServlet()), "/metrics");
-        try {
-            server.start();
-        } catch (Exception e) {
-            log.error("Failed to start HTTP server at port {}. Use \"-Dstats_server_port=1234\" to change port number",
-                    port, e);
-            throw e;
-        }
-
         // Start the regular ZooKeeper server
         QuorumPeerMain.main(args);

Review comment:
       OK,just call QuorumMain from CLI scripts is better, i update it.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] sijie commented on pull request #10803: use zookeeper prometheus metric provider to export zookeeper metrics

Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #10803:
URL: https://github.com/apache/pulsar/pull/10803#issuecomment-853707894


   This sounds like a bug that should be highlighted in the release notes of 2.8.0


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] codelipenghui merged pull request #10803: use zookeeper prometheus metric provider to export zookeeper metrics

Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #10803:
URL: https://github.com/apache/pulsar/pull/10803


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] codelipenghui merged pull request #10803: use zookeeper prometheus metric provider to export zookeeper metrics

Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #10803:
URL: https://github.com/apache/pulsar/pull/10803


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] sijie commented on pull request #10803: use zookeeper prometheus metric provider to export zookeeper metrics

Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #10803:
URL: https://github.com/apache/pulsar/pull/10803#issuecomment-853707894


   This sounds like a bug that should be highlighted in the release notes of 2.8.0


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] hangc0276 commented on a change in pull request #10803: use zookeeper prometheus metric provider to export zookeeper metrics

Posted by GitBox <gi...@apache.org>.
hangc0276 commented on a change in pull request #10803:
URL: https://github.com/apache/pulsar/pull/10803#discussion_r644820249



##########
File path: pulsar-zookeeper/src/main/java/org/apache/pulsar/zookeeper/ZooKeeperStarter.java
##########
@@ -18,49 +18,11 @@
  */
 package org.apache.pulsar.zookeeper;
 
-import java.net.InetSocketAddress;
-
 import org.apache.zookeeper.server.quorum.QuorumPeerMain;
-import org.eclipse.jetty.server.Server;
-import org.eclipse.jetty.servlet.ServletContextHandler;
-import org.eclipse.jetty.servlet.ServletHolder;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import io.prometheus.client.exporter.MetricsServlet;
-import io.prometheus.client.hotspot.DefaultExports;
 
 public class ZooKeeperStarter {
     public static void main(String[] args) throws Exception {
-        start(args, "8000");
-    }
-
-    protected static void start(String[] args, String defaultStatsPort) throws Exception {
-        // Register basic JVM metrics
-        DefaultExports.initialize();
-
-        // Start Jetty to serve stats
-        int port = Integer.parseInt(System.getProperties().getProperty("stats_server_port", defaultStatsPort));
-
-        log.info("Starting ZK stats HTTP server at port {}", port);
-        InetSocketAddress httpEndpoint = InetSocketAddress.createUnresolved("0.0.0.0", port);
-
-        Server server = new Server(httpEndpoint);
-        ServletContextHandler context = new ServletContextHandler();
-        context.setContextPath("/");
-        server.setHandler(context);
-        context.addServlet(new ServletHolder(new MetricsServlet()), "/metrics");
-        try {
-            server.start();
-        } catch (Exception e) {
-            log.error("Failed to start HTTP server at port {}. Use \"-Dstats_server_port=1234\" to change port number",
-                    port, e);
-            throw e;
-        }
-
         // Start the regular ZooKeeper server
         QuorumPeerMain.main(args);

Review comment:
       OK,just call QuorumMain from CLI scripts is better, i update it.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org