You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by si...@apache.org on 2018/06/04 20:52:34 UTC
[bookkeeper] 01/06: Issue #1396: Export metrics in same http server
This is an automated email from the ASF dual-hosted git repository.
sijie pushed a commit to branch branch-4.7
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
commit df4b389058c6c7b11fdf76f08523f97f59c675d3
Author: Sijie Guo <si...@apache.org>
AuthorDate: Mon May 14 13:03:54 2018 -0700
Issue #1396: Export metrics in same http server
Descriptions of the changes in this PR:
*Motivation*
Currently metrics provider and http admin endpoint are isolated due to the way how things are modularized.
This requires two ports to be used, one is for metrics provider to expose metrics, while the other one is used by
http admin endpoint for expose admin endpoints.
When implementing a bookkeeper operator on k8s, it becomes confusing on managing those ports and do health checks.
It would be good to allow export metrics in same http admin endpoint if it is enabled.
*Solution*
- Introduce a method `writeAllMetrics` in stats provider to allow stats provider exporting metrics.
- Add a `MetricsService` in bookie http service and export stats provider's metrics under `/metrics` endpoint.
This fixes #1396
Master Issue: #1396
Author: Sijie Guo <si...@apache.org>
Reviewers: Enrico Olivelli <eo...@gmail.com>, Jia Zhai <None>
This closes #1399 from sijie/same_http_metrics_port, closes #1396
---
.../org/apache/bookkeeper/http/HttpServer.java | 1 +
.../java/org/apache/bookkeeper/server/Main.java | 1 +
.../server/http/BKHttpServiceProvider.java | 18 +++-
.../server/http/service/MetricsService.java | 78 ++++++++++++++
.../server/http/service/MetricsServiceTest.java | 118 +++++++++++++++++++++
.../prometheus/PrometheusMetricsProvider.java | 8 +-
.../prometheus/TestPrometheusMetricsProvider.java | 11 ++
.../org/apache/bookkeeper/stats/StatsProvider.java | 11 ++
8 files changed, 241 insertions(+), 5 deletions(-)
diff --git a/bookkeeper-http/http-server/src/main/java/org/apache/bookkeeper/http/HttpServer.java b/bookkeeper-http/http-server/src/main/java/org/apache/bookkeeper/http/HttpServer.java
index d2c4e81..30e4d05 100644
--- a/bookkeeper-http/http-server/src/main/java/org/apache/bookkeeper/http/HttpServer.java
+++ b/bookkeeper-http/http-server/src/main/java/org/apache/bookkeeper/http/HttpServer.java
@@ -64,6 +64,7 @@ public interface HttpServer {
enum ApiType {
HEARTBEAT,
SERVER_CONFIG,
+ METRICS,
// ledger
DELETE_LEDGER,
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/Main.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/Main.java
index a8c28c7..4815fee 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/Main.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/Main.java
@@ -315,6 +315,7 @@ public class Main {
BKHttpServiceProvider provider = new BKHttpServiceProvider.Builder()
.setBookieServer(bookieService.getServer())
.setServerConfiguration(conf.getServerConf())
+ .setStatsProvider(statsProviderService.getStatsProvider())
.build();
HttpService httpService =
new HttpService(provider, conf, rootStatsLogger);
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/BKHttpServiceProvider.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/BKHttpServiceProvider.java
index 2ffe737..052b50e 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/BKHttpServiceProvider.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/BKHttpServiceProvider.java
@@ -49,10 +49,12 @@ import org.apache.bookkeeper.server.http.service.ListDiskFilesService;
import org.apache.bookkeeper.server.http.service.ListLedgerService;
import org.apache.bookkeeper.server.http.service.ListUnderReplicatedLedgerService;
import org.apache.bookkeeper.server.http.service.LostBookieRecoveryDelayService;
+import org.apache.bookkeeper.server.http.service.MetricsService;
import org.apache.bookkeeper.server.http.service.ReadLedgerEntryService;
import org.apache.bookkeeper.server.http.service.RecoveryBookieService;
import org.apache.bookkeeper.server.http.service.TriggerAuditService;
import org.apache.bookkeeper.server.http.service.WhoIsAuditorService;
+import org.apache.bookkeeper.stats.StatsProvider;
import org.apache.bookkeeper.zookeeper.ZooKeeperClient;
import org.apache.zookeeper.KeeperException;
import org.apache.zookeeper.ZooKeeper;
@@ -67,6 +69,7 @@ import org.apache.zookeeper.ZooKeeper;
@Slf4j
public class BKHttpServiceProvider implements HttpServiceProvider {
+ private final StatsProvider statsProvider;
private final BookieServer bookieServer;
private final AutoRecoveryMain autoRecovery;
private final ServerConfiguration serverConf;
@@ -76,11 +79,13 @@ public class BKHttpServiceProvider implements HttpServiceProvider {
private BKHttpServiceProvider(BookieServer bookieServer,
AutoRecoveryMain autoRecovery,
- ServerConfiguration serverConf)
+ ServerConfiguration serverConf,
+ StatsProvider statsProvider)
throws IOException, KeeperException, InterruptedException, BKException {
this.bookieServer = bookieServer;
this.autoRecovery = autoRecovery;
this.serverConf = serverConf;
+ this.statsProvider = statsProvider;
String zkServers = ZKMetadataDriverBase.resolveZkServers(serverConf);
this.zk = ZooKeeperClient.newBuilder()
.connectString(zkServers)
@@ -134,6 +139,7 @@ public class BKHttpServiceProvider implements HttpServiceProvider {
BookieServer bookieServer = null;
AutoRecoveryMain autoRecovery = null;
ServerConfiguration serverConf = null;
+ StatsProvider statsProvider = null;
public Builder setBookieServer(BookieServer bookieServer) {
this.bookieServer = bookieServer;
@@ -150,12 +156,18 @@ public class BKHttpServiceProvider implements HttpServiceProvider {
return this;
}
+ public Builder setStatsProvider(StatsProvider statsProvider) {
+ this.statsProvider = statsProvider;
+ return this;
+ }
+
public BKHttpServiceProvider build()
throws IOException, KeeperException, InterruptedException, BKException {
return new BKHttpServiceProvider(
bookieServer,
autoRecovery,
- serverConf
+ serverConf,
+ statsProvider
);
}
}
@@ -172,6 +184,8 @@ public class BKHttpServiceProvider implements HttpServiceProvider {
return new HeartbeatService();
case SERVER_CONFIG:
return new ConfigurationService(configuration);
+ case METRICS:
+ return new MetricsService(configuration, statsProvider);
// ledger
case DELETE_LEDGER:
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/service/MetricsService.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/service/MetricsService.java
new file mode 100644
index 0000000..0d62ca9
--- /dev/null
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/service/MetricsService.java
@@ -0,0 +1,78 @@
+/*
+ * 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.bookkeeper.server.http.service;
+
+import java.io.IOException;
+import java.io.StringWriter;
+import org.apache.bookkeeper.conf.ServerConfiguration;
+import org.apache.bookkeeper.http.HttpServer.Method;
+import org.apache.bookkeeper.http.HttpServer.StatusCode;
+import org.apache.bookkeeper.http.service.HttpEndpointService;
+import org.apache.bookkeeper.http.service.HttpServiceRequest;
+import org.apache.bookkeeper.http.service.HttpServiceResponse;
+import org.apache.bookkeeper.stats.StatsProvider;
+
+/**
+ * HttpEndpointService that handle exposing metrics.
+ *
+ * <p>The GET method will return all the emtrics collected at stats provider.
+ */
+public class MetricsService implements HttpEndpointService {
+
+ private final ServerConfiguration conf;
+ private final StatsProvider statsProvider;
+
+ public MetricsService(ServerConfiguration conf,
+ StatsProvider statsProvider) {
+ this.conf = conf;
+ this.statsProvider = statsProvider;
+ }
+
+ @Override
+ public HttpServiceResponse handle(HttpServiceRequest request) throws Exception {
+ HttpServiceResponse response = new HttpServiceResponse();
+ if (Method.GET != request.getMethod()) {
+ response.setCode(StatusCode.FORBIDDEN);
+ response.setBody(request.getMethod() + " is forbidden. Should be GET method");
+ return response;
+ }
+
+ if (null == statsProvider) {
+ response.setCode(StatusCode.INTERNAL_ERROR);
+ response.setBody("Stats provider is not enabled. Please enable it by set statsProviderClass"
+ + " on bookie configuration");
+ return response;
+ }
+
+ // GET
+ try (StringWriter writer = new StringWriter(1024)) {
+ statsProvider.writeAllMetrics(writer);
+ writer.flush();
+ response.setCode(StatusCode.OK);
+ response.setBody(writer.getBuffer().toString());
+ } catch (UnsupportedOperationException uoe) {
+ response.setCode(StatusCode.INTERNAL_ERROR);
+ response.setBody("Currently stats provider doesn't support exporting metrics in http service");
+ } catch (IOException ioe) {
+ response.setCode(StatusCode.INTERNAL_ERROR);
+ response.setBody("Exceptions are thrown when exporting metrics : " + ioe.getMessage());
+ }
+ return response;
+ }
+}
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/server/http/service/MetricsServiceTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/server/http/service/MetricsServiceTest.java
new file mode 100644
index 0000000..3f5e63d
--- /dev/null
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/server/http/service/MetricsServiceTest.java
@@ -0,0 +1,118 @@
+/*
+ * 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.bookkeeper.server.http.service;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.CALLS_REAL_METHODS;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
+
+import java.io.IOException;
+import java.io.Writer;
+import org.apache.bookkeeper.conf.ServerConfiguration;
+import org.apache.bookkeeper.http.HttpServer.Method;
+import org.apache.bookkeeper.http.HttpServer.StatusCode;
+import org.apache.bookkeeper.http.service.HttpServiceRequest;
+import org.apache.bookkeeper.http.service.HttpServiceResponse;
+import org.apache.bookkeeper.stats.StatsProvider;
+import org.junit.Before;
+import org.junit.Test;
+
+/**
+ * Unit test {@link MetricsService}.
+ */
+public class MetricsServiceTest {
+
+ private StatsProvider mockStatsProvider;
+ private MetricsService service;
+
+ @Before
+ public void setup() {
+ this.mockStatsProvider = mock(StatsProvider.class);
+ this.service = new MetricsService(new ServerConfiguration(), mockStatsProvider);
+ }
+
+ @Test
+ public void testForbiddenMethods() throws Exception {
+ HttpServiceRequest request = new HttpServiceRequest().setMethod(Method.PUT);
+ HttpServiceResponse response = service.handle(request);
+ assertEquals(StatusCode.FORBIDDEN.getValue(), response.getStatusCode());
+ assertEquals(
+ "PUT is forbidden. Should be GET method",
+ response.getBody());
+ }
+
+ @Test
+ public void testNullStatsProvider() throws Exception {
+ service = new MetricsService(new ServerConfiguration(), null);
+ HttpServiceRequest request = new HttpServiceRequest().setMethod(Method.GET);
+ HttpServiceResponse response = service.handle(request);
+ assertEquals(StatusCode.INTERNAL_ERROR.getValue(), response.getStatusCode());
+ assertEquals(
+ "Stats provider is not enabled. Please enable it by set statsProviderClass"
+ + " on bookie configuration",
+ response.getBody());
+ }
+
+ @Test
+ public void testWriteMetrics() throws Exception {
+ String content = "test-metrics";
+
+ doAnswer(invocationOnMock -> {
+ Writer writer = invocationOnMock.getArgument(0);
+ writer.write(content);
+ return null;
+ }).when(mockStatsProvider).writeAllMetrics(any(Writer.class));
+
+ HttpServiceRequest request = new HttpServiceRequest().setMethod(Method.GET);
+ HttpServiceResponse response = service.handle(request);
+
+ assertEquals(StatusCode.OK.getValue(), response.getStatusCode());
+ assertEquals(content, response.getBody());
+ }
+
+ @Test
+ public void testWriteMetricsException() throws Exception {
+ doThrow(new IOException("write-metrics-exception"))
+ .when(mockStatsProvider).writeAllMetrics(any(Writer.class));
+
+ HttpServiceRequest request = new HttpServiceRequest().setMethod(Method.GET);
+ HttpServiceResponse response = service.handle(request);
+
+ assertEquals(StatusCode.INTERNAL_ERROR.getValue(), response.getStatusCode());
+ assertEquals("Exceptions are thrown when exporting metrics : write-metrics-exception",
+ response.getBody());
+ }
+
+ @Test
+ public void testWriteMetricsUnimplemented() throws Exception {
+ mockStatsProvider = mock(StatsProvider.class, CALLS_REAL_METHODS);
+ service = new MetricsService(new ServerConfiguration(), mockStatsProvider);
+
+ HttpServiceRequest request = new HttpServiceRequest().setMethod(Method.GET);
+ HttpServiceResponse response = service.handle(request);
+
+ assertEquals(StatusCode.INTERNAL_ERROR.getValue(), response.getStatusCode());
+ assertEquals("Currently stats provider doesn't support exporting metrics in http service",
+ response.getBody());
+ }
+
+}
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 ee9b541..645a686 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
@@ -107,7 +107,9 @@ public class PrometheusMetricsProvider implements StatsProvider {
@Override
public void start(Configuration conf) {
boolean httpEnabled = conf.getBoolean(PROMETHEUS_STATS_HTTP_ENABLE, DEFAULT_PROMETHEUS_STATS_HTTP_ENABLE);
- if (httpEnabled) {
+ boolean bkHttpServerEnabled = conf.getBoolean("httpServerEnabled", false);
+ // only start its own http server when prometheus http is enabled and bk http server is not enabled.
+ if (httpEnabled && !bkHttpServerEnabled) {
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);
@@ -173,8 +175,8 @@ public class PrometheusMetricsProvider implements StatsProvider {
return this.cachingStatsProvider.getStatsLogger(scope);
}
- @VisibleForTesting
- void writeAllMetrics(Writer writer) throws IOException {
+ @Override
+ public void writeAllMetrics(Writer writer) throws IOException {
PrometheusTextFormatUtil.writeMetricsCollectedByPrometheusClient(writer, registry);
gauges.forEach((name, gauge) -> PrometheusTextFormatUtil.writeGauge(writer, name, gauge));
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 a69ac42..8e90e04 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
@@ -21,6 +21,7 @@ import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
+import lombok.Cleanup;
import org.apache.bookkeeper.stats.Counter;
import org.apache.bookkeeper.stats.OpStatsLogger;
import org.apache.bookkeeper.stats.StatsLogger;
@@ -65,6 +66,16 @@ public class TestPrometheusMetricsProvider {
}
@Test
+ public void testStartNoHttpWhenBkHttpEnabled() {
+ PropertiesConfiguration config = new PropertiesConfiguration();
+ config.setProperty(PrometheusMetricsProvider.PROMETHEUS_STATS_HTTP_ENABLE, true);
+ config.setProperty("httpServerEnabled", true);
+ @Cleanup("stop") PrometheusMetricsProvider provider = new PrometheusMetricsProvider();
+ provider.start(config);
+ assertNull(provider.server);
+ }
+
+ @Test
public void testStartWithHttp() {
PropertiesConfiguration config = new PropertiesConfiguration();
config.setProperty(PrometheusMetricsProvider.PROMETHEUS_STATS_HTTP_ENABLE, true);
diff --git a/bookkeeper-stats/src/main/java/org/apache/bookkeeper/stats/StatsProvider.java b/bookkeeper-stats/src/main/java/org/apache/bookkeeper/stats/StatsProvider.java
index 6449e1f..b6e3460 100644
--- a/bookkeeper-stats/src/main/java/org/apache/bookkeeper/stats/StatsProvider.java
+++ b/bookkeeper-stats/src/main/java/org/apache/bookkeeper/stats/StatsProvider.java
@@ -16,6 +16,8 @@
*/
package org.apache.bookkeeper.stats;
+import java.io.IOException;
+import java.io.Writer;
import org.apache.commons.configuration.Configuration;
/**
@@ -36,6 +38,15 @@ public interface StatsProvider {
void stop();
/**
+ *
+ * @param writer
+ * @throws IOException
+ */
+ default void writeAllMetrics(Writer writer) throws IOException {
+ throw new UnsupportedOperationException("writeAllMetrics is not implemented yet");
+ }
+
+ /**
* Return the stats logger to a given <i>scope</i>.
* @param scope
* Scope for the given stats
--
To stop receiving notification emails like this one, please contact
sijie@apache.org.