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/14 20:04:06 UTC
[GitHub] sijie closed pull request #1399: Issue #1396: Export metrics in
same http server
sijie closed pull request #1399: Issue #1396: Export metrics in same http server
URL: https://github.com/apache/bookkeeper/pull/1399
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-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 d2c4e8193..30e4d05ae 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 int getValue() {
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 a8c28c7c8..4815fee0c 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 @@ static LifecycleComponentStack buildBookieServer(BookieConfiguration conf) throw
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 2ffe737b2..052b50ecf 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.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 @@
@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 @@
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 @@ private Bookie getBookie() {
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 Builder setServerConfiguration(ServerConfiguration conf) {
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 HttpEndpointService provideHttpEndpointService(ApiType type) {
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 000000000..0d62ca9da
--- /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 000000000..3f5e63d60
--- /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 ee9b541ba..645a686a4 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 StatsLogger getStatsLogger(String scope) {
@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 StatsLogger getStatsLogger(String scope) {
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 a69ac42ae..8e90e0402 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.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;
@@ -64,6 +65,16 @@ public void testStartNoHttp() {
}
}
+ @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();
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 6449e1f84..b6e34600d 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;
/**
@@ -35,6 +37,15 @@
*/
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
----------------------------------------------------------------
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