From e0eb744e529d32383dfa2d78f65296168350dea7 Mon Sep 17 00:00:00 2001 From: jason plumb <75337021+breedx-splk@users.noreply.github.com> Date: Sat, 12 Nov 2022 06:49:56 -0800 Subject: [PATCH] De-singleton ZPageServer implementation (#4935) * remove the static/singleton nature from ZPageServer * add convenience method for creating zpages-based sdktracerprovider * update readme for zpageserver * oh checkstyle. * overload build and add test --- sdk-extensions/incubator/README.md | 35 +++++---- .../incubator/trace/zpages/ZPageServer.java | 74 +++++++++++++------ .../trace/zpages/ZPageServerTest.java | 23 +++++- 3 files changed, 91 insertions(+), 41 deletions(-) diff --git a/sdk-extensions/incubator/README.md b/sdk-extensions/incubator/README.md index c497fb2e6d9..ef52abea68d 100644 --- a/sdk-extensions/incubator/README.md +++ b/sdk-extensions/incubator/README.md @@ -108,30 +108,37 @@ make sure your version of the JDK includes this package. To setup the zPages, register zPages with your `OpenTelemetrySdk` and -call `ZPageServer.startHttpServerAndRegisterAllPages(int port)`: +call `startHttpServerAndRegisterAllPages(int port)` on your ZPageServer instance: ```java public class MyMainClass { public static void main(String[] args) throws Exception { + // Create a new ZPageServer + ZPageServer zpageServer = ZPageServer.create(); // Configure OpenTelemetrySdk with zPages - OpenTelemetry openTelemetry = OpenTelemetrySdk.builder() - .setTracerProvider( - SdkTracerProvider.builder() - .addSpanProcessor(ZPageServer.getSpanProcessor()) - .setSpanLimits(ZPageServer.getTracezTraceConfigSupplier()) - .setSampler(ZPageServer.getTracezSampler()) - .build()) - .build(); + OpenTelemetry openTelemetry = + OpenTelemetrySdk.builder().setTracerProvider(zpageServer.buildSdkTracerProvider()).build(); // Start zPages server - ZPageServer.startHttpServerAndRegisterAllPages(8080); - // ... do work + zpageServer.startHttpServerAndRegisterAllPages(8080); + // ...Do work (this is just an example) + long count = 0; + while (true) { + Tracer tracer = openTelemetry.getTracer("demo"); + Span span = tracer.spanBuilder("exampleSpan" + ++count).startSpan(); + try (Scope scope = span.makeCurrent()) { + System.out.println("Inside a span..."); + TimeUnit.SECONDS.sleep(2); + } + span.end(); + } } } ``` -Alternatively, you can call `ZPageServer.registerAllPagesToHttpServer(HttpServer server)` to -register the zPages to a shared server: +Note that `startHttpServerAndRegisterAllPages()` will create a new `HttpServer` and register the zPages +with it. If you already have an existing or shared `HttpServer`, you can instead call +`registerAllPagesToHttpServer(HttpServer server)`: ```java public class MyMainClass { @@ -140,7 +147,7 @@ public class MyMainClass { // Start zPages server HttpServer server = HttpServer.create(new InetSocketAddress(8000), 10); - ZPageServer.registerAllPagesToHttpServer(server); + zPageServer.registerAllPagesToHttpServer(server); server.start(); // ... do work } diff --git a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/trace/zpages/ZPageServer.java b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/trace/zpages/ZPageServer.java index 74bb2f7b7e8..2688c0ad8b2 100644 --- a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/trace/zpages/ZPageServer.java +++ b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/trace/zpages/ZPageServer.java @@ -7,6 +7,8 @@ import com.sun.net.httpserver.HttpServer; import io.opentelemetry.api.internal.GuardedBy; +import io.opentelemetry.sdk.trace.SdkTracerProvider; +import io.opentelemetry.sdk.trace.SdkTracerProviderBuilder; import io.opentelemetry.sdk.trace.SpanLimits; import io.opentelemetry.sdk.trace.SpanProcessor; import io.opentelemetry.sdk.trace.samplers.Sampler; @@ -74,42 +76,46 @@ public final class ZPageServer { // Length of time to wait for the HttpServer to stop private static final int HTTPSERVER_STOP_DELAY = 1; // Tracez SpanProcessor and DataAggregator for constructing TracezZPageHandler - private static final TracezSpanProcessor tracezSpanProcessor = - TracezSpanProcessor.builder().build(); - private static final TracezTraceConfigSupplier tracezTraceConfigSupplier = + private final TracezSpanProcessor tracezSpanProcessor = TracezSpanProcessor.builder().build(); + private final TracezTraceConfigSupplier tracezTraceConfigSupplier = new TracezTraceConfigSupplier(); - private static final TracezDataAggregator tracezDataAggregator = + private final TracezDataAggregator tracezDataAggregator = new TracezDataAggregator(tracezSpanProcessor); // Handler for /tracez page - private static final ZPageHandler tracezZPageHandler = - new TracezZPageHandler(tracezDataAggregator); + private final ZPageHandler tracezZPageHandler = new TracezZPageHandler(tracezDataAggregator); // Handler for /traceconfigz page - private static final ZPageHandler traceConfigzZPageHandler = + private final ZPageHandler traceConfigzZPageHandler = new TraceConfigzZPageHandler(tracezTraceConfigSupplier); // Handler for index page, **please include all available ZPageHandlers in the constructor** - private static final ZPageHandler indexZPageHandler = + private final ZPageHandler indexZPageHandler = new IndexZPageHandler(Arrays.asList(tracezZPageHandler, traceConfigzZPageHandler)); - private static final Object mutex = new Object(); + private final Object mutex = new Object(); @GuardedBy("mutex") @Nullable - private static HttpServer server; + private HttpServer server; + + private ZPageServer() {} + + public static ZPageServer create() { + return new ZPageServer(); + } /** Returns a supplier of {@link SpanLimits} which can be reconfigured using zpages. */ - public static Supplier getTracezTraceConfigSupplier() { + public Supplier getTracezTraceConfigSupplier() { return tracezTraceConfigSupplier; } /** Returns a {@link Sampler} which can be reconfigured using zpages. */ - public static Sampler getTracezSampler() { + public Sampler getTracezSampler() { return tracezTraceConfigSupplier; } /** * Returns a {@link SpanProcessor} which will allow processing of spans by {@link ZPageServer}. */ - public static SpanProcessor getSpanProcessor() { + public SpanProcessor getSpanProcessor() { return tracezSpanProcessor; } @@ -119,7 +125,7 @@ public static SpanProcessor getSpanProcessor() { * * @param server the {@link HttpServer} for the page to register to. */ - static void registerIndexZPageHandler(HttpServer server) { + private void registerIndexZPageHandler(HttpServer server) { server.createContext(indexZPageHandler.getUrlPath(), new ZPageHttpHandler(indexZPageHandler)); } @@ -138,7 +144,7 @@ static void registerIndexZPageHandler(HttpServer server) { * * @param server the {@link HttpServer} for the page to register to. */ - static void registerTracezZPageHandler(HttpServer server) { + private void registerTracezZPageHandler(HttpServer server) { server.createContext(tracezZPageHandler.getUrlPath(), new ZPageHttpHandler(tracezZPageHandler)); } @@ -154,7 +160,7 @@ static void registerTracezZPageHandler(HttpServer server) { * * @param server the {@link HttpServer} for the page to register to. */ - static void registerTraceConfigzZPageHandler(HttpServer server) { + private void registerTraceConfigzZPageHandler(HttpServer server) { server.createContext( traceConfigzZPageHandler.getUrlPath(), new ZPageHttpHandler(traceConfigzZPageHandler)); } @@ -164,7 +170,7 @@ static void registerTraceConfigzZPageHandler(HttpServer server) { * * @param server the {@link HttpServer} for the page to register to. */ - public static void registerAllPagesToHttpServer(HttpServer server) { + public void registerAllPagesToHttpServer(HttpServer server) { // For future zPages, register them to the server in here registerIndexZPageHandler(server); registerTracezZPageHandler(server); @@ -173,7 +179,7 @@ public static void registerAllPagesToHttpServer(HttpServer server) { } /** Method for stopping the {@link HttpServer} {@code server}. */ - private static void stop() { + private void stop() { synchronized (mutex) { if (server == null) { return; @@ -183,6 +189,30 @@ private static void stop() { } } + /** + * Convenience method to return a new SdkTracerProvider that has been configured with our ZPage + * specific span processor, sampler, and limits. + * + * @return new SdkTracerProvider + */ + public SdkTracerProvider buildSdkTracerProvider() { + return buildSdkTracerProvider(SdkTracerProvider.builder()); + } + + /** + * Convenience method to return a new SdkTracerProvider that has been configured with our ZPage + * specific span processor, sampler, and limits. + * + * @return new SdkTracerProvider + */ + public SdkTracerProvider buildSdkTracerProvider(SdkTracerProviderBuilder builder) { + return builder + .addSpanProcessor(getSpanProcessor()) + .setSpanLimits(getTracezTraceConfigSupplier()) + .setSampler(getTracezSampler()) + .build(); + } + /** * Starts a private {@link HttpServer} and registers all zPages to it. When the JVM shuts down the * server is stopped. @@ -193,13 +223,13 @@ private static void stop() { * @throws IllegalStateException if the server is already started. * @throws IOException if the server cannot bind to the specified port. */ - public static void startHttpServerAndRegisterAllPages(int port) throws IOException { + public void startHttpServerAndRegisterAllPages(int port) throws IOException { synchronized (mutex) { if (server != null) { throw new IllegalStateException("The HttpServer is already started."); } server = HttpServer.create(new InetSocketAddress(port), HTTPSERVER_BACKLOG); - ZPageServer.registerAllPagesToHttpServer(server); + registerAllPagesToHttpServer(server); server.start(); } @@ -208,10 +238,8 @@ public static void startHttpServerAndRegisterAllPages(int port) throws IOExcepti new Thread() { @Override public void run() { - ZPageServer.stop(); + ZPageServer.this.stop(); } }); } - - private ZPageServer() {} } diff --git a/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/trace/zpages/ZPageServerTest.java b/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/trace/zpages/ZPageServerTest.java index 2ba0e7726bd..a2e1a2c659f 100644 --- a/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/trace/zpages/ZPageServerTest.java +++ b/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/trace/zpages/ZPageServerTest.java @@ -7,23 +7,38 @@ import static org.assertj.core.api.Assertions.assertThat; +import io.opentelemetry.sdk.trace.SdkTracerProvider; +import io.opentelemetry.sdk.trace.SpanLimits; import org.junit.jupiter.api.Test; class ZPageServerTest { @Test void spanProcessor() { - assertThat(ZPageServer.getSpanProcessor()).isInstanceOf(TracezSpanProcessor.class); + ZPageServer server = ZPageServer.create(); + assertThat(server.getSpanProcessor()).isInstanceOf(TracezSpanProcessor.class); } @Test void traceConfigSupplier() { - assertThat(ZPageServer.getTracezTraceConfigSupplier()) - .isInstanceOf(TracezTraceConfigSupplier.class); + ZPageServer server = ZPageServer.create(); + assertThat(server.getTracezTraceConfigSupplier()).isInstanceOf(TracezTraceConfigSupplier.class); } @Test void testSampler() { - assertThat(ZPageServer.getTracezSampler()).isInstanceOf(TracezTraceConfigSupplier.class); + ZPageServer server = ZPageServer.create(); + assertThat(server.getTracezSampler()).isInstanceOf(TracezTraceConfigSupplier.class); + } + + @Test + void buildTracerProvider() { + ZPageServer server = ZPageServer.create(); + SpanLimits expectedLimits = server.getTracezTraceConfigSupplier().get(); + + try (SdkTracerProvider provider = server.buildSdkTracerProvider()) { + assertThat(provider.getSpanLimits()).isEqualTo(expectedLimits); + assertThat(provider.getSampler()).isSameAs(server.getTracezSampler()); + } } }