Skip to content

Commit

Permalink
Convert servlet instrumentation to instrumenter api (#4078)
Browse files Browse the repository at this point in the history
* Convert servlet to instrumenter api

* make classes final

* Remove some tracer files

* remove xxx

* fix liberty and wildfly exception smoke test

* fix async smoke test on liberty

* Apply suggestions from code review

Co-authored-by: Mateusz Rzeszutek <[email protected]>

* generic TextMapGetter for servlets

* not going to use http.route for servlets

* simplify

* add servlet timeout in attribute extractor

* move classes from library to javaagent

* remove unneeded dependency

* make method private

* move helper class initialization to singleton, remove helpers that don't have any methods, add shouldStart checks

* Update instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/ServletRequestContext.java

Co-authored-by: Mateusz Rzeszutek <[email protected]>

* add import

* rename methods that start and end spans

Co-authored-by: Mateusz Rzeszutek <[email protected]>
  • Loading branch information
laurit and Mateusz Rzeszutek authored Sep 13, 2021
1 parent a576dd5 commit 15277cf
Show file tree
Hide file tree
Showing 75 changed files with 1,932 additions and 798 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import io.opentelemetry.context.Context;
import io.opentelemetry.context.ContextKey;
import org.checkerframework.checker.nullness.qual.Nullable;

/**
* Helper container for Context attributes for transferring certain information between servlet
Expand Down Expand Up @@ -42,6 +43,7 @@ public static Context init(Context ctx, boolean shouldRecordException) {
}

private final boolean servletShouldRecordException;
private Throwable exception;

private AppServerBridge(boolean shouldRecordException) {
servletShouldRecordException = shouldRecordException;
Expand All @@ -53,18 +55,47 @@ private AppServerBridge(boolean shouldRecordException) {
* during servlet invocation are propagated to the method where server span is closed and can be
* added to server span there and <code>true</code> otherwise.
*
* @param ctx server context
* @param context server context
* @return <code>true</code>, if servlet integration should record exception thrown during servlet
* invocation in server span, or <code>false</code> otherwise.
*/
public static boolean shouldRecordException(Context ctx) {
AppServerBridge appServerBridge = ctx.get(AppServerBridge.CONTEXT_KEY);
public static boolean shouldRecordException(Context context) {
AppServerBridge appServerBridge = context.get(AppServerBridge.CONTEXT_KEY);
if (appServerBridge != null) {
return appServerBridge.servletShouldRecordException;
}
return true;
}

/**
* Record exception that happened during servlet invocation so that app server instrumentation can
* add it to server span.
*
* @param context server context
* @param exception exception that happened during servlet invocation
*/
public static void recordException(Context context, Throwable exception) {
AppServerBridge appServerBridge = context.get(AppServerBridge.CONTEXT_KEY);
if (appServerBridge != null && appServerBridge.servletShouldRecordException) {
appServerBridge.exception = exception;
}
}

/**
* Get exception that happened during servlet invocation.
*
* @param context server context
* @return exception that happened during servlet invocation
*/
@Nullable
public static Throwable getException(Context context) {
AppServerBridge appServerBridge = context.get(AppServerBridge.CONTEXT_KEY);
if (appServerBridge != null) {
return appServerBridge.exception;
}
return null;
}

/**
* Class used as key in CallDepthThreadLocalMap for counting servlet invocation depth in
* Servlet3Advice and Servlet2Advice. We can not use helper classes like Servlet3Advice and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@

package io.opentelemetry.javaagent.instrumentation.jetty.v11_0;

import static io.opentelemetry.javaagent.instrumentation.jetty.v11_0.Jetty11HttpServerTracer.tracer;
import static io.opentelemetry.javaagent.instrumentation.jetty.v11_0.Jetty11Singletons.helper;

import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import io.opentelemetry.javaagent.instrumentation.jetty.common.JettyHandlerAdviceHelper;
import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge;
import io.opentelemetry.javaagent.instrumentation.servlet.ServletRequestContext;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import net.bytebuddy.asm.Advice;
Expand All @@ -22,30 +23,39 @@ public static void onEnter(
@Advice.This Object source,
@Advice.Argument(2) HttpServletRequest request,
@Advice.Argument(3) HttpServletResponse response,
@Advice.Local("otelRequest") ServletRequestContext<HttpServletRequest> requestContext,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) {

Context attachedContext = tracer().getServerContext(request);
Context attachedContext = helper().getServerContext(request);
if (attachedContext != null) {
// We are inside nested handler, don't create new span
return;
}

context = tracer().startServerSpan(request);
Context parentContext = Java8BytecodeBridge.currentContext();
requestContext = new ServletRequestContext<>(request);

if (!helper().shouldStart(parentContext, requestContext)) {
return;
}

context = helper().start(parentContext, requestContext);
scope = context.makeCurrent();

// Must be set here since Jetty handlers can use startAsync outside of servlet scope.
tracer().setAsyncListenerResponse(request, response);
helper().setAsyncListenerResponse(request, response);
}

@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
public static void stopSpan(
@Advice.Argument(2) HttpServletRequest request,
@Advice.Argument(3) HttpServletResponse response,
@Advice.Thrown Throwable throwable,
@Advice.Local("otelRequest") ServletRequestContext<HttpServletRequest> requestContext,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) {

JettyHandlerAdviceHelper.stopSpan(tracer(), request, response, throwable, context, scope);
helper().end(requestContext, request, response, throwable, context, scope);
}
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.instrumentation.jetty.v11_0;

import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
import io.opentelemetry.javaagent.instrumentation.jetty.common.JettyHelper;
import io.opentelemetry.javaagent.instrumentation.servlet.ServletInstrumenterBuilder;
import io.opentelemetry.javaagent.instrumentation.servlet.ServletRequestContext;
import io.opentelemetry.javaagent.instrumentation.servlet.ServletResponseContext;
import io.opentelemetry.javaagent.instrumentation.servlet.v5_0.Servlet5Accessor;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;

public final class Jetty11Singletons {
private static final String INSTRUMENTATION_NAME = "io.opentelemetry.jetty-11.0";

private static final Instrumenter<
ServletRequestContext<HttpServletRequest>, ServletResponseContext<HttpServletResponse>>
INSTRUMENTER =
ServletInstrumenterBuilder.newInstrumenter(
INSTRUMENTATION_NAME, Servlet5Accessor.INSTANCE);
private static final JettyHelper<HttpServletRequest, HttpServletResponse> HELPER =
new JettyHelper<>(INSTRUMENTER, Servlet5Accessor.INSTANCE);

public static JettyHelper<HttpServletRequest, HttpServletResponse> helper() {
return HELPER;
}

private Jetty11Singletons() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@

package io.opentelemetry.javaagent.instrumentation.jetty.v8_0;

import static io.opentelemetry.javaagent.instrumentation.jetty.v8_0.Jetty8HttpServerTracer.tracer;
import static io.opentelemetry.javaagent.instrumentation.jetty.v8_0.Jetty8Singletons.helper;

import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import io.opentelemetry.javaagent.instrumentation.jetty.common.JettyHandlerAdviceHelper;
import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge;
import io.opentelemetry.javaagent.instrumentation.servlet.ServletRequestContext;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import net.bytebuddy.asm.Advice;
Expand All @@ -22,30 +23,39 @@ public static void onEnter(
@Advice.This Object source,
@Advice.Argument(2) HttpServletRequest request,
@Advice.Argument(3) HttpServletResponse response,
@Advice.Local("otelRequest") ServletRequestContext<HttpServletRequest> requestContext,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) {

Context attachedContext = tracer().getServerContext(request);
Context attachedContext = helper().getServerContext(request);
if (attachedContext != null) {
// We are inside nested handler, don't create new span
return;
}

context = tracer().startServerSpan(request);
Context parentContext = Java8BytecodeBridge.currentContext();
requestContext = new ServletRequestContext<>(request);

if (!helper().shouldStart(parentContext, requestContext)) {
return;
}

context = helper().start(parentContext, requestContext);
scope = context.makeCurrent();

// Must be set here since Jetty handlers can use startAsync outside of servlet scope.
tracer().setAsyncListenerResponse(request, response);
helper().setAsyncListenerResponse(request, response);
}

@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
public static void stopSpan(
@Advice.Argument(2) HttpServletRequest request,
@Advice.Argument(3) HttpServletResponse response,
@Advice.Thrown Throwable throwable,
@Advice.Local("otelRequest") ServletRequestContext<HttpServletRequest> requestContext,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) {

JettyHandlerAdviceHelper.stopSpan(tracer(), request, response, throwable, context, scope);
helper().end(requestContext, request, response, throwable, context, scope);
}
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.instrumentation.jetty.v8_0;

import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
import io.opentelemetry.instrumentation.servlet.v3_0.Servlet3Accessor;
import io.opentelemetry.javaagent.instrumentation.jetty.common.JettyHelper;
import io.opentelemetry.javaagent.instrumentation.servlet.ServletInstrumenterBuilder;
import io.opentelemetry.javaagent.instrumentation.servlet.ServletRequestContext;
import io.opentelemetry.javaagent.instrumentation.servlet.ServletResponseContext;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

public final class Jetty8Singletons {
private static final String INSTRUMENTATION_NAME = "io.opentelemetry.jetty-8.0";

private static final Instrumenter<
ServletRequestContext<HttpServletRequest>, ServletResponseContext<HttpServletResponse>>
INSTRUMENTER =
ServletInstrumenterBuilder.newInstrumenter(
INSTRUMENTATION_NAME, Servlet3Accessor.INSTANCE);
private static final JettyHelper<HttpServletRequest, HttpServletResponse> HELPER =
new JettyHelper<>(INSTRUMENTER, Servlet3Accessor.INSTANCE);

public static JettyHelper<HttpServletRequest, HttpServletResponse> helper() {
return HELPER;
}

private Jetty8Singletons() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@
*
* <p>As instrumentation points differ between servlet instrumentations and this one, this module
* has its own {@code JettyHandlerInstrumentation} and {@code JettyHandlerAdvice}. But this is the
* only difference between two instrumentations, thus {@link
* io.opentelemetry.javaagent.instrumentation.jetty.v8_0.Jetty8HttpServerTracer} is a very thin
* subclass of {@link io.opentelemetry.instrumentation.servlet.v3_0.Servlet3HttpServerTracer}.
* only difference between two instrumentations, thus jetty instrumentation largely reuses servlet
* instrumentation.
*/
package io.opentelemetry.javaagent.instrumentation.jetty.v8_0;

This file was deleted.

Loading

0 comments on commit 15277cf

Please sign in to comment.