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

Set server span name for aborted requests in quarkus resteasy reactive #8891

Merged
merged 3 commits into from
Jul 11, 2023
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
Expand Up @@ -17,6 +17,8 @@
import io.opentelemetry.testing.internal.armeria.common.AggregatedHttpResponse;
import io.opentelemetry.testing.internal.armeria.common.HttpMethod;
import java.time.Duration;
import java.util.Collections;
import java.util.Map;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
Expand All @@ -42,8 +44,15 @@ static void setUp() {
}

private static AggregatedHttpResponse request(String path) {
return request(path, Collections.emptyMap());
}

private static AggregatedHttpResponse request(String path, Map<String, String> headers) {
AggregatedHttpRequest request =
AggregatedHttpRequest.of(HttpMethod.GET, "h1c://localhost:" + port + path);
if (!headers.isEmpty()) {
request = AggregatedHttpRequest.of(request.headers().toBuilder().add(headers).build());
}
return client.execute(request).aggregate().join();
}

Expand Down Expand Up @@ -96,4 +105,16 @@ void testSubResourceLocator() {
span.hasName("GET /test-sub-resource-locator/call/sub")
.hasKind(SpanKind.SERVER)));
}

@Test
void testAbort() {
AggregatedHttpResponse response = request("/hello", Collections.singletonMap("abort", "true"));
assertThat(response.status().code()).isEqualTo(401);
assertThat(response.contentUtf8()).isEqualTo("Aborted");

testing.waitAndAssertTraces(
trace ->
trace.hasSpansSatisfyingExactly(
span -> span.hasName("GET /hello").hasKind(SpanKind.SERVER)));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.instrumentation.quarkus.resteasy.reactive;

import static net.bytebuddy.matcher.ElementMatchers.named;

import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.matcher.ElementMatcher;
import org.jboss.resteasy.reactive.common.core.AbstractResteasyReactiveContext;
import org.jboss.resteasy.reactive.server.core.ResteasyReactiveRequestContext;

public class AbstractResteasyReactiveContextInstrumentation implements TypeInstrumentation {
@Override
public ElementMatcher<TypeDescription> typeMatcher() {
return named("org.jboss.resteasy.reactive.common.core.AbstractResteasyReactiveContext");
}

@Override
public void transform(TypeTransformer transformer) {
transformer.applyAdviceToMethod(
named("run"),
AbstractResteasyReactiveContextInstrumentation.class.getName() + "$RunAdvice");
}

@SuppressWarnings("unused")
public static class RunAdvice {

@Advice.OnMethodEnter(suppress = Throwable.class)
public static OtelRequestContext onEnter(
@Advice.This AbstractResteasyReactiveContext<?, ?> requestContext) {
if (requestContext instanceof ResteasyReactiveRequestContext) {
ResteasyReactiveRequestContext context = (ResteasyReactiveRequestContext) requestContext;
return OtelRequestContext.start(context);
}
return null;
}

@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
public static void onExit(@Advice.Enter OtelRequestContext context) {
if (context != null) {
context.close();
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

import static net.bytebuddy.matcher.ElementMatchers.named;

import io.opentelemetry.context.Scope;
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
import net.bytebuddy.asm.Advice;
Expand All @@ -31,10 +30,8 @@ public void transform(TypeTransformer transformer) {
public static class HandleAdvice {

@Advice.OnMethodEnter(suppress = Throwable.class)
public static void onEnter(
@Advice.Argument(0) ResteasyReactiveRequestContext requestContext,
@Advice.Local("otelScope") Scope scope) {
ResteasyReactiveSpanName.INSTANCE.updateServerSpanName(requestContext);
public static void onEnter(@Advice.Argument(0) ResteasyReactiveRequestContext requestContext) {
OtelRequestContext.onInvoke(requestContext);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.instrumentation.quarkus.resteasy.reactive;

import io.opentelemetry.instrumentation.api.instrumenter.http.HttpRouteSource;
import org.jboss.resteasy.reactive.server.core.ResteasyReactiveRequestContext;

public final class OtelRequestContext {
private static final ThreadLocal<OtelRequestContext> contextThreadLocal = new ThreadLocal<>();
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to use OTel Context instead of a thread local?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the ThreadLocal uses slightly less code. I don't have a strong preference, if you want I can rewrite to otel context.

private boolean firstInvoke = true;

public static OtelRequestContext start(ResteasyReactiveRequestContext requestContext) {
OtelRequestContext context = new OtelRequestContext();
contextThreadLocal.set(context);
ResteasyReactiveSpanName.INSTANCE.updateServerSpanName(
requestContext, HttpRouteSource.CONTROLLER);
return context;
}

public static void onInvoke(ResteasyReactiveRequestContext requestContext) {
OtelRequestContext context = contextThreadLocal.get();
if (context == null) {
return;
}
// we ignore the first invoke as it uses the same context that we get in start, the second etc.
// invoke will be for sub resource locator that changes the path
if (context.firstInvoke) {
context.firstInvoke = false;
return;
}
ResteasyReactiveSpanName.INSTANCE.updateServerSpanName(
requestContext, HttpRouteSource.NESTED_CONTROLLER);
}

public void close() {
contextThreadLocal.remove();
}

private OtelRequestContext() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

package io.opentelemetry.javaagent.instrumentation.quarkus.resteasy.reactive;

import static java.util.Collections.singletonList;
import static java.util.Arrays.asList;

import com.google.auto.service.AutoService;
import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
Expand All @@ -21,6 +21,8 @@ public QuarkusResteasyReactiveInstrumentationModule() {

@Override
public List<TypeInstrumentation> typeInstrumentations() {
return singletonList(new InvocationHandlerInstrumentation());
return asList(
new AbstractResteasyReactiveContextInstrumentation(),
new InvocationHandlerInstrumentation());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,25 @@
import org.jboss.resteasy.reactive.server.mapping.RuntimeResource;
import org.jboss.resteasy.reactive.server.mapping.URITemplate;

public final class ResteasyReactiveSpanName {
final class ResteasyReactiveSpanName {
// remember previous path to handle sub path locators
private static final VirtualField<ResteasyReactiveRequestContext, String> pathField =
VirtualField.find(ResteasyReactiveRequestContext.class, String.class);

public static final ResteasyReactiveSpanName INSTANCE = new ResteasyReactiveSpanName();

public void updateServerSpanName(ResteasyReactiveRequestContext requestContext) {
void updateServerSpanName(ResteasyReactiveRequestContext requestContext, HttpRouteSource source) {
Context context = Context.current();
String jaxRsName = calculateJaxRsName(requestContext);
HttpRouteHolder.updateHttpRoute(context, HttpRouteSource.NESTED_CONTROLLER, jaxRsName);
HttpRouteHolder.updateHttpRoute(context, source, jaxRsName);
pathField.set(requestContext, jaxRsName);
}

private static String calculateJaxRsName(ResteasyReactiveRequestContext requestContext) {
RuntimeResource target = requestContext.getTarget();
if (target == null) {
return null;
}
URITemplate classPath = target.getClassPath();
URITemplate path = target.getPath();
String name = normalize(classPath) + normalize(path);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.quarkus.resteasy.reactive.v2_0;

import javax.ws.rs.container.ContainerRequestContext;
import javax.ws.rs.container.ContainerRequestFilter;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import javax.ws.rs.ext.Provider;

@Provider
public class TestFilter implements ContainerRequestFilter {

@Override
public void filter(ContainerRequestContext containerRequestContext) {
if (containerRequestContext.getHeaderString("abort") != null) {
containerRequestContext.abortWith(
Response.status(Response.Status.UNAUTHORIZED)
.entity("Aborted")
.type(MediaType.TEXT_PLAIN_TYPE)
.build());
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.instrumentation.quarkus.resteasy.reactive.v3_0;

import jakarta.ws.rs.container.ContainerRequestContext;
import jakarta.ws.rs.container.ContainerRequestFilter;
import jakarta.ws.rs.core.MediaType;
import jakarta.ws.rs.core.Response;
import jakarta.ws.rs.ext.Provider;

@Provider
public class TestFilter implements ContainerRequestFilter {

@Override
public void filter(ContainerRequestContext containerRequestContext) {
if (containerRequestContext.getHeaderString("abort") != null) {
containerRequestContext.abortWith(
Response.status(Response.Status.UNAUTHORIZED)
.entity("Aborted")
.type(MediaType.TEXT_PLAIN_TYPE)
.build());
}
}
}