Remove RequestDispatcherAdvice
MODIFY
instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/JakartaServletInstrumentationModule.java
instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/JakartaServletInstrumentationModule.java
@@ -9,7 +9,6 @@ import com.google.auto.service.AutoService;
import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.instrumentation.servlet.common.async.AsyncContextInstrumentation;
-import io.opentelemetry.javaagent.instrumentation.servlet.common.dispatcher.RequestDispatcherInstrumentation;
import io.opentelemetry.javaagent.instrumentation.servlet.common.response.HttpServletResponseInstrumentation;
import io.opentelemetry.javaagent.instrumentation.servlet.common.service.ServletAndFilterInstrumentation;
import java.util.Arrays;
@@ -34,9 +33,7 @@ public class JakartaServletInstrumentationModule extends InstrumentationModule {
adviceClassName(".service.JakartaServletInitAdvice"),
adviceClassName(".service.JakartaServletFilterInitAdvice")),
new HttpServletResponseInstrumentation(
- BASE_PACKAGE, adviceClassName(".response.ResponseSendAdvice")),
- new RequestDispatcherInstrumentation(
- BASE_PACKAGE, adviceClassName(".dispatcher.RequestDispatcherAdvice")));
+ BASE_PACKAGE, adviceClassName(".response.ResponseSendAdvice")));
}
private static String adviceClassName(String suffix) {

DELETE
instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/dispatcher/RequestDispatcherAdvice.java
-/*
- * Copyright The OpenTelemetry Authors
- * SPDX-License-Identifier: Apache-2.0
- */
-
-package io.opentelemetry.javaagent.instrumentation.servlet.v5_0.dispatcher;
-
-import static io.opentelemetry.instrumentation.api.tracer.HttpServerTracer.CONTEXT_ATTRIBUTE;
-
-import io.opentelemetry.context.Context;
-import io.opentelemetry.context.Scope;
-import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge;
-import io.opentelemetry.javaagent.instrumentation.servlet.common.dispatcher.RequestDispatcherAdviceHelper;
-import jakarta.servlet.RequestDispatcher;
-import jakarta.servlet.ServletRequest;
-import java.lang.reflect.Method;
-import net.bytebuddy.asm.Advice;
-
-public class RequestDispatcherAdvice {
-
- @Advice.OnMethodEnter(suppress = Throwable.class)
- public static void start(
- @Advice.Origin Method method,
- @Advice.This RequestDispatcher dispatcher,
- @Advice.Local("otelRequestContext") Context requestContext,
- @Advice.Local("otelContext") Context context,
- @Advice.Local("otelScope") Scope scope,
- @Advice.Argument(0) ServletRequest request) {
-
- Context currentContext = Java8BytecodeBridge.currentContext();
-
- Object requestContextAttr = request.getAttribute(CONTEXT_ATTRIBUTE);
- requestContext = requestContextAttr instanceof Context ? (Context) requestContextAttr : null;
-
- context = RequestDispatcherAdviceHelper.getStartParentContext(currentContext, requestContext);
- if (context == null) {
- return;
- }
-
- // this tells the dispatched servlet to use the current span as the parent for its work
- request.setAttribute(CONTEXT_ATTRIBUTE, context);
-
- // TODO (trask) do we need this, since doing manual propagation above?
- scope = context.makeCurrent();
- }
-
- @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
- public static void stop(
- @Advice.Argument(0) ServletRequest request,
- @Advice.Local("otelRequestContext") Context requestContext,
- @Advice.Local("otelContext") Context context,
- @Advice.Local("otelScope") Scope scope,
- @Advice.Thrown Throwable throwable) {
-
- scope.close();
-
- if (requestContext != null) {
- // restore the original request context
- request.setAttribute(CONTEXT_ATTRIBUTE, requestContext);
- }
- }
-}

DELETE
instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/common/dispatcher/RequestDispatcherAdviceHelper.java
-/*
- * Copyright The OpenTelemetry Authors
- * SPDX-License-Identifier: Apache-2.0
- */
-
-package io.opentelemetry.javaagent.instrumentation.servlet.common.dispatcher;
-
-import io.opentelemetry.api.trace.Span;
-import io.opentelemetry.api.trace.SpanContext;
-import io.opentelemetry.context.Context;
-import io.opentelemetry.instrumentation.api.tracer.HttpServerTracer;
-import org.checkerframework.checker.nullness.qual.Nullable;
-
-public class RequestDispatcherAdviceHelper {
- /**
- * Determines if the advice for {@link RequestDispatcherInstrumentation} should create a new span
- * and provides the context in which that span should be created.
- *
- * @param requestContext Value of the {@link HttpServerTracer#CONTEXT_ATTRIBUTE} attribute of the
- * servlet request.
- * @return The context in which the advice should create the dispatcher span in. Returns <code>
- * null</code> in case a new span should not be created.
- */
- // TODO (trask) do we need to guard against context leak here?
- // this could be simplified by always using currentContext, only falling back to requestContext
- // if currentContext does not have a valid span
- public static @Nullable Context getStartParentContext(
- Context currentContext, @Nullable Context requestContext) {
- Span currentSpan = Span.fromContext(currentContext);
- SpanContext currentSpanContext = currentSpan.getSpanContext();
-
- if (!currentSpanContext.isValid()) {
- return requestContext;
- }
-
- if (requestContext == null) {
- return currentContext;
- }
-
- // at this point: currentContext has a valid span and requestContext is not null
-
- Span requestSpan = Span.fromContext(requestContext);
- if (requestSpan.getSpanContext().getTraceId().equals(currentSpanContext.getTraceId())) {
- // currentContext is part of the same trace, so return it
- return currentContext;
- }
-
- // currentContext is part of a different trace, so lets ignore it.
- // This can happen with the way Tomcat does error handling.
- return requestContext;
- }
-}

DELETE
instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/common/dispatcher/RequestDispatcherInstrumentation.java
-/*
- * Copyright The OpenTelemetry Authors
- * SPDX-License-Identifier: Apache-2.0
- */
-
-package io.opentelemetry.javaagent.instrumentation.servlet.common.dispatcher;
-
-import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface;
-import static io.opentelemetry.javaagent.extension.matcher.ClassLoaderMatcher.hasClassesNamed;
-import static io.opentelemetry.javaagent.extension.matcher.NameMatchers.namedOneOf;
-import static java.util.Collections.singletonMap;
-import static net.bytebuddy.matcher.ElementMatchers.isPublic;
-import static net.bytebuddy.matcher.ElementMatchers.named;
-import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
-import static net.bytebuddy.matcher.ElementMatchers.takesArguments;
-
-import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
-import java.util.Map;
-import net.bytebuddy.description.method.MethodDescription;
-import net.bytebuddy.description.type.TypeDescription;
-import net.bytebuddy.matcher.ElementMatcher;
-
-public class RequestDispatcherInstrumentation implements TypeInstrumentation {
- private final String basePackageName;
- private final String adviceClassName;
-
- public RequestDispatcherInstrumentation(String basePackageName, String adviceClassName) {
- this.basePackageName = basePackageName;
- this.adviceClassName = adviceClassName;
- }
-
- @Override
- public ElementMatcher<ClassLoader> classLoaderOptimization() {
- return hasClassesNamed(basePackageName + ".RequestDispatcher");
- }
-
- @Override
- public ElementMatcher<TypeDescription> typeMatcher() {
- return implementsInterface(named(basePackageName + ".RequestDispatcher"));
- }
-
- @Override
- public Map<? extends ElementMatcher<? super MethodDescription>, String> transformers() {
- return singletonMap(
- namedOneOf("forward", "include")
- .and(takesArguments(2))
- .and(takesArgument(0, named(basePackageName + ".ServletRequest")))
- .and(takesArgument(1, named(basePackageName + ".ServletResponse")))
- .and(isPublic()),
- adviceClassName);
- }
-}

MODIFY
instrumentation/servlet/servlet-javax-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/javax/JavaxServletInstrumentationModule.java
instrumentation/servlet/servlet-javax-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/javax/JavaxServletInstrumentationModule.java
@@ -8,9 +8,8 @@ package io.opentelemetry.javaagent.instrumentation.servlet.javax;
import com.google.auto.service.AutoService;
import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
-import io.opentelemetry.javaagent.instrumentation.servlet.common.dispatcher.RequestDispatcherInstrumentation;
import io.opentelemetry.javaagent.instrumentation.servlet.common.response.HttpServletResponseInstrumentation;
-import java.util.Arrays;
+import java.util.Collections;
import java.util.List;
@AutoService(InstrumentationModule.class)
@@ -23,11 +22,9 @@ public class JavaxServletInstrumentationModule extends InstrumentationModule {
@Override
public List<TypeInstrumentation> typeInstrumentations() {
- return Arrays.asList(
+ return Collections.singletonList(
new HttpServletResponseInstrumentation(
- BASE_PACKAGE, adviceClassName(".response.ResponseSendAdvice")),
- new RequestDispatcherInstrumentation(
- BASE_PACKAGE, adviceClassName(".dispatcher.RequestDispatcherAdvice")));
+ BASE_PACKAGE, adviceClassName(".response.ResponseSendAdvice")));
}
private static String adviceClassName(String suffix) {

DELETE
instrumentation/servlet/servlet-javax-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/javax/dispatcher/RequestDispatcherAdvice.java
-/*
- * Copyright The OpenTelemetry Authors
- * SPDX-License-Identifier: Apache-2.0
- */
-
-package io.opentelemetry.javaagent.instrumentation.servlet.javax.dispatcher;
-
-import static io.opentelemetry.instrumentation.api.tracer.HttpServerTracer.CONTEXT_ATTRIBUTE;
-
-import io.opentelemetry.context.Context;
-import io.opentelemetry.context.Scope;
-import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge;
-import io.opentelemetry.javaagent.instrumentation.servlet.common.dispatcher.RequestDispatcherAdviceHelper;
-import java.lang.reflect.Method;
-import javax.servlet.ServletRequest;
-import net.bytebuddy.asm.Advice;
-
-public class RequestDispatcherAdvice {
-
- @Advice.OnMethodEnter(suppress = Throwable.class)
- public static void start(
- @Advice.Origin Method method,
- @Advice.Local("otelRequestContext") Context requestContext,
- @Advice.Local("otelContext") Context context,
- @Advice.Local("otelScope") Scope scope,
- @Advice.Argument(0) ServletRequest request) {
-
- Context currentContext = Java8BytecodeBridge.currentContext();
-
- Object requestContextAttr = request.getAttribute(CONTEXT_ATTRIBUTE);
- requestContext = requestContextAttr instanceof Context ? (Context) requestContextAttr : null;
-
- context = RequestDispatcherAdviceHelper.getStartParentContext(currentContext, requestContext);
- if (context == null) {
- return;
- }
-
- // this tells the dispatched servlet to use the current span as the parent for its work
- request.setAttribute(CONTEXT_ATTRIBUTE, context);
-
- // TODO (trask) do we need this, since doing manual propagation above?
- scope = context.makeCurrent();
- }
-
- @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
- public static void stop(
- @Advice.Argument(0) ServletRequest request,
- @Advice.Local("otelRequestContext") Context requestContext,
- @Advice.Local("otelContext") Context context,
- @Advice.Local("otelScope") Scope scope,
- @Advice.Thrown Throwable throwable) {
-
- scope.close();
-
- if (requestContext != null) {
- // restore the original request context
- request.setAttribute(CONTEXT_ATTRIBUTE, requestContext);
- }
- }
-}

DELETE
instrumentation/servlet/servlet-javax-common/javaagent/src/test/groovy/RequestDispatcherTest.groovy
-/*
- * Copyright The OpenTelemetry Authors
- * SPDX-License-Identifier: Apache-2.0
- */
-
-import static io.opentelemetry.instrumentation.test.utils.TraceUtils.basicSpan
-import static io.opentelemetry.instrumentation.test.utils.TraceUtils.runUnderTrace
-
-import io.opentelemetry.context.Context
-import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification
-import javax.servlet.ServletException
-import javax.servlet.http.HttpServletRequest
-import javax.servlet.http.HttpServletResponse
-
-class RequestDispatcherTest extends AgentInstrumentationSpecification {
-
- def request = Mock(HttpServletRequest)
- def response = Mock(HttpServletResponse)
- def dispatcher = new RequestDispatcherUtils(request, response)
-
- def "test dispatch no-parent"() {
- when:
- dispatcher.forward("")
- dispatcher.include("")
-
- then:
- assertTraces(2) {
- traces.sort(orderByRootSpanName("forward-child", "include-child"))
- trace(0, 1) {
- basicSpan(it, 0, "forward-child")
- }
- trace(1, 1) {
- basicSpan(it, 0, "include-child")
- }
- }
- }
-
- def "test dispatcher #method with parent"() {
- when:
- runUnderTrace("parent") {
- dispatcher."$method"(target)
- }
-
- then:
- assertTraces(1) {
- trace(0, 2) {
- basicSpan(it, 0, "parent")
- basicSpan(it, 1, "$operation-child", span(0))
- }
- }
-
- where:
- operation | method
- "forward" | "forward"
- "forward" | "forwardNamed"
- "include" | "include"
- "include" | "includeNamed"
-
- target = "test-$method"
- }
-
- def "test dispatcher #method with parent from request attribute"() {
- setup:
- Context context
- runUnderTrace("parent") {
- context = Context.current()
- }
-
- when:
- runUnderTrace("notParent") {
- context.makeCurrent().withCloseable {
- dispatcher."$method"(target)
- }
- }
-
- then:
- assertTraces(2) {
- traces.sort(orderByRootSpanName("parent", "notParent"))
- trace(0, 2) {
- basicSpan(it, 0, "parent")
- basicSpan(it, 1, "$operation-child", span(0))
- }
- trace(1, 1) {
- basicSpan(it, 0, "notParent")
- }
- }
-
- where:
- operation | method
- "forward" | "forward"
- "forward" | "forwardNamed"
- "include" | "include"
- "include" | "includeNamed"
-
- target = "test-$method"
- }
-
- def "test dispatcher #method exception"() {
- setup:
- def ex = new ServletException("some error")
- def dispatcher = new RequestDispatcherUtils(request, response, ex)
-
- when:
- runUnderTrace("parent") {
- dispatcher."$method"(target)
- }
-
- then:
- def th = thrown(ServletException)
- th == ex
-
- assertTraces(1) {
- trace(0, 2) {
- basicSpan(it, 0, "parent", null, ex)
- basicSpan(it, 1, "$operation-child", span(0))
- }
- }
-
- where:
- operation | method
- "forward" | "forward"
- "forward" | "forwardNamed"
- "include" | "include"
- "include" | "includeNamed"
-
- target = "test-$method"
- }
-}

DELETE
instrumentation/servlet/servlet-javax-common/javaagent/src/test/java/RequestDispatcherUtils.java
-/*
- * Copyright The OpenTelemetry Authors
- * SPDX-License-Identifier: Apache-2.0
- */
-
-import static io.opentelemetry.instrumentation.test.utils.TraceUtils.runUnderTrace;
-
-import java.io.IOException;
-import java.io.InputStream;
-import java.net.URL;
-import java.util.Enumeration;
-import java.util.Set;
-import java.util.concurrent.Callable;
-import javax.servlet.RequestDispatcher;
-import javax.servlet.Servlet;
-import javax.servlet.ServletContext;
-import javax.servlet.ServletException;
-import javax.servlet.ServletRequest;
-import javax.servlet.ServletResponse;
-
-public class RequestDispatcherUtils {
- private final ServletRequest req;
- private final ServletResponse resp;
- private final ServletException toThrow;
-
- public RequestDispatcherUtils(ServletRequest req, ServletResponse resp) {
- this.req = req;
- this.resp = resp;
- toThrow = null;
- }
-
- public RequestDispatcherUtils(
- ServletRequest req, ServletResponse resp, ServletException toThrow) {
- this.req = req;
- this.resp = resp;
- this.toThrow = toThrow;
- }
-
- /* RequestDispatcher can't be visible to groovy otherwise things break, so everything is
- * encapsulated in here where groovy doesn't need to access it.
- */
-
- void forward(String target) throws ServletException, IOException {
- new TestContext().getRequestDispatcher(target).forward(req, resp);
- }
-
- void include(String target) throws ServletException, IOException {
- new TestContext().getRequestDispatcher(target).include(req, resp);
- }
-
- void forwardNamed(String target) throws ServletException, IOException {
- new TestContext().getNamedDispatcher(target).forward(req, resp);
- }
-
- void includeNamed(String target) throws ServletException, IOException {
- new TestContext().getNamedDispatcher(target).include(req, resp);
- }
-
- class TestContext implements ServletContext {
- @Override
- public String getContextPath() {
- return null;
- }
-
- @Override
- public ServletContext getContext(String s) {
- return null;
- }
-
- @Override
- public int getMajorVersion() {
- return 0;
- }
-
- @Override
- public int getMinorVersion() {
- return 0;
- }
-
- @Override
- public String getMimeType(String s) {
- return null;
- }
-
- @Override
- public Set getResourcePaths(String s) {
- return null;
- }
-
- @Override
- public URL getResource(String s) {
- return null;
- }
-
- @Override
- public InputStream getResourceAsStream(String s) {
- return null;
- }
-
- @Override
- public RequestDispatcher getRequestDispatcher(String s) {
- return new TestDispatcher();
- }
-
- @Override
- public RequestDispatcher getNamedDispatcher(String s) {
- return new TestDispatcher();
- }
-
- @Override
- public Servlet getServlet(String s) {
- return null;
- }
-
- @Override
- public Enumeration getServlets() {
- return null;
- }
-
- @Override
- public Enumeration getServletNames() {
- return null;
- }
-
- @Override
- public void log(String s) {}
-
- @Override
- public void log(Exception e, String s) {}
-
- @Override
- public void log(String s, Throwable throwable) {}
-
- @Override
- public String getRealPath(String s) {
- return null;
- }
-
- @Override
- public String getServerInfo() {
- return null;
- }
-
- @Override
- public String getInitParameter(String s) {
- return null;
- }
-
- @Override
- public Enumeration getInitParameterNames() {
- return null;
- }
-
- @Override
- public Object getAttribute(String s) {
- return null;
- }
-
- @Override
- public Enumeration getAttributeNames() {
- return null;
- }
-
- @Override
- public void setAttribute(String s, Object o) {}
-
- @Override
- public void removeAttribute(String s) {}
-
- @Override
- public String getServletContextName() {
- return null;
- }
- }
-
- class TestDispatcher implements RequestDispatcher {
- @Override
- public void forward(ServletRequest servletRequest, ServletResponse servletResponse)
- throws ServletException {
- runUnderTrace(
- "forward-child",
- new Callable<Object>() {
- @Override
- public Object call() {
- return null;
- }
- });
- if (toThrow != null) {
- throw toThrow;
- }
- }
-
- @Override
- public void include(ServletRequest servletRequest, ServletResponse servletResponse)
- throws ServletException {
- runUnderTrace(
- "include-child",
- new Callable<Object>() {
- @Override
- public Object call() {
- return null;
- }
- });
- if (toThrow != null) {
- throw toThrow;
- }
- }
- }
-}

Remove RequestDispatcherAdvice (#2942)