You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2018/05/07 08:46:42 UTC

[GitHub] sijie closed pull request #1394: Enable Prometheus stats provider to use custom http container

sijie closed pull request #1394: Enable Prometheus stats provider to use custom http container
URL: https://github.com/apache/bookkeeper/pull/1394
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/prometheus/PrometheusMetricsProvider.java b/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/prometheus/PrometheusMetricsProvider.java
index c3dbe2d9c..ee9b541ba 100644
--- a/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/prometheus/PrometheusMetricsProvider.java
+++ b/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/prometheus/PrometheusMetricsProvider.java
@@ -59,15 +59,18 @@
 
     private ScheduledExecutorService executor;
 
-    private static final String PROMETHEUS_STATS_HTTP_PORT = "prometheusStatsHttpPort";
-    private static final int DEFAULT_PROMETHEUS_STATS_HTTP_PORT = 8000;
+    public static final String PROMETHEUS_STATS_HTTP_ENABLE = "prometheusStatsHttpEnable";
+    public static final boolean DEFAULT_PROMETHEUS_STATS_HTTP_ENABLE = true;
 
-    private static final String PROMETHEUS_STATS_LATENCY_ROLLOVER_SECONDS = "prometheusStatsLatencyRolloverSeconds";
-    private static final int DEFAULT_PROMETHEUS_STATS_LATENCY_ROLLOVER_SECONDS = 60;
+    public static final String PROMETHEUS_STATS_HTTP_PORT = "prometheusStatsHttpPort";
+    public static final int DEFAULT_PROMETHEUS_STATS_HTTP_PORT = 8000;
 
-    final CollectorRegistry registry = new CollectorRegistry();
+    public static final String PROMETHEUS_STATS_LATENCY_ROLLOVER_SECONDS = "prometheusStatsLatencyRolloverSeconds";
+    public static final int DEFAULT_PROMETHEUS_STATS_LATENCY_ROLLOVER_SECONDS = 60;
 
-    private Server server;
+    final CollectorRegistry registry;
+
+    Server server;
     private final CachingStatsProvider cachingStatsProvider;
 
     /*
@@ -78,6 +81,11 @@
     final ConcurrentMap<String, DataSketchesOpStatsLogger> opStats = new ConcurrentSkipListMap<>();
 
     public PrometheusMetricsProvider() {
+        this(new CollectorRegistry());
+    }
+
+    public PrometheusMetricsProvider(CollectorRegistry registry) {
+        this.registry = registry;
         this.cachingStatsProvider = new CachingStatsProvider(new StatsProvider() {
             @Override
             public void start(Configuration conf) {
@@ -98,19 +106,23 @@ public StatsLogger getStatsLogger(String scope) {
 
     @Override
     public void start(Configuration conf) {
-        int httpPort = conf.getInt(PROMETHEUS_STATS_HTTP_PORT, DEFAULT_PROMETHEUS_STATS_HTTP_PORT);
-        InetSocketAddress httpEndpoint = InetSocketAddress.createUnresolved("0.0.0.0", httpPort);
-        this.server = new Server(httpEndpoint);
-        ServletContextHandler context = new ServletContextHandler();
-        context.setContextPath("/");
-        server.setHandler(context);
+        boolean httpEnabled = conf.getBoolean(PROMETHEUS_STATS_HTTP_ENABLE, DEFAULT_PROMETHEUS_STATS_HTTP_ENABLE);
+        if (httpEnabled) {
+            int httpPort = conf.getInt(PROMETHEUS_STATS_HTTP_PORT, DEFAULT_PROMETHEUS_STATS_HTTP_PORT);
+            InetSocketAddress httpEndpoint = InetSocketAddress.createUnresolved("0.0.0.0", httpPort);
+            this.server = new Server(httpEndpoint);
+            ServletContextHandler context = new ServletContextHandler();
+            context.setContextPath("/");
+            server.setHandler(context);
 
-        context.addServlet(new ServletHolder(new PrometheusServlet(this)), "/metrics");
+            context.addServlet(new ServletHolder(new PrometheusServlet(this)), "/metrics");
 
-        try {
-            server.start();
-        } catch (Exception e) {
-            throw new RuntimeException(e);
+            try {
+                server.start();
+                log.info("Started Prometheus stats endpoint at {}", httpEndpoint);
+            } catch (Exception e) {
+                throw new RuntimeException(e);
+            }
         }
 
         // Include standard JVM stats
@@ -143,7 +155,6 @@ public double get() {
             rotateLatencyCollection();
         }, 1, latencyRolloverSeconds, TimeUnit.SECONDS);
 
-        log.info("Started Prometheus stats endpoint at {}", httpEndpoint);
     }
 
     @Override
diff --git a/bookkeeper-stats-providers/prometheus-metrics-provider/src/test/java/org/apache/bookkeeper/stats/prometheus/TestPrometheusMetricsProvider.java b/bookkeeper-stats-providers/prometheus-metrics-provider/src/test/java/org/apache/bookkeeper/stats/prometheus/TestPrometheusMetricsProvider.java
index b96997076..a69ac42ae 100644
--- a/bookkeeper-stats-providers/prometheus-metrics-provider/src/test/java/org/apache/bookkeeper/stats/prometheus/TestPrometheusMetricsProvider.java
+++ b/bookkeeper-stats-providers/prometheus-metrics-provider/src/test/java/org/apache/bookkeeper/stats/prometheus/TestPrometheusMetricsProvider.java
@@ -17,11 +17,14 @@
 package org.apache.bookkeeper.stats.prometheus;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
 
 import org.apache.bookkeeper.stats.Counter;
 import org.apache.bookkeeper.stats.OpStatsLogger;
 import org.apache.bookkeeper.stats.StatsLogger;
+import org.apache.commons.configuration.PropertiesConfiguration;
 import org.junit.Test;
 
 /**
@@ -48,6 +51,33 @@ public void testCache() {
         assertSame(scope1, scope2);
     }
 
+    @Test
+    public void testStartNoHttp() {
+        PropertiesConfiguration config = new PropertiesConfiguration();
+        config.setProperty(PrometheusMetricsProvider.PROMETHEUS_STATS_HTTP_ENABLE, false);
+        PrometheusMetricsProvider provider = new PrometheusMetricsProvider();
+        try {
+            provider.start(config);
+            assertNull(provider.server);
+        } finally {
+            provider.stop();
+        }
+    }
+
+    @Test
+    public void testStartWithHttp() {
+        PropertiesConfiguration config = new PropertiesConfiguration();
+        config.setProperty(PrometheusMetricsProvider.PROMETHEUS_STATS_HTTP_ENABLE, true);
+        config.setProperty(PrometheusMetricsProvider.PROMETHEUS_STATS_HTTP_PORT, 0); // ephemeral
+        PrometheusMetricsProvider provider = new PrometheusMetricsProvider();
+        try {
+            provider.start(config);
+            assertNotNull(provider.server);
+        } finally {
+            provider.stop();
+        }
+    }
+
     @Test
     public void testCounter() {
         LongAdderCounter counter = new LongAdderCounter();


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


With regards,
Apache Git Services