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 all 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
@@ -0,0 +1,34 @@
/*
* Copyright (c) 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.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.helidon.tests.functional.context.hello;

import javax.ws.rs.container.ContainerRequestContext;
import javax.ws.rs.container.ContainerRequestFilter;
import javax.ws.rs.ext.Provider;

import io.helidon.common.context.Contexts;

/**
* A filter that adds request scoped context record.
*/
@Provider
public class ContextFilter implements ContainerRequestFilter {
@Override
public void filter(ContainerRequestContext requestContext) {
Contexts.context().ifPresent(ctx -> ctx.register(new MyMessage()));
}
}
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 @@ -24,8 +24,8 @@

import io.helidon.common.context.Context;
import io.helidon.common.context.Contexts;
import io.helidon.metrics.RegistryFactory;

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

Expand All @@ -42,9 +42,12 @@ public class HelloBean {
* @return Hello string.
*/
public String getHello() {
Context context = Contexts.context().get();
Context context = Contexts.context().orElse(null);
Objects.requireNonNull(context);
Objects.requireNonNull(context.get(SpanContext.class).get());
// application scoped context
Objects.requireNonNull(context.get(RegistryFactory.class).orElse(null));
// request scoped context
Objects.requireNonNull(context.get(MyMessage.class).orElse(null));
return "Hello World";
}

Expand All @@ -55,9 +58,12 @@ public String getHello() {
*/
@Timeout(1000)
public String getHelloTimeout() {
Context context = Contexts.context().get();
Context context = Contexts.context().orElse(null);
Objects.requireNonNull(context);
Objects.requireNonNull(context.get(SpanContext.class).get());
// application scoped context
Objects.requireNonNull(context.get(RegistryFactory.class).orElse(null));
// request scoped context
Objects.requireNonNull(context.get(MyMessage.class).orElse(null));
return "Hello World";
}

Expand All @@ -68,9 +74,12 @@ public String getHelloTimeout() {
*/
@Asynchronous
public CompletionStage<String> getHelloAsync() {
Context context = Contexts.context().get();
Context context = Contexts.context().orElse(null);
Objects.requireNonNull(context);
Objects.requireNonNull(context.get(SpanContext.class).get());
// application scoped context
Objects.requireNonNull(context.get(RegistryFactory.class).orElse(null));
// request scoped context
Objects.requireNonNull(context.get(MyMessage.class).orElse(null));
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) 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 @@ -16,18 +16,10 @@

package io.helidon.tests.functional.context.hello;

import java.util.Set;

import javax.enterprise.context.ApplicationScoped;
import javax.ws.rs.core.Application;

/**
* HelloApplication class.
* Class to test context.
*/
@ApplicationScoped
public class HelloApplication extends Application {
@Override
public Set<Class<?>> getClasses() {
return Set.of(HelloResource.class);
class MyMessage {
MyMessage() {
}
}
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 (shouldTrace && checkedIfShouldTrace.compareAndSet(false, true)) {
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