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

Tracing performance optimization. #1916

Merged
merged 4 commits into from
Jun 2, 2020
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
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2019, 2020 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -25,7 +25,6 @@
import io.helidon.common.context.Context;
import io.helidon.common.context.Contexts;

import io.opentracing.SpanContext;
import org.eclipse.microprofile.faulttolerance.Asynchronous;
import org.eclipse.microprofile.faulttolerance.Timeout;

Expand All @@ -44,7 +43,7 @@ public class HelloBean {
public String getHello() {
Context context = Contexts.context().get();
Objects.requireNonNull(context);
Objects.requireNonNull(context.get(SpanContext.class).get());

return "Hello World";
}

Expand All @@ -57,7 +56,8 @@ public String getHello() {
public String getHelloTimeout() {
Context context = Contexts.context().get();
Objects.requireNonNull(context);
Objects.requireNonNull(context.get(SpanContext.class).get());
// span context may be null if tracer is not configured
//Objects.requireNonNull(context.get(SpanContext.class).get());
return "Hello World";
}

Expand All @@ -70,7 +70,7 @@ public String getHelloTimeout() {
public CompletionStage<String> getHelloAsync() {
Context context = Contexts.context().get();
Objects.requireNonNull(context);
Objects.requireNonNull(context.get(SpanContext.class).get());
//Objects.requireNonNull(context.get(SpanContext.class).get());
return CompletableFuture.completedFuture("Hello World");
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2019, 2020 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2019, 2020 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -55,19 +55,19 @@ static void destroyClass() {
}

@Test
void TestHello() {
void testHello() {
WebTarget target = baseTarget.path("/hello");
assertOk(target.request().get(), "Hello World");
}

@Test
void TestHelloTimeout() {
void testHelloTimeout() {
WebTarget target = baseTarget.path("/helloTimeout");
assertOk(target.request().get(), "Hello World");
}

@Test
void TestHelloAsync() {
void testHelloAsync() {
WebTarget target = baseTarget.path("/helloAsync");
assertOk(target.request().get(), "Hello World");
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2017, 2019 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2017, 2020 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -71,7 +71,8 @@ public class JerseyExampleResource {
public Response webServerInjection() {
return Response.ok("request=" + request.getClass().getName()
+ "\nresponse=" + response.getClass().getName()
+ "\nspanContext=" + spanContext.getClass().getName()).build();
+ "\nspanContext=" + (null == spanContext ? null : spanContext.getClass().getName()))
.build();
}

@GET
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2017, 2019 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2017, 2020 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -54,9 +54,6 @@ public class JerseySupportTest {
public static void startServerAndClient() throws Exception {
JerseyExampleMain.INSTANCE.webServer(true);
webTarget = JerseyExampleMain.INSTANCE.target();

// JerseyGrizzlyRiMain.main(null);
// webTarget = JerseyExampleMain.client().target("http://localhost:8080");
}

@Test
Expand All @@ -76,7 +73,7 @@ public void injection() throws Exception {
doAssert(response,
"request=io.helidon.webserver.RequestRouting$RoutedRequest\n"
+ "response=io.helidon.webserver.RequestRouting$RoutedResponse\n"
+ "spanContext=io.opentracing.noop.NoopSpanContextImpl");
+ "spanContext=null");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019 Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2019, 2020 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -20,6 +20,7 @@
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.stream.Collectors;

import io.helidon.common.context.Context;
Expand All @@ -33,6 +34,7 @@
import io.opentracing.Span;
import io.opentracing.SpanContext;
import io.opentracing.Tracer;
import io.opentracing.noop.NoopScopeManager;
import io.opentracing.noop.NoopSpanBuilder;
import io.opentracing.propagation.Format;
import io.opentracing.propagation.TextMapAdapter;
Expand Down Expand Up @@ -224,13 +226,24 @@ public void accept(ServerRequest req, ServerResponse res) {

static final class RequestSpanHandler implements Handler {
private static final String TRACING_SPAN_HTTP_REQUEST = "HTTP Request";
private final AtomicBoolean checkedIfShouldTrace = new AtomicBoolean();
private volatile boolean shouldTrace = true;

RequestSpanHandler() {
}

@Override
public void accept(ServerRequest req, ServerResponse res) {
doAccept(req, res);
if (checkedIfShouldTrace.compareAndSet(false, true)) {
tomas-langer marked this conversation as resolved.
Show resolved Hide resolved
if (req.tracer().scopeManager() instanceof NoopScopeManager) {
shouldTrace = false;
}
}

if (shouldTrace) {
doAccept(req, res);
}

req.next();
}

Expand All @@ -257,22 +270,6 @@ private void doAccept(ServerRequest req, ServerResponse res) {
SpanContext inboundSpanContext = tracer.extract(Format.Builtin.HTTP_HEADERS, new TextMapAdapter(headersMap));

if (inboundSpanContext instanceof NoopSpanBuilder) {
// this is all a noop stuff, does not matter what I do here - this is to prevent null pointers
// when span is used
// TODO once we return Optional from ServerRequest.spanContext(), we can also remove the
// following codeblock and we do not need to create teh span and register it with context
Span span = tracer.buildSpan("helidon-webserver")
.asChildOf(inboundSpanContext)
.start();

context.register(span);
context.register(span.context());
context.register(ServerRequest.class, span);
context.register(ServerRequest.class, span.context());

res.whenSent()
.thenRun(span::finish);

// no tracing
return;
}
Expand Down Expand Up @@ -304,9 +301,6 @@ private void doAccept(ServerRequest req, ServerResponse res) {
// cannot use startActive, as it conflicts with the thread model we use
Span span = spanBuilder.start();

// TODO remove the next single line for version 2.0 - we should only expose SpanContext
context.register(span);
context.register(ServerRequest.class, span);
context.register(span.context());
context.register(ServerRequest.class, span.context());

Expand Down