From 382184e71a02c122c4eb88495d10b4895dc9578e Mon Sep 17 00:00:00 2001 From: Tom Jorissen Date: Mon, 18 Mar 2024 07:40:44 +0100 Subject: [PATCH] Issue 2161: set metrics endpoint content-type (#4208) * fix: set metrics endpoint content-type when bookie http server is enabled * fix: checkstyle * fix: use existing prometheus content type constant * chore: use minimal dependency / license fixes * Revert "chore: use minimal dependency / license fixes" This reverts commit 7daf8bec6d18f3d467398a39c012871e2fc059aa. * Revert "fix: use existing prometheus content type constant" This reverts commit c265c493ac3b7a55dd34c970a1ac8380bd0e8623. --- .../bookkeeper/http/service/HttpServiceResponse.java | 10 ++++++++++ .../bookkeeper/http/vertx/VertxAbstractHandler.java | 4 ++++ .../bookkeeper/server/http/service/MetricsService.java | 3 +++ .../server/http/service/MetricsServiceTest.java | 6 ++++++ 4 files changed, 23 insertions(+) diff --git a/bookkeeper-http/http-server/src/main/java/org/apache/bookkeeper/http/service/HttpServiceResponse.java b/bookkeeper-http/http-server/src/main/java/org/apache/bookkeeper/http/service/HttpServiceResponse.java index eb300a0effc..75b2a6a7753 100644 --- a/bookkeeper-http/http-server/src/main/java/org/apache/bookkeeper/http/service/HttpServiceResponse.java +++ b/bookkeeper-http/http-server/src/main/java/org/apache/bookkeeper/http/service/HttpServiceResponse.java @@ -28,6 +28,7 @@ */ public class HttpServiceResponse { private String body; + private String contentType; private HttpServer.StatusCode code = HttpServer.StatusCode.OK; public HttpServiceResponse() {} @@ -41,6 +42,10 @@ public String getBody() { return body; } + public String getContentType() { + return contentType; + } + public int getStatusCode() { return code.getValue(); } @@ -50,6 +55,11 @@ public HttpServiceResponse setBody(String body) { return this; } + public HttpServiceResponse setContentType(String contentType) { + this.contentType = contentType; + return this; + } + public HttpServiceResponse setCode(HttpServer.StatusCode code) { this.code = code; return this; diff --git a/bookkeeper-http/vertx-http-server/src/main/java/org/apache/bookkeeper/http/vertx/VertxAbstractHandler.java b/bookkeeper-http/vertx-http-server/src/main/java/org/apache/bookkeeper/http/vertx/VertxAbstractHandler.java index 16c5ffc04b6..9c595511f61 100644 --- a/bookkeeper-http/vertx-http-server/src/main/java/org/apache/bookkeeper/http/vertx/VertxAbstractHandler.java +++ b/bookkeeper-http/vertx-http-server/src/main/java/org/apache/bookkeeper/http/vertx/VertxAbstractHandler.java @@ -20,6 +20,7 @@ */ package org.apache.bookkeeper.http.vertx; +import io.netty.handler.codec.http.HttpHeaderNames; import io.vertx.core.Handler; import io.vertx.core.http.HttpMethod; import io.vertx.core.http.HttpServerRequest; @@ -56,6 +57,9 @@ void processRequest(HttpEndpointService httpEndpointService, RoutingContext cont response = new ErrorHttpService().handle(request); } httpResponse.setStatusCode(response.getStatusCode()); + if (response.getContentType() != null) { + httpResponse.putHeader(HttpHeaderNames.CONTENT_TYPE, response.getContentType()); + } httpResponse.end(response.getBody()); } 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 index 0d62ca9dafe..d3d57f72fa4 100644 --- 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 @@ -35,6 +35,8 @@ */ public class MetricsService implements HttpEndpointService { + public static final String PROMETHEUS_CONTENT_TYPE_004 = "text/plain; version=0.0.4; charset=utf-8"; + private final ServerConfiguration conf; private final StatsProvider statsProvider; @@ -65,6 +67,7 @@ public HttpServiceResponse handle(HttpServiceRequest request) throws Exception { statsProvider.writeAllMetrics(writer); writer.flush(); response.setCode(StatusCode.OK); + response.setContentType(PROMETHEUS_CONTENT_TYPE_004); response.setBody(writer.getBuffer().toString()); } catch (UnsupportedOperationException uoe) { response.setCode(StatusCode.INTERNAL_ERROR); 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 index 3f5e63d60b4..c46dfe29976 100644 --- 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 @@ -19,6 +19,7 @@ package org.apache.bookkeeper.server.http.service; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.CALLS_REAL_METHODS; import static org.mockito.Mockito.doAnswer; @@ -55,6 +56,7 @@ public void testForbiddenMethods() throws Exception { HttpServiceRequest request = new HttpServiceRequest().setMethod(Method.PUT); HttpServiceResponse response = service.handle(request); assertEquals(StatusCode.FORBIDDEN.getValue(), response.getStatusCode()); + assertNull(response.getContentType()); assertEquals( "PUT is forbidden. Should be GET method", response.getBody()); @@ -66,6 +68,7 @@ public void testNullStatsProvider() throws Exception { HttpServiceRequest request = new HttpServiceRequest().setMethod(Method.GET); HttpServiceResponse response = service.handle(request); assertEquals(StatusCode.INTERNAL_ERROR.getValue(), response.getStatusCode()); + assertNull(response.getContentType()); assertEquals( "Stats provider is not enabled. Please enable it by set statsProviderClass" + " on bookie configuration", @@ -86,6 +89,7 @@ public void testWriteMetrics() throws Exception { HttpServiceResponse response = service.handle(request); assertEquals(StatusCode.OK.getValue(), response.getStatusCode()); + assertEquals(MetricsService.PROMETHEUS_CONTENT_TYPE_004, response.getContentType()); assertEquals(content, response.getBody()); } @@ -98,6 +102,7 @@ public void testWriteMetricsException() throws Exception { HttpServiceResponse response = service.handle(request); assertEquals(StatusCode.INTERNAL_ERROR.getValue(), response.getStatusCode()); + assertNull(response.getContentType()); assertEquals("Exceptions are thrown when exporting metrics : write-metrics-exception", response.getBody()); } @@ -111,6 +116,7 @@ public void testWriteMetricsUnimplemented() throws Exception { HttpServiceResponse response = service.handle(request); assertEquals(StatusCode.INTERNAL_ERROR.getValue(), response.getStatusCode()); + assertNull(response.getContentType()); assertEquals("Currently stats provider doesn't support exporting metrics in http service", response.getBody()); }