From 6c8cd2e7f25b16adfa43e5ccded4f3e2c9f74835 Mon Sep 17 00:00:00 2001 From: Nikita Salnikov-Tarnovski Date: Mon, 25 May 2020 22:53:12 +0300 Subject: [PATCH] Change names of servlet based server spans (#428) * Add documentation describing non-obvious points of Servlet instrumentations * Change names of servlet based server spans --- .../JSPInstrumentationBasicTests.groovy | 25 ++++--- .../JSPInstrumentationForwardTests.groovy | 20 ++--- instrumentation/servlet/README.md | 74 +++++++++++++++++++ .../test/groovy/GlassFishServerTest.groovy | 13 ++-- .../servlet/v2_3/Servlet2Advice.java | 18 +++-- .../v2_3/Servlet2ResponseRedirectAdvice.java | 28 ------- .../v2_3/Servlet2ResponseStatusAdvice.java | 29 -------- ...Servlet2ResponseStatusInstrumentation.java | 40 ++++++++-- .../src/test/groovy/JettyServlet2Test.groovy | 2 +- .../servlet/v3_0/Servlet3Advice.java | 4 +- .../test/groovy/AbstractServlet3Test.groovy | 9 ++- .../src/test/groovy/TomcatServlet3Test.groovy | 5 ++ .../servlet/filter/FilterInstrumentation.java | 5 ++ .../http/HttpServletInstrumentation.java | 7 +- 14 files changed, 180 insertions(+), 99 deletions(-) create mode 100644 instrumentation/servlet/README.md delete mode 100644 instrumentation/servlet/request-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3/Servlet2ResponseRedirectAdvice.java delete mode 100644 instrumentation/servlet/request-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3/Servlet2ResponseStatusAdvice.java diff --git a/instrumentation/jsp-2.3/src/test/groovy/JSPInstrumentationBasicTests.groovy b/instrumentation/jsp-2.3/src/test/groovy/JSPInstrumentationBasicTests.groovy index 722aeddbdd73..42afee3f57d1 100644 --- a/instrumentation/jsp-2.3/src/test/groovy/JSPInstrumentationBasicTests.groovy +++ b/instrumentation/jsp-2.3/src/test/groovy/JSPInstrumentationBasicTests.groovy @@ -14,7 +14,6 @@ * limitations under the License. */ import com.google.common.io.Files -import io.opentelemetry.auto.bootstrap.instrumentation.decorator.HttpServerDecorator import io.opentelemetry.auto.instrumentation.api.MoreTags import io.opentelemetry.auto.instrumentation.api.Tags import io.opentelemetry.auto.test.AgentTestRunner @@ -34,6 +33,7 @@ import spock.lang.Unroll import static io.opentelemetry.trace.Span.Kind.SERVER +//TODO should this be HttpServerTest? class JSPInstrumentationBasicTests extends AgentTestRunner { static { @@ -106,7 +106,7 @@ class JSPInstrumentationBasicTests extends AgentTestRunner { trace(0, 3) { span(0) { parent() - operationName expectedOperationName("GET") + operationName expectedOperationName() spanKind SERVER errored false tags { @@ -168,7 +168,7 @@ class JSPInstrumentationBasicTests extends AgentTestRunner { trace(0, 3) { span(0) { parent() - operationName expectedOperationName("GET") + operationName expectedOperationName() spanKind SERVER errored false tags { @@ -227,7 +227,7 @@ class JSPInstrumentationBasicTests extends AgentTestRunner { trace(0, 3) { span(0) { parent() - operationName expectedOperationName("POST") + operationName expectedOperationName() spanKind SERVER errored false tags { @@ -283,7 +283,7 @@ class JSPInstrumentationBasicTests extends AgentTestRunner { trace(0, 3) { span(0) { parent() - operationName expectedOperationName("GET") + operationName expectedOperationName() spanKind SERVER errored true tags { @@ -358,7 +358,7 @@ class JSPInstrumentationBasicTests extends AgentTestRunner { trace(0, 3) { span(0) { parent() - operationName expectedOperationName("GET") + operationName expectedOperationName() spanKind SERVER errored false tags { @@ -413,7 +413,7 @@ class JSPInstrumentationBasicTests extends AgentTestRunner { trace(0, 7) { span(0) { parent() - operationName expectedOperationName("GET") + operationName expectedOperationName() spanKind SERVER errored false tags { @@ -508,7 +508,7 @@ class JSPInstrumentationBasicTests extends AgentTestRunner { trace(0, 2) { span(0) { parent() - operationName expectedOperationName("GET") + operationName expectedOperationName() spanKind SERVER errored true tags { @@ -562,7 +562,7 @@ class JSPInstrumentationBasicTests extends AgentTestRunner { span(0) { parent() // serviceName jspWebappContext - operationName expectedOperationName("GET") + operationName expectedOperationName() spanKind SERVER // FIXME: this is not a great span name for serving static content. // spanName "GET /$jspWebappContext/$staticFile" @@ -588,7 +588,10 @@ class JSPInstrumentationBasicTests extends AgentTestRunner { staticFile = "common/hello.html" } - String expectedOperationName(String method) { - return method != null ? "HTTP $method" : HttpServerDecorator.DEFAULT_SPAN_NAME + //Simple class name plus method name of the entry point of the given servlet container. + //"Entry point" here means the first filter or servlet that accepts incoming requests. + //This will serve as a default name of the SERVER span created for this request. + protected String expectedOperationName() { + 'ApplicationFilterChain.doFilter' } } diff --git a/instrumentation/jsp-2.3/src/test/groovy/JSPInstrumentationForwardTests.groovy b/instrumentation/jsp-2.3/src/test/groovy/JSPInstrumentationForwardTests.groovy index 2b8590d4a4cd..5ad61d543725 100644 --- a/instrumentation/jsp-2.3/src/test/groovy/JSPInstrumentationForwardTests.groovy +++ b/instrumentation/jsp-2.3/src/test/groovy/JSPInstrumentationForwardTests.groovy @@ -14,7 +14,6 @@ * limitations under the License. */ import com.google.common.io.Files -import io.opentelemetry.auto.bootstrap.instrumentation.decorator.HttpServerDecorator import io.opentelemetry.auto.instrumentation.api.MoreTags import io.opentelemetry.auto.instrumentation.api.Tags import io.opentelemetry.auto.test.AgentTestRunner @@ -105,7 +104,7 @@ class JSPInstrumentationForwardTests extends AgentTestRunner { trace(0, 5) { span(0) { parent() - operationName expectedOperationName("GET") + operationName expectedOperationName() spanKind SERVER errored false tags { @@ -186,7 +185,7 @@ class JSPInstrumentationForwardTests extends AgentTestRunner { trace(0, 3) { span(0) { parent() - operationName expectedOperationName("GET") + operationName expectedOperationName() spanKind SERVER errored false tags { @@ -241,7 +240,7 @@ class JSPInstrumentationForwardTests extends AgentTestRunner { trace(0, 9) { span(0) { parent() - operationName expectedOperationName("GET") + operationName expectedOperationName() spanKind SERVER errored false tags { @@ -359,7 +358,7 @@ class JSPInstrumentationForwardTests extends AgentTestRunner { trace(0, 7) { span(0) { parent() - operationName expectedOperationName("GET") + operationName expectedOperationName() spanKind SERVER errored false tags { @@ -456,7 +455,7 @@ class JSPInstrumentationForwardTests extends AgentTestRunner { trace(0, 4) { span(0) { parent() - operationName expectedOperationName("GET") + operationName expectedOperationName() spanKind SERVER errored true tags { @@ -524,7 +523,7 @@ class JSPInstrumentationForwardTests extends AgentTestRunner { trace(0, 3) { span(0) { parent() - operationName expectedOperationName("GET") + operationName expectedOperationName() spanKind SERVER errored false tags { @@ -566,7 +565,10 @@ class JSPInstrumentationForwardTests extends AgentTestRunner { res.close() } - String expectedOperationName(String method) { - return method != null ? "HTTP $method" : HttpServerDecorator.DEFAULT_SPAN_NAME + //Simple class name plus method name of the entry point of the given servlet container. + //"Entry point" here means the first filter or servlet that accepts incoming requests. + //This will serve as a default name of the SERVER span created for this request. + protected String expectedOperationName() { + 'ApplicationFilterChain.doFilter' } } diff --git a/instrumentation/servlet/README.md b/instrumentation/servlet/README.md new file mode 100644 index 000000000000..9c8f5170442d --- /dev/null +++ b/instrumentation/servlet/README.md @@ -0,0 +1,74 @@ +# Instrumentation for Java Servlets + +In order to fully understand how java servlet instrumentation work, +let us first take a look at the following stacktrace from Spring PetClinic application. +Unimportant frames are redacted, points of interests are highlighted and discussed below. + +
+at org.springframework.samples.petclinic.owner.OwnerController.initCreationForm(OwnerController.java:60)
+...
+at org.springframework.web.servlet.mvc.method.AbstractHandlerMethodAdapter.handle(AbstractHandlerMethodAdapter.java:87)
+at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:1040)
+at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:943)
+at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1006)
+at org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:898)
+at javax.servlet.http.HttpServlet.service(HttpServlet.java:634)
+at org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:883)
+at javax.servlet.http.HttpServlet.service(HttpServlet.java:741)
+...
+at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
+...
+at org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)
+at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
+at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
+at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
+at java.base/java.lang.Thread.run(Thread.java:834)
+
+ +Everything starts when HTTP request processing reaches the first class from Servlet specification. +In the example above this is `ApplicationFilterChain.doFilter(ServletRequest, ServletResponse)` method. +Let us call this first servlet specific method an "entry point". +This is the main target for `Servlet3Instrumentation` and `Servlet2Instrumentation` instrumenters: + +`public void javax.servlet.FilterChain#doFilter(ServletRequest, ServletResponse)` + +`public void javax.servlet.http.HttpServlet#service(ServletRequest, ServletResponse)`. + +For example, Jetty Servlet container does not have default filter chain and in many cases will have +the second method as instrumentation entry point. +These instrumentations are located in two separate submodules `request-3.0` and `request-2.3`, respectively, +because they and corresponding tests depend on different versions of servlet specification. + +Next, request passes several other methods from Servlet specification, such as + +`protected void javax.servlet.http.HttpServlet#service(HttpServletRequest, HttpServletResponse)` or + +`protected void org.springframework.web.servlet.FrameworkServlet#doGet(HttpServletRequest, HttpServletResponse)`. + +They are the targets for `HttpServletInstrumentation`. +From the observability point of view nothing of interest usually happens inside these methods. +Thus it usually does not make sense to create spans from them, as they would only add useless noise. +For this reason `HttpServletInstrumentation` is disabled by default. +In rare cases when you need it, you can enable it using configuration property `ota.integration.servlet-service.enabled`. + +In exactly the same situation are all other Servlet filters beyond the initial entry point. +Usually unimportant, they may be sometimes of interest during troubleshooting. +They are instrumented by `FilterInstrumentation` which is too disabled by default. +You can enable it with the configuration property `ota.integration.servlet-filter.enabled`. +At last, request processing may reach the specific framework that you application uses. +In this case Spring MVC and `OwnerController.initCreationForm`. + +If all instrumentations are enabled, then a new span will be created for every highlighted frame. +All spans from Servlet API will have `kind=SERVER` and name based on corresponding class ana method names, +such as `ApplicationFilterChain.doFilter` or `FrameworkServlet.doGet`. +Span created by Spring MVC instrumentation will have `kind=INTERNAL` and named `OwnerController.initCreationForm`. + +The state described above has one significant problem. +Observability backends usually aggregate traces based on their root spans. +This means that ALL traces from any application deployed to Servlet container will be grouped together. +Becaue their root spans will all have the same named based on common entry point. +In order to alleviate this problem, instrumentations for specific frameworks, such as Spring MVC here, +_update_ name of the span corresponding to the entry point. +Each framework instrumentation can decide what is the best span name based on framework implementation details. +Of course, still adhering to OpenTelemetry +[semantic conventions](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/http.md). \ No newline at end of file diff --git a/instrumentation/servlet/glassfish-testing/src/test/groovy/GlassFishServerTest.groovy b/instrumentation/servlet/glassfish-testing/src/test/groovy/GlassFishServerTest.groovy index 7275beeaea1c..96805043f83e 100644 --- a/instrumentation/servlet/glassfish-testing/src/test/groovy/GlassFishServerTest.groovy +++ b/instrumentation/servlet/glassfish-testing/src/test/groovy/GlassFishServerTest.groovy @@ -36,10 +36,6 @@ import static io.opentelemetry.trace.Span.Kind.SERVER // TODO: Figure out a better way to test with OSGi included. class GlassFishServerTest extends HttpServerTest { -// static { -// System.setProperty("ota.integration.grizzly.enabled", "true") -// } - @Override URI buildAddress() { return new URI("http://localhost:$port/$context/") @@ -88,7 +84,7 @@ class GlassFishServerTest extends HttpServerTest { @Override void serverSpan(TraceAssert trace, int index, String traceID = null, String parentID = null, String method = "GET", ServerEndpoint endpoint = SUCCESS) { trace.span(index) { - operationName expectedOperationName(method) + operationName entryPointName() spanKind SERVER errored endpoint.errored if (parentID != null) { @@ -117,5 +113,12 @@ class GlassFishServerTest extends HttpServerTest { } } } + + //Simple class name plus method name of the entry point of the given servlet container. + //"Entry point" here means the first filter or servlet that accepts incoming requests. + //This will serve as a default name of the SERVER span created for this request. + protected String entryPointName() { + 'HttpServlet.service' + } } diff --git a/instrumentation/servlet/request-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3/Servlet2Advice.java b/instrumentation/servlet/request-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3/Servlet2Advice.java index 7f0fba512b54..d06683c8c60a 100644 --- a/instrumentation/servlet/request-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3/Servlet2Advice.java +++ b/instrumentation/servlet/request-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3/Servlet2Advice.java @@ -29,31 +29,35 @@ import io.opentelemetry.auto.instrumentation.api.Tags; import io.opentelemetry.trace.Span; import io.opentelemetry.trace.Status; +import java.lang.reflect.Method; import java.security.Principal; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import net.bytebuddy.asm.Advice; -import net.bytebuddy.implementation.bytecode.assign.Assigner; public class Servlet2Advice { @Advice.OnMethodEnter(suppress = Throwable.class) public static SpanWithScope onEnter( @Advice.This final Object servlet, + @Advice.Origin final Method method, @Advice.Argument(0) final ServletRequest request, - @Advice.Argument(value = 1, typing = Assigner.Typing.DYNAMIC) - final ServletResponse response) { + @Advice.Argument(1) final ServletResponse response) { + if (!(request instanceof HttpServletRequest)) { + return null; + } + final boolean hasServletTrace = request.getAttribute(SPAN_ATTRIBUTE) instanceof Span; - final boolean invalidRequest = !(request instanceof HttpServletRequest); - if (invalidRequest || hasServletTrace) { + if (hasServletTrace) { // Tracing might already be applied by the FilterChain or a parent request (forward/include). return null; } final HttpServletRequest httpServletRequest = (HttpServletRequest) request; + // TODO this logic should be moved to Servlet2 specific Decorator if (response instanceof HttpServletResponse) { // For use by HttpServletResponseInstrumentation: InstrumentationContext.get(HttpServletResponse.class, HttpServletRequest.class) @@ -64,9 +68,8 @@ public static SpanWithScope onEnter( } final Span.Builder builder = - TRACER.spanBuilder(DECORATE.spanNameForRequest(httpServletRequest)).setSpanKind(SERVER); + TRACER.spanBuilder(DECORATE.spanNameForMethod(method)).setSpanKind(SERVER); builder.setParent(extract(httpServletRequest, GETTER)); - final Span span = builder.setAttribute("span.origin.type", servlet.getClass().getName()).startSpan(); @@ -99,6 +102,7 @@ public static void stopSpan( if (spanWithScope == null) { return; } + final Span span = spanWithScope.getSpan(); if (response instanceof HttpServletResponse) { diff --git a/instrumentation/servlet/request-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3/Servlet2ResponseRedirectAdvice.java b/instrumentation/servlet/request-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3/Servlet2ResponseRedirectAdvice.java deleted file mode 100644 index 374818ca7460..000000000000 --- a/instrumentation/servlet/request-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3/Servlet2ResponseRedirectAdvice.java +++ /dev/null @@ -1,28 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * - * 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.opentelemetry.auto.instrumentation.servlet.v2_3; - -import io.opentelemetry.auto.bootstrap.InstrumentationContext; -import javax.servlet.ServletResponse; -import javax.servlet.http.HttpServletResponse; -import net.bytebuddy.asm.Advice; - -public class Servlet2ResponseRedirectAdvice { - @Advice.OnMethodEnter(suppress = Throwable.class) - public static void onEnter(@Advice.This final HttpServletResponse response) { - InstrumentationContext.get(ServletResponse.class, Integer.class).put(response, 302); - } -} diff --git a/instrumentation/servlet/request-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3/Servlet2ResponseStatusAdvice.java b/instrumentation/servlet/request-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3/Servlet2ResponseStatusAdvice.java deleted file mode 100644 index 7f32a45da7cf..000000000000 --- a/instrumentation/servlet/request-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3/Servlet2ResponseStatusAdvice.java +++ /dev/null @@ -1,29 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * - * 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.opentelemetry.auto.instrumentation.servlet.v2_3; - -import io.opentelemetry.auto.bootstrap.InstrumentationContext; -import javax.servlet.ServletResponse; -import javax.servlet.http.HttpServletResponse; -import net.bytebuddy.asm.Advice; - -public class Servlet2ResponseStatusAdvice { - @Advice.OnMethodEnter(suppress = Throwable.class) - public static void onEnter( - @Advice.This final HttpServletResponse response, @Advice.Argument(0) final Integer status) { - InstrumentationContext.get(ServletResponse.class, Integer.class).put(response, status); - } -} diff --git a/instrumentation/servlet/request-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3/Servlet2ResponseStatusInstrumentation.java b/instrumentation/servlet/request-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3/Servlet2ResponseStatusInstrumentation.java index b326e882af65..714e565af91f 100644 --- a/instrumentation/servlet/request-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3/Servlet2ResponseStatusInstrumentation.java +++ b/instrumentation/servlet/request-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3/Servlet2ResponseStatusInstrumentation.java @@ -22,13 +22,30 @@ import static net.bytebuddy.matcher.ElementMatchers.not; import com.google.auto.service.AutoService; +import io.opentelemetry.auto.bootstrap.InstrumentationContext; +import io.opentelemetry.auto.instrumentation.api.SpanWithScope; import io.opentelemetry.auto.tooling.Instrumenter; import java.util.HashMap; import java.util.Map; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletResponse; +import net.bytebuddy.asm.Advice; import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; +/** + * Class javax.servlet.http.HttpServletResponse got method getStatus only + * in Servlet specification version 3.0. This means that we cannot set {@link + * io.opentelemetry.auto.instrumentation.api.Tags#HTTP_STATUS} attribute on the created span using + * just response object. + * + *

This instrumentation intercepts status setting methods from Servlet 2.0 specification and + * stores that status into context store. Then {@link Servlet2Advice#stopSpan(ServletRequest, + * ServletResponse, SpanWithScope, Throwable)} can get it from context and set required span + * attribute. + */ @AutoService(Instrumenter.class) public final class Servlet2ResponseStatusInstrumentation extends Instrumenter.Default { public Servlet2ResponseStatusInstrumentation() { @@ -51,16 +68,27 @@ public Map contextStore() { return singletonMap("javax.servlet.ServletResponse", Integer.class.getName()); } - /** - * Unlike Servlet2Instrumentation it doesn't matter if the HttpServletResponseInstrumentation - * applies first - */ @Override public Map, String> transformers() { final Map, String> transformers = new HashMap<>(); transformers.put( - named("sendError").or(named("setStatus")), packageName + ".Servlet2ResponseStatusAdvice"); - transformers.put(named("sendRedirect"), packageName + ".Servlet2ResponseRedirectAdvice"); + named("sendError").or(named("setStatus")), Servlet2ResponseStatusAdvice.class.getName()); + transformers.put(named("sendRedirect"), Servlet2ResponseRedirectAdvice.class.getName()); return transformers; } + + public static class Servlet2ResponseRedirectAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onEnter(@Advice.This final HttpServletResponse response) { + InstrumentationContext.get(ServletResponse.class, Integer.class).put(response, 302); + } + } + + public static class Servlet2ResponseStatusAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onEnter( + @Advice.This final HttpServletResponse response, @Advice.Argument(0) final Integer status) { + InstrumentationContext.get(ServletResponse.class, Integer.class).put(response, status); + } + } } diff --git a/instrumentation/servlet/request-2.3/src/test/groovy/JettyServlet2Test.groovy b/instrumentation/servlet/request-2.3/src/test/groovy/JettyServlet2Test.groovy index 3033a8903fde..31224ad321fd 100644 --- a/instrumentation/servlet/request-2.3/src/test/groovy/JettyServlet2Test.groovy +++ b/instrumentation/servlet/request-2.3/src/test/groovy/JettyServlet2Test.groovy @@ -85,7 +85,7 @@ class JettyServlet2Test extends HttpServerTest { // parent span must be cast otherwise it breaks debugging classloading (junit loads it early) void serverSpan(TraceAssert trace, int index, String traceID = null, String parentID = null, String method = "GET", ServerEndpoint endpoint = SUCCESS) { trace.span(index) { - operationName expectedOperationName(method) + operationName 'HttpServlet.service' spanKind SERVER errored endpoint.errored if (parentID != null) { diff --git a/instrumentation/servlet/request-3.0/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v3_0/Servlet3Advice.java b/instrumentation/servlet/request-3.0/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v3_0/Servlet3Advice.java index 8f1cca4f87fb..e673e98daf08 100644 --- a/instrumentation/servlet/request-3.0/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v3_0/Servlet3Advice.java +++ b/instrumentation/servlet/request-3.0/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v3_0/Servlet3Advice.java @@ -29,6 +29,7 @@ import io.opentelemetry.auto.instrumentation.api.Tags; import io.opentelemetry.trace.Span; import io.opentelemetry.trace.Status; +import java.lang.reflect.Method; import java.security.Principal; import java.util.concurrent.atomic.AtomicBoolean; import javax.servlet.ServletRequest; @@ -42,6 +43,7 @@ public class Servlet3Advice { @Advice.OnMethodEnter(suppress = Throwable.class) public static SpanWithScope onEnter( @Advice.This final Object servlet, + @Advice.Origin final Method method, @Advice.Argument(0) final ServletRequest request, @Advice.Argument(1) final ServletResponse response) { if (!(request instanceof HttpServletRequest)) { @@ -75,7 +77,7 @@ public static SpanWithScope onEnter( .put((HttpServletResponse) response, httpServletRequest); final Span.Builder builder = - TRACER.spanBuilder(DECORATE.spanNameForRequest(httpServletRequest)).setSpanKind(SERVER); + TRACER.spanBuilder(DECORATE.spanNameForMethod(method)).setSpanKind(SERVER); builder.setParent(extract(httpServletRequest, GETTER)); final Span span = builder.setAttribute("span.origin.type", servlet.getClass().getName()).startSpan(); diff --git a/instrumentation/servlet/request-3.0/src/test/groovy/AbstractServlet3Test.groovy b/instrumentation/servlet/request-3.0/src/test/groovy/AbstractServlet3Test.groovy index 94806895cd1d..b0cd736993ea 100644 --- a/instrumentation/servlet/request-3.0/src/test/groovy/AbstractServlet3Test.groovy +++ b/instrumentation/servlet/request-3.0/src/test/groovy/AbstractServlet3Test.groovy @@ -71,7 +71,7 @@ abstract class AbstractServlet3Test extends HttpServerTest extends HttpServerTest return "tomcat-context" } + @Override + protected String entryPointName() { + return 'ApplicationFilterChain.doFilter' + } + @Override void addServlet(Context servletContext, String path, Class servlet) { String name = UUID.randomUUID() diff --git a/instrumentation/servlet/src/main/java/io/opentelemetry/auto/instrumentation/servlet/filter/FilterInstrumentation.java b/instrumentation/servlet/src/main/java/io/opentelemetry/auto/instrumentation/servlet/filter/FilterInstrumentation.java index 3d54f7c75e7c..66a3db9503dc 100644 --- a/instrumentation/servlet/src/main/java/io/opentelemetry/auto/instrumentation/servlet/filter/FilterInstrumentation.java +++ b/instrumentation/servlet/src/main/java/io/opentelemetry/auto/instrumentation/servlet/filter/FilterInstrumentation.java @@ -36,6 +36,11 @@ import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; +/** + * Instruments all filter invocations in filter chain. + * + *

See README.md for more information about different servlet instrumentations. + */ @AutoService(Instrumenter.class) public final class FilterInstrumentation extends Instrumenter.Default { public FilterInstrumentation() { diff --git a/instrumentation/servlet/src/main/java/io/opentelemetry/auto/instrumentation/servlet/http/HttpServletInstrumentation.java b/instrumentation/servlet/src/main/java/io/opentelemetry/auto/instrumentation/servlet/http/HttpServletInstrumentation.java index a2d4c9cf153a..72a95b65fad0 100644 --- a/instrumentation/servlet/src/main/java/io/opentelemetry/auto/instrumentation/servlet/http/HttpServletInstrumentation.java +++ b/instrumentation/servlet/src/main/java/io/opentelemetry/auto/instrumentation/servlet/http/HttpServletInstrumentation.java @@ -31,6 +31,7 @@ import io.opentelemetry.auto.instrumentation.api.SpanWithScope; import io.opentelemetry.auto.tooling.Instrumenter; import io.opentelemetry.trace.Span; +import io.opentelemetry.trace.Span.Kind; import java.lang.reflect.Method; import java.util.Map; import net.bytebuddy.asm.Advice; @@ -38,6 +39,7 @@ import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; +// Please read README.md of this subproject to understand what is this instrumentation. @AutoService(Instrumenter.class) public final class HttpServletInstrumentation extends Instrumenter.Default { public HttpServletInstrumentation() { @@ -93,7 +95,10 @@ public static SpanWithScope start(@Advice.Origin final Method method) { } // Here we use the Method instead of "this.class.name" to distinguish calls to "super". - final Span span = TRACER.spanBuilder(DECORATE.spanNameForMethod(method)).startSpan(); + final Span span = + TRACER + .spanBuilder(DECORATE.spanNameForMethod(method)) + .startSpan(); DECORATE.afterStart(span); return new SpanWithScope(span, currentContextWith(span));