From b1efc40da8c2c4eb4577d52b26929d5467e6fee8 Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Thu, 10 Nov 2022 15:36:09 -0800 Subject: [PATCH 1/5] remove the static/singleton nature from ZPageServer --- .../incubator/trace/zpages/ZPageServer.java | 47 ++++++++++--------- .../trace/zpages/ZPageServerTest.java | 9 ++-- 2 files changed, 32 insertions(+), 24 deletions(-) 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..d9b0233f0fa 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 @@ -74,42 +74,49 @@ 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 = + private final TracezSpanProcessor tracezSpanProcessor = TracezSpanProcessor.builder().build(); - private static final TracezTraceConfigSupplier tracezTraceConfigSupplier = + 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 = + 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 +126,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 +145,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 +161,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 +171,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 +180,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; @@ -193,13 +200,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 +215,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..d50f1de27f2 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 @@ -13,17 +13,20 @@ class ZPageServerTest { @Test void spanProcessor() { - assertThat(ZPageServer.getSpanProcessor()).isInstanceOf(TracezSpanProcessor.class); + ZPageServer zPageServer = ZPageServer.create(); + assertThat(zPageServer.getSpanProcessor()).isInstanceOf(TracezSpanProcessor.class); } @Test void traceConfigSupplier() { - assertThat(ZPageServer.getTracezTraceConfigSupplier()) + ZPageServer zPageServer = ZPageServer.create(); + assertThat(zPageServer.getTracezTraceConfigSupplier()) .isInstanceOf(TracezTraceConfigSupplier.class); } @Test void testSampler() { - assertThat(ZPageServer.getTracezSampler()).isInstanceOf(TracezTraceConfigSupplier.class); + ZPageServer zPageServer = ZPageServer.create(); + assertThat(zPageServer.getTracezSampler()).isInstanceOf(TracezTraceConfigSupplier.class); } } From 48af61d022f1c6f340486ace9d1acd3aaf9ab9ea Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Thu, 10 Nov 2022 15:42:17 -0800 Subject: [PATCH 2/5] add convenience method for creating zpages-based sdktracerprovider --- .../incubator/trace/zpages/ZPageServer.java | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) 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 d9b0233f0fa..b164a1fe88f 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,7 @@ import com.sun.net.httpserver.HttpServer; import io.opentelemetry.api.internal.GuardedBy; +import io.opentelemetry.sdk.trace.SdkTracerProvider; import io.opentelemetry.sdk.trace.SpanLimits; import io.opentelemetry.sdk.trace.SpanProcessor; import io.opentelemetry.sdk.trace.samplers.Sampler; @@ -74,15 +75,13 @@ 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 final TracezSpanProcessor tracezSpanProcessor = - TracezSpanProcessor.builder().build(); + private final TracezSpanProcessor tracezSpanProcessor = TracezSpanProcessor.builder().build(); private final TracezTraceConfigSupplier tracezTraceConfigSupplier = new TracezTraceConfigSupplier(); private final TracezDataAggregator tracezDataAggregator = new TracezDataAggregator(tracezSpanProcessor); // Handler for /tracez page - private final ZPageHandler tracezZPageHandler = - new TracezZPageHandler(tracezDataAggregator); + private final ZPageHandler tracezZPageHandler = new TracezZPageHandler(tracezDataAggregator); // Handler for /traceconfigz page private final ZPageHandler traceConfigzZPageHandler = new TraceConfigzZPageHandler(tracezTraceConfigSupplier); @@ -96,8 +95,7 @@ public final class ZPageServer { @Nullable private HttpServer server; - private ZPageServer() { - } + private ZPageServer() {} public static ZPageServer create() { return new ZPageServer(); @@ -190,6 +188,20 @@ private 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 SdkTracerProvider.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. From 18609c882162619ca06dc4818c80a0592ccb3e80 Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Thu, 10 Nov 2022 15:49:13 -0800 Subject: [PATCH 3/5] update readme for zpageserver --- sdk-extensions/incubator/README.md | 35 ++++++++++++++++++------------ 1 file changed, 21 insertions(+), 14 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 } From 84e6a4f44abb7709a789a2833fdd079f61d4d425 Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Thu, 10 Nov 2022 18:48:53 -0800 Subject: [PATCH 4/5] oh checkstyle. --- .../incubator/trace/zpages/ZPageServerTest.java | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) 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 d50f1de27f2..6deeeb4572c 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 @@ -13,20 +13,19 @@ class ZPageServerTest { @Test void spanProcessor() { - ZPageServer zPageServer = ZPageServer.create(); - assertThat(zPageServer.getSpanProcessor()).isInstanceOf(TracezSpanProcessor.class); + ZPageServer server = ZPageServer.create(); + assertThat(server.getSpanProcessor()).isInstanceOf(TracezSpanProcessor.class); } @Test void traceConfigSupplier() { - ZPageServer zPageServer = ZPageServer.create(); - assertThat(zPageServer.getTracezTraceConfigSupplier()) - .isInstanceOf(TracezTraceConfigSupplier.class); + ZPageServer server = ZPageServer.create(); + assertThat(server.getTracezTraceConfigSupplier()).isInstanceOf(TracezTraceConfigSupplier.class); } @Test void testSampler() { - ZPageServer zPageServer = ZPageServer.create(); - assertThat(zPageServer.getTracezSampler()).isInstanceOf(TracezTraceConfigSupplier.class); + ZPageServer server = ZPageServer.create(); + assertThat(server.getTracezSampler()).isInstanceOf(TracezTraceConfigSupplier.class); } } From 4ecb895bccbcb8130f3a7ad1cb7776a56827d2f3 Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Fri, 11 Nov 2022 11:05:00 -0800 Subject: [PATCH 5/5] overload build and add test --- .../incubator/trace/zpages/ZPageServer.java | 13 ++++++++++++- .../incubator/trace/zpages/ZPageServerTest.java | 13 +++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) 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 b164a1fe88f..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 @@ -8,6 +8,7 @@ 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; @@ -195,7 +196,17 @@ private void stop() { * @return new SdkTracerProvider */ public SdkTracerProvider buildSdkTracerProvider() { - return SdkTracerProvider.builder() + 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()) 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 6deeeb4572c..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,6 +7,8 @@ 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 { @@ -28,4 +30,15 @@ void testSampler() { 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()); + } + } }