Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

De-singleton ZPageServer implementation #4935

Merged
merged 5 commits into from
Nov 12, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 21 additions & 14 deletions sdk-extensions/incubator/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -74,42 +75,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<SpanLimits> getTracezTraceConfigSupplier() {
public Supplier<SpanLimits> 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;
}

Expand All @@ -119,7 +124,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));
}

Expand All @@ -138,7 +143,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));
}

Expand All @@ -154,7 +159,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));
}
Expand All @@ -164,7 +169,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);
Expand All @@ -173,7 +178,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;
Expand All @@ -183,6 +188,20 @@ 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 SdkTracerProvider.builder()
.addSpanProcessor(getSpanProcessor())
.setSpanLimits(getTracezTraceConfigSupplier())
.setSampler(getTracezSampler())
.build();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have this accept a SdkTracerProviderBuilder as an argument to make it more flexible.

Also, can we add test coverage for this method?


/**
* Starts a private {@link HttpServer} and registers all zPages to it. When the JVM shuts down the
* server is stopped.
Expand All @@ -193,13 +212,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();
}

Expand All @@ -208,10 +227,8 @@ public static void startHttpServerAndRegisterAllPages(int port) throws IOExcepti
new Thread() {
@Override
public void run() {
ZPageServer.stop();
ZPageServer.this.stop();
}
});
}

private ZPageServer() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}