From 763779e08ae8ae5b2b303fa18e6964797f73c1f5 Mon Sep 17 00:00:00 2001 From: Nikita Salnikov-Tarnovski Date: Thu, 9 Jul 2020 02:43:36 +0300 Subject: [PATCH] Move servlet instrumentations around (#601) --- README.md | 2 +- instrumentation-core/servlet/servlet.gradle | 13 ++++++++ .../servlet/HttpServletRequestGetter.java | 2 +- .../servlet/ServletHttpServerTracer.java | 2 +- .../jetty/JettyHandlerInstrumentation.java | 4 +-- instrumentation/servlet/README.md | 30 +++++++++++++++++- .../servlet-2.2.gradle} | 2 +- .../servlet/v2_2}/Servlet2Advice.java | 4 +-- .../v2_2}/Servlet2HttpServerTracer.java | 6 ++-- .../v2_2}/Servlet2Instrumentation.java | 6 ++-- ...Servlet2ResponseStatusInstrumentation.java | 2 +- .../src/test/groovy/JettyServlet2Test.groovy | 17 ++++++++-- .../groovy/ServletTestInstrumentation.java | 0 .../src/test/groovy/TestServlet2.groovy | 0 .../src/test/resources/realm.properties | 0 .../servlet/servlet-3.0/servlet-3.0.gradle | 2 +- .../v3_0/AsyncContextInstrumentation.java | 4 +-- .../v3_0/Servlet3HttpServerTracer.java | 2 +- .../servlet/v3_0/Servlet3Instrumentation.java | 4 +-- .../servlet-common/servlet-common.gradle | 2 +- .../RequestDispatcherDecorator.java | 0 .../RequestDispatcherInstrumentation.java | 0 .../ServletContextInstrumentation.java | 0 .../servlet/filter/FilterDecorator.java | 0 .../servlet/filter/FilterInstrumentation.java | 0 .../servlet/http/HttpServletDecorator.java | 0 .../http/HttpServletInstrumentation.java | 0 .../http/HttpServletResponseDecorator.java | 0 .../HttpServletResponseInstrumentation.java | 0 .../src/test/groovy/FilterTest.groovy | 0 .../groovy/HttpServletResponseTest.groovy | 10 ++++++ .../src/test/groovy/HttpServletTest.groovy | 0 .../test/groovy/RequestDispatcherTest.groovy | 0 .../test/groovy/RequestDispatcherUtils.java | 5 +++ instrumentation/servlet/servlet.gradle | 31 ------------------- .../spring-webmvc-3.1.gradle | 1 - .../test/boot/SpringBootBasedTest.groovy | 13 +------- .../test/filter/ServletFilterTest.groovy | 26 +++++----------- settings.gradle | 4 +-- .../auto/test/base/HttpServerTest.groovy | 2 +- 40 files changed, 105 insertions(+), 91 deletions(-) create mode 100644 instrumentation-core/servlet/servlet.gradle rename {instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/auto => instrumentation-core/servlet/src/main/java/io/opentelemetry}/instrumentation/servlet/HttpServletRequestGetter.java (94%) rename {instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/auto => instrumentation-core/servlet/src/main/java/io/opentelemetry}/instrumentation/servlet/ServletHttpServerTracer.java (98%) rename instrumentation/servlet/{servlet-2.3/servlet-2.3.gradle => servlet-2.2/servlet-2.2.gradle} (93%) rename instrumentation/servlet/{servlet-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3 => servlet-2.2/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_2}/Servlet2Advice.java (95%) rename instrumentation/servlet/{servlet-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3 => servlet-2.2/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_2}/Servlet2HttpServerTracer.java (81%) rename instrumentation/servlet/{servlet-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3 => servlet-2.2/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_2}/Servlet2Instrumentation.java (93%) rename instrumentation/servlet/{servlet-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3 => servlet-2.2/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_2}/Servlet2ResponseStatusInstrumentation.java (98%) rename instrumentation/servlet/{servlet-2.3 => servlet-2.2}/src/test/groovy/JettyServlet2Test.groovy (92%) rename instrumentation/servlet/{servlet-2.3 => servlet-2.2}/src/test/groovy/ServletTestInstrumentation.java (100%) rename instrumentation/servlet/{servlet-2.3 => servlet-2.2}/src/test/groovy/TestServlet2.groovy (100%) rename instrumentation/servlet/{servlet-2.3 => servlet-2.2}/src/test/resources/realm.properties (100%) rename instrumentation/servlet/{ => servlet-common}/src/main/java/io/opentelemetry/auto/instrumentation/servlet/dispatcher/RequestDispatcherDecorator.java (100%) rename instrumentation/servlet/{ => servlet-common}/src/main/java/io/opentelemetry/auto/instrumentation/servlet/dispatcher/RequestDispatcherInstrumentation.java (100%) rename instrumentation/servlet/{ => servlet-common}/src/main/java/io/opentelemetry/auto/instrumentation/servlet/dispatcher/ServletContextInstrumentation.java (100%) rename instrumentation/servlet/{ => servlet-common}/src/main/java/io/opentelemetry/auto/instrumentation/servlet/filter/FilterDecorator.java (100%) rename instrumentation/servlet/{ => servlet-common}/src/main/java/io/opentelemetry/auto/instrumentation/servlet/filter/FilterInstrumentation.java (100%) rename instrumentation/servlet/{ => servlet-common}/src/main/java/io/opentelemetry/auto/instrumentation/servlet/http/HttpServletDecorator.java (100%) rename instrumentation/servlet/{ => servlet-common}/src/main/java/io/opentelemetry/auto/instrumentation/servlet/http/HttpServletInstrumentation.java (100%) rename instrumentation/servlet/{ => servlet-common}/src/main/java/io/opentelemetry/auto/instrumentation/servlet/http/HttpServletResponseDecorator.java (100%) rename instrumentation/servlet/{ => servlet-common}/src/main/java/io/opentelemetry/auto/instrumentation/servlet/http/HttpServletResponseInstrumentation.java (100%) rename instrumentation/servlet/{ => servlet-common}/src/test/groovy/FilterTest.groovy (100%) rename instrumentation/servlet/{ => servlet-common}/src/test/groovy/HttpServletResponseTest.groovy (97%) rename instrumentation/servlet/{ => servlet-common}/src/test/groovy/HttpServletTest.groovy (100%) rename instrumentation/servlet/{ => servlet-common}/src/test/groovy/RequestDispatcherTest.groovy (100%) rename instrumentation/servlet/{ => servlet-common}/src/test/groovy/RequestDispatcherUtils.java (98%) delete mode 100644 instrumentation/servlet/servlet.gradle diff --git a/README.md b/README.md index c4adb38c8c..c3cf30554e 100644 --- a/README.md +++ b/README.md @@ -146,7 +146,7 @@ provide the path to a JAR file including an SPI implementation using the system | [Rediscala](https://github.com/etaty/rediscala) | 1.8+ | | [RMI](https://docs.oracle.com/en/java/javase/11/docs/api/java.rmi/java/rmi/package-summary.html) | Java 7+ | | [RxJava](https://github.com/ReactiveX/RxJava) | 1.0+ | -| [Servlet](https://javaee.github.io/javaee-spec/javadocs/javax/servlet/package-summary.html) | 2.3+ | +| [Servlet](https://javaee.github.io/javaee-spec/javadocs/javax/servlet/package-summary.html) | 2.2+ | | [Spark Web Framework](https://github.com/perwendel/spark) | 2.3+ | | [Spring Data](https://spring.io/projects/spring-data) | 1.8+ | | [Spring Scheduling](https://docs.spring.io/spring/docs/current/javadoc-api/org/springframework/scheduling/package-summary.html) | 3.1+ | diff --git a/instrumentation-core/servlet/servlet.gradle b/instrumentation-core/servlet/servlet.gradle new file mode 100644 index 0000000000..118f11189f --- /dev/null +++ b/instrumentation-core/servlet/servlet.gradle @@ -0,0 +1,13 @@ +group = 'io.opentelemetry.instrumentation' + +apply from: "$rootDir/gradle/java.gradle" + +dependencies { + api deps.opentelemetryApi + api project(':auto-bootstrap') + + implementation deps.slf4j + + compileOnly group: 'javax.servlet', name: 'servlet-api', version: '2.3' +} + diff --git a/instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/auto/instrumentation/servlet/HttpServletRequestGetter.java b/instrumentation-core/servlet/src/main/java/io/opentelemetry/instrumentation/servlet/HttpServletRequestGetter.java similarity index 94% rename from instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/auto/instrumentation/servlet/HttpServletRequestGetter.java rename to instrumentation-core/servlet/src/main/java/io/opentelemetry/instrumentation/servlet/HttpServletRequestGetter.java index 501cafe43c..ceb82df1ec 100644 --- a/instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/auto/instrumentation/servlet/HttpServletRequestGetter.java +++ b/instrumentation-core/servlet/src/main/java/io/opentelemetry/instrumentation/servlet/HttpServletRequestGetter.java @@ -14,7 +14,7 @@ * limitations under the License. */ -package io.opentelemetry.auto.instrumentation.servlet; +package io.opentelemetry.instrumentation.servlet; import io.opentelemetry.context.propagation.HttpTextFormat; import javax.servlet.http.HttpServletRequest; diff --git a/instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/auto/instrumentation/servlet/ServletHttpServerTracer.java b/instrumentation-core/servlet/src/main/java/io/opentelemetry/instrumentation/servlet/ServletHttpServerTracer.java similarity index 98% rename from instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/auto/instrumentation/servlet/ServletHttpServerTracer.java rename to instrumentation-core/servlet/src/main/java/io/opentelemetry/instrumentation/servlet/ServletHttpServerTracer.java index e33bf61664..6b212021a9 100644 --- a/instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/auto/instrumentation/servlet/ServletHttpServerTracer.java +++ b/instrumentation-core/servlet/src/main/java/io/opentelemetry/instrumentation/servlet/ServletHttpServerTracer.java @@ -14,7 +14,7 @@ * limitations under the License. */ -package io.opentelemetry.auto.instrumentation.servlet; +package io.opentelemetry.instrumentation.servlet; import io.grpc.Context; import io.opentelemetry.auto.bootstrap.instrumentation.decorator.HttpServerTracer; diff --git a/instrumentation/jetty-8.0/src/main/java/io/opentelemetry/auto/instrumentation/jetty/JettyHandlerInstrumentation.java b/instrumentation/jetty-8.0/src/main/java/io/opentelemetry/auto/instrumentation/jetty/JettyHandlerInstrumentation.java index 82cfd8a5f8..e643950fec 100644 --- a/instrumentation/jetty-8.0/src/main/java/io/opentelemetry/auto/instrumentation/jetty/JettyHandlerInstrumentation.java +++ b/instrumentation/jetty-8.0/src/main/java/io/opentelemetry/auto/instrumentation/jetty/JettyHandlerInstrumentation.java @@ -56,8 +56,8 @@ public final class JettyHandlerInstrumentation extends Instrumenter.Default { packageName + ".JettyHttpServerTracer", "io.opentelemetry.auto.instrumentation.servlet.v3_0.Servlet3HttpServerTracer", "io.opentelemetry.auto.instrumentation.servlet.v3_0.TagSettingAsyncListener", - "io.opentelemetry.auto.instrumentation.servlet.ServletHttpServerTracer", - "io.opentelemetry.auto.instrumentation.servlet.HttpServletRequestGetter", + "io.opentelemetry.instrumentation.servlet.ServletHttpServerTracer", + "io.opentelemetry.instrumentation.servlet.HttpServletRequestGetter", }; } diff --git a/instrumentation/servlet/README.md b/instrumentation/servlet/README.md index 4ef1424aa3..959fd495a5 100644 --- a/instrumentation/servlet/README.md +++ b/instrumentation/servlet/README.md @@ -1,5 +1,19 @@ # Instrumentation for Java Servlets +## A word about version + +We support Servlet API starting from version 2.2. +But various instrumentations apply to different versions of the API. +They are divided into 3 sub-modules: + +`servlet-common` contains instrumentations applicable to all API versions that we support. + +`servlet-2.2` contains instrumentations applicable to Servlet API 2.2, but not to to 3+. + +`servlet-3.0` contains instrumentations that require Servlet API 3.0 or newer. + +## Implementation details + 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. @@ -66,9 +80,23 @@ Span created by Spring MVC instrumentation will have `kind=INTERNAL` and named ` 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. +Because 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). + +## Additional instrumentations +`RequestDispatcherInstrumentation` instruments `javax.servlet.RequestDispatcher.forward` and +`javax.servlet.RequestDispatcher.include` methods to create new `INTERNAL` spans around their +invocations. + +`ServletContextInstrumentation` instruments `javax.servlet.ServletContext.getRequestDispatcher` and +`javax.servlet.ServletContext.getNamedDispatcher`. The only job of this instrumentation is to +preserve the input parameter of those methods and to make that available for `RequestDispatcherInstrumentation` +described above. The latter uses that name for `dispatcher.target` span attribute. + +`HttpServletResponseInstrumentation` instruments `javax.servlet.http.HttpServletResponse.sendError` +and `javax.servlet.http.HttpServletResponse.sendRedirect` methods to create new `INTERNAL` spans +around their invocations. diff --git a/instrumentation/servlet/servlet-2.3/servlet-2.3.gradle b/instrumentation/servlet/servlet-2.2/servlet-2.2.gradle similarity index 93% rename from instrumentation/servlet/servlet-2.3/servlet-2.3.gradle rename to instrumentation/servlet/servlet-2.2/servlet-2.2.gradle index 8da0561fc3..05f0cfca6f 100644 --- a/instrumentation/servlet/servlet-2.3/servlet-2.3.gradle +++ b/instrumentation/servlet/servlet-2.2/servlet-2.2.gradle @@ -24,7 +24,7 @@ testSets { dependencies { compileOnly group: 'javax.servlet', name: 'servlet-api', version: '2.2' - implementation(project(':instrumentation:servlet:servlet-common')) + api(project(':instrumentation-core:servlet')) testImplementation(project(':testing')) { exclude group: 'org.eclipse.jetty', module: 'jetty-server' diff --git a/instrumentation/servlet/servlet-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3/Servlet2Advice.java b/instrumentation/servlet/servlet-2.2/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_2/Servlet2Advice.java similarity index 95% rename from instrumentation/servlet/servlet-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3/Servlet2Advice.java rename to instrumentation/servlet/servlet-2.2/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_2/Servlet2Advice.java index d04b693dbf..a30241c28b 100644 --- a/instrumentation/servlet/servlet-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3/Servlet2Advice.java +++ b/instrumentation/servlet/servlet-2.2/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_2/Servlet2Advice.java @@ -14,9 +14,9 @@ * limitations under the License. */ -package io.opentelemetry.auto.instrumentation.servlet.v2_3; +package io.opentelemetry.auto.instrumentation.servlet.v2_2; -import static io.opentelemetry.auto.instrumentation.servlet.v2_3.Servlet2HttpServerTracer.TRACER; +import static io.opentelemetry.auto.instrumentation.servlet.v2_2.Servlet2HttpServerTracer.TRACER; import io.opentelemetry.auto.bootstrap.InstrumentationContext; import io.opentelemetry.context.Scope; diff --git a/instrumentation/servlet/servlet-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3/Servlet2HttpServerTracer.java b/instrumentation/servlet/servlet-2.2/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_2/Servlet2HttpServerTracer.java similarity index 81% rename from instrumentation/servlet/servlet-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3/Servlet2HttpServerTracer.java rename to instrumentation/servlet/servlet-2.2/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_2/Servlet2HttpServerTracer.java index eda70e1f66..f0a8e1e1a7 100644 --- a/instrumentation/servlet/servlet-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3/Servlet2HttpServerTracer.java +++ b/instrumentation/servlet/servlet-2.2/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_2/Servlet2HttpServerTracer.java @@ -14,14 +14,14 @@ * limitations under the License. */ -package io.opentelemetry.auto.instrumentation.servlet.v2_3; +package io.opentelemetry.auto.instrumentation.servlet.v2_2; -import io.opentelemetry.auto.instrumentation.servlet.ServletHttpServerTracer; +import io.opentelemetry.instrumentation.servlet.ServletHttpServerTracer; public class Servlet2HttpServerTracer extends ServletHttpServerTracer { public static final Servlet2HttpServerTracer TRACER = new Servlet2HttpServerTracer(); protected String getInstrumentationName() { - return "io.opentelemetry.auto.servlet-2.3"; + return "io.opentelemetry.auto.servlet-2.2"; } } diff --git a/instrumentation/servlet/servlet-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3/Servlet2Instrumentation.java b/instrumentation/servlet/servlet-2.2/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_2/Servlet2Instrumentation.java similarity index 93% rename from instrumentation/servlet/servlet-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3/Servlet2Instrumentation.java rename to instrumentation/servlet/servlet-2.2/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_2/Servlet2Instrumentation.java index 0d8cbaef0b..8266a780ed 100644 --- a/instrumentation/servlet/servlet-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3/Servlet2Instrumentation.java +++ b/instrumentation/servlet/servlet-2.2/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_2/Servlet2Instrumentation.java @@ -14,7 +14,7 @@ * limitations under the License. */ -package io.opentelemetry.auto.instrumentation.servlet.v2_3; +package io.opentelemetry.auto.instrumentation.servlet.v2_2; import static io.opentelemetry.auto.tooling.ClassLoaderMatcher.hasClassesNamed; import static io.opentelemetry.auto.tooling.bytebuddy.matcher.AgentElementMatchers.safeHasSuperType; @@ -61,8 +61,8 @@ public final class Servlet2Instrumentation extends Instrumenter.Default { @Override public String[] helperClassNames() { return new String[] { - "io.opentelemetry.auto.instrumentation.servlet.ServletHttpServerTracer", - "io.opentelemetry.auto.instrumentation.servlet.HttpServletRequestGetter", + "io.opentelemetry.instrumentation.servlet.ServletHttpServerTracer", + "io.opentelemetry.instrumentation.servlet.HttpServletRequestGetter", packageName + ".Servlet2HttpServerTracer" }; } diff --git a/instrumentation/servlet/servlet-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3/Servlet2ResponseStatusInstrumentation.java b/instrumentation/servlet/servlet-2.2/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_2/Servlet2ResponseStatusInstrumentation.java similarity index 98% rename from instrumentation/servlet/servlet-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3/Servlet2ResponseStatusInstrumentation.java rename to instrumentation/servlet/servlet-2.2/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_2/Servlet2ResponseStatusInstrumentation.java index 63d95ef680..f364a8c406 100644 --- a/instrumentation/servlet/servlet-2.3/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_3/Servlet2ResponseStatusInstrumentation.java +++ b/instrumentation/servlet/servlet-2.2/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v2_2/Servlet2ResponseStatusInstrumentation.java @@ -14,7 +14,7 @@ * limitations under the License. */ -package io.opentelemetry.auto.instrumentation.servlet.v2_3; +package io.opentelemetry.auto.instrumentation.servlet.v2_2; import static io.opentelemetry.auto.tooling.ClassLoaderMatcher.hasClassesNamed; import static io.opentelemetry.auto.tooling.bytebuddy.matcher.AgentElementMatchers.safeHasSuperType; diff --git a/instrumentation/servlet/servlet-2.3/src/test/groovy/JettyServlet2Test.groovy b/instrumentation/servlet/servlet-2.2/src/test/groovy/JettyServlet2Test.groovy similarity index 92% rename from instrumentation/servlet/servlet-2.3/src/test/groovy/JettyServlet2Test.groovy rename to instrumentation/servlet/servlet-2.2/src/test/groovy/JettyServlet2Test.groovy index b52df3d1ed..3814e176ff 100644 --- a/instrumentation/servlet/servlet-2.3/src/test/groovy/JettyServlet2Test.groovy +++ b/instrumentation/servlet/servlet-2.2/src/test/groovy/JettyServlet2Test.groovy @@ -17,19 +17,20 @@ import io.opentelemetry.auto.instrumentation.api.MoreAttributes import io.opentelemetry.auto.test.asserts.TraceAssert import io.opentelemetry.auto.test.base.HttpServerTest +import io.opentelemetry.sdk.trace.data.SpanData +import javax.servlet.http.HttpServletRequest import io.opentelemetry.trace.attributes.SemanticAttributes import org.eclipse.jetty.server.Server import org.eclipse.jetty.server.handler.ErrorHandler import org.eclipse.jetty.servlet.ServletContextHandler -import javax.servlet.http.HttpServletRequest - import static io.opentelemetry.auto.test.base.HttpServerTest.ServerEndpoint.AUTH_REQUIRED import static io.opentelemetry.auto.test.base.HttpServerTest.ServerEndpoint.ERROR import static io.opentelemetry.auto.test.base.HttpServerTest.ServerEndpoint.EXCEPTION import static io.opentelemetry.auto.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM import static io.opentelemetry.auto.test.base.HttpServerTest.ServerEndpoint.REDIRECT import static io.opentelemetry.auto.test.base.HttpServerTest.ServerEndpoint.SUCCESS +import static io.opentelemetry.trace.Span.Kind.INTERNAL import static io.opentelemetry.trace.Span.Kind.SERVER class JettyServlet2Test extends HttpServerTest { @@ -83,6 +84,18 @@ class JettyServlet2Test extends HttpServerTest { false } + @Override + void responseSpan(TraceAssert trace, int index, Object parent, String method = "GET", ServerEndpoint endpoint = SUCCESS) { + trace.span(index) { + operationName endpoint == REDIRECT ? "HttpServletResponse.sendRedirect" : "HttpServletResponse.sendError" + spanKind INTERNAL + errored false + childOf((SpanData) parent) + attributes { + } + } + } + // 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) { diff --git a/instrumentation/servlet/servlet-2.3/src/test/groovy/ServletTestInstrumentation.java b/instrumentation/servlet/servlet-2.2/src/test/groovy/ServletTestInstrumentation.java similarity index 100% rename from instrumentation/servlet/servlet-2.3/src/test/groovy/ServletTestInstrumentation.java rename to instrumentation/servlet/servlet-2.2/src/test/groovy/ServletTestInstrumentation.java diff --git a/instrumentation/servlet/servlet-2.3/src/test/groovy/TestServlet2.groovy b/instrumentation/servlet/servlet-2.2/src/test/groovy/TestServlet2.groovy similarity index 100% rename from instrumentation/servlet/servlet-2.3/src/test/groovy/TestServlet2.groovy rename to instrumentation/servlet/servlet-2.2/src/test/groovy/TestServlet2.groovy diff --git a/instrumentation/servlet/servlet-2.3/src/test/resources/realm.properties b/instrumentation/servlet/servlet-2.2/src/test/resources/realm.properties similarity index 100% rename from instrumentation/servlet/servlet-2.3/src/test/resources/realm.properties rename to instrumentation/servlet/servlet-2.2/src/test/resources/realm.properties diff --git a/instrumentation/servlet/servlet-3.0/servlet-3.0.gradle b/instrumentation/servlet/servlet-3.0/servlet-3.0.gradle index 8b377c452b..befc920be0 100644 --- a/instrumentation/servlet/servlet-3.0/servlet-3.0.gradle +++ b/instrumentation/servlet/servlet-3.0/servlet-3.0.gradle @@ -22,8 +22,8 @@ testSets { } dependencies { - api(project(':instrumentation:servlet:servlet-common')) compileOnly group: 'javax.servlet', name: 'javax.servlet-api', version: '3.0.1' + api(project(':instrumentation-core:servlet')) testImplementation(project(':testing')) { exclude group: 'org.eclipse.jetty', module: 'jetty-server' diff --git a/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v3_0/AsyncContextInstrumentation.java b/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v3_0/AsyncContextInstrumentation.java index ccfb31c087..411f4d3eca 100644 --- a/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v3_0/AsyncContextInstrumentation.java +++ b/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v3_0/AsyncContextInstrumentation.java @@ -48,8 +48,8 @@ public final class AsyncContextInstrumentation extends Instrumenter.Default { @Override public String[] helperClassNames() { return new String[] { - "io.opentelemetry.auto.instrumentation.servlet.HttpServletRequestGetter", - "io.opentelemetry.auto.instrumentation.servlet.ServletHttpServerTracer", + "io.opentelemetry.instrumentation.servlet.HttpServletRequestGetter", + "io.opentelemetry.instrumentation.servlet.ServletHttpServerTracer", packageName + ".Servlet3HttpServerTracer" }; } diff --git a/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v3_0/Servlet3HttpServerTracer.java b/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v3_0/Servlet3HttpServerTracer.java index 7c686612f7..c2a8b075ce 100644 --- a/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v3_0/Servlet3HttpServerTracer.java +++ b/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v3_0/Servlet3HttpServerTracer.java @@ -19,7 +19,7 @@ package io.opentelemetry.auto.instrumentation.servlet.v3_0; import static io.opentelemetry.trace.TracingContextUtils.getSpan; import io.grpc.Context; -import io.opentelemetry.auto.instrumentation.servlet.ServletHttpServerTracer; +import io.opentelemetry.instrumentation.servlet.ServletHttpServerTracer; import io.opentelemetry.trace.Span; import io.opentelemetry.trace.Status; import javax.servlet.http.HttpServletRequest; diff --git a/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v3_0/Servlet3Instrumentation.java b/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v3_0/Servlet3Instrumentation.java index 31daa83632..97c9fb553b 100644 --- a/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v3_0/Servlet3Instrumentation.java +++ b/instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/auto/instrumentation/servlet/v3_0/Servlet3Instrumentation.java @@ -52,8 +52,8 @@ public final class Servlet3Instrumentation extends Instrumenter.Default { @Override public String[] helperClassNames() { return new String[] { - "io.opentelemetry.auto.instrumentation.servlet.HttpServletRequestGetter", - "io.opentelemetry.auto.instrumentation.servlet.ServletHttpServerTracer", + "io.opentelemetry.instrumentation.servlet.HttpServletRequestGetter", + "io.opentelemetry.instrumentation.servlet.ServletHttpServerTracer", packageName + ".Servlet3HttpServerTracer", packageName + ".TagSettingAsyncListener" }; diff --git a/instrumentation/servlet/servlet-common/servlet-common.gradle b/instrumentation/servlet/servlet-common/servlet-common.gradle index 836134629c..56a3fd7338 100644 --- a/instrumentation/servlet/servlet-common/servlet-common.gradle +++ b/instrumentation/servlet/servlet-common/servlet-common.gradle @@ -15,7 +15,7 @@ testSets { } dependencies { - compileOnly group: 'javax.servlet', name: 'servlet-api', version: '2.2' + compileOnly group: 'javax.servlet', name: 'servlet-api', version: '2.3' testImplementation(project(':testing')) { exclude group: 'org.eclipse.jetty', module: 'jetty-server' diff --git a/instrumentation/servlet/src/main/java/io/opentelemetry/auto/instrumentation/servlet/dispatcher/RequestDispatcherDecorator.java b/instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/auto/instrumentation/servlet/dispatcher/RequestDispatcherDecorator.java similarity index 100% rename from instrumentation/servlet/src/main/java/io/opentelemetry/auto/instrumentation/servlet/dispatcher/RequestDispatcherDecorator.java rename to instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/auto/instrumentation/servlet/dispatcher/RequestDispatcherDecorator.java diff --git a/instrumentation/servlet/src/main/java/io/opentelemetry/auto/instrumentation/servlet/dispatcher/RequestDispatcherInstrumentation.java b/instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/auto/instrumentation/servlet/dispatcher/RequestDispatcherInstrumentation.java similarity index 100% rename from instrumentation/servlet/src/main/java/io/opentelemetry/auto/instrumentation/servlet/dispatcher/RequestDispatcherInstrumentation.java rename to instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/auto/instrumentation/servlet/dispatcher/RequestDispatcherInstrumentation.java diff --git a/instrumentation/servlet/src/main/java/io/opentelemetry/auto/instrumentation/servlet/dispatcher/ServletContextInstrumentation.java b/instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/auto/instrumentation/servlet/dispatcher/ServletContextInstrumentation.java similarity index 100% rename from instrumentation/servlet/src/main/java/io/opentelemetry/auto/instrumentation/servlet/dispatcher/ServletContextInstrumentation.java rename to instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/auto/instrumentation/servlet/dispatcher/ServletContextInstrumentation.java diff --git a/instrumentation/servlet/src/main/java/io/opentelemetry/auto/instrumentation/servlet/filter/FilterDecorator.java b/instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/auto/instrumentation/servlet/filter/FilterDecorator.java similarity index 100% rename from instrumentation/servlet/src/main/java/io/opentelemetry/auto/instrumentation/servlet/filter/FilterDecorator.java rename to instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/auto/instrumentation/servlet/filter/FilterDecorator.java diff --git a/instrumentation/servlet/src/main/java/io/opentelemetry/auto/instrumentation/servlet/filter/FilterInstrumentation.java b/instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/auto/instrumentation/servlet/filter/FilterInstrumentation.java similarity index 100% rename from instrumentation/servlet/src/main/java/io/opentelemetry/auto/instrumentation/servlet/filter/FilterInstrumentation.java rename to instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/auto/instrumentation/servlet/filter/FilterInstrumentation.java diff --git a/instrumentation/servlet/src/main/java/io/opentelemetry/auto/instrumentation/servlet/http/HttpServletDecorator.java b/instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/auto/instrumentation/servlet/http/HttpServletDecorator.java similarity index 100% rename from instrumentation/servlet/src/main/java/io/opentelemetry/auto/instrumentation/servlet/http/HttpServletDecorator.java rename to instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/auto/instrumentation/servlet/http/HttpServletDecorator.java diff --git a/instrumentation/servlet/src/main/java/io/opentelemetry/auto/instrumentation/servlet/http/HttpServletInstrumentation.java b/instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/auto/instrumentation/servlet/http/HttpServletInstrumentation.java similarity index 100% rename from instrumentation/servlet/src/main/java/io/opentelemetry/auto/instrumentation/servlet/http/HttpServletInstrumentation.java rename to instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/auto/instrumentation/servlet/http/HttpServletInstrumentation.java diff --git a/instrumentation/servlet/src/main/java/io/opentelemetry/auto/instrumentation/servlet/http/HttpServletResponseDecorator.java b/instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/auto/instrumentation/servlet/http/HttpServletResponseDecorator.java similarity index 100% rename from instrumentation/servlet/src/main/java/io/opentelemetry/auto/instrumentation/servlet/http/HttpServletResponseDecorator.java rename to instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/auto/instrumentation/servlet/http/HttpServletResponseDecorator.java diff --git a/instrumentation/servlet/src/main/java/io/opentelemetry/auto/instrumentation/servlet/http/HttpServletResponseInstrumentation.java b/instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/auto/instrumentation/servlet/http/HttpServletResponseInstrumentation.java similarity index 100% rename from instrumentation/servlet/src/main/java/io/opentelemetry/auto/instrumentation/servlet/http/HttpServletResponseInstrumentation.java rename to instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/auto/instrumentation/servlet/http/HttpServletResponseInstrumentation.java diff --git a/instrumentation/servlet/src/test/groovy/FilterTest.groovy b/instrumentation/servlet/servlet-common/src/test/groovy/FilterTest.groovy similarity index 100% rename from instrumentation/servlet/src/test/groovy/FilterTest.groovy rename to instrumentation/servlet/servlet-common/src/test/groovy/FilterTest.groovy diff --git a/instrumentation/servlet/src/test/groovy/HttpServletResponseTest.groovy b/instrumentation/servlet/servlet-common/src/test/groovy/HttpServletResponseTest.groovy similarity index 97% rename from instrumentation/servlet/src/test/groovy/HttpServletResponseTest.groovy rename to instrumentation/servlet/servlet-common/src/test/groovy/HttpServletResponseTest.groovy index a58283d940..e4ecadeda3 100644 --- a/instrumentation/servlet/src/test/groovy/HttpServletResponseTest.groovy +++ b/instrumentation/servlet/servlet-common/src/test/groovy/HttpServletResponseTest.groovy @@ -221,6 +221,11 @@ class HttpServletResponseTest extends AgentTestRunner { return null } + @Override + String getContentType() { + return null + } + @Override ServletOutputStream getOutputStream() throws IOException { return null @@ -231,6 +236,11 @@ class HttpServletResponseTest extends AgentTestRunner { return null } + @Override + void setCharacterEncoding(String charset) { + + } + @Override void setContentLength(int i) { diff --git a/instrumentation/servlet/src/test/groovy/HttpServletTest.groovy b/instrumentation/servlet/servlet-common/src/test/groovy/HttpServletTest.groovy similarity index 100% rename from instrumentation/servlet/src/test/groovy/HttpServletTest.groovy rename to instrumentation/servlet/servlet-common/src/test/groovy/HttpServletTest.groovy diff --git a/instrumentation/servlet/src/test/groovy/RequestDispatcherTest.groovy b/instrumentation/servlet/servlet-common/src/test/groovy/RequestDispatcherTest.groovy similarity index 100% rename from instrumentation/servlet/src/test/groovy/RequestDispatcherTest.groovy rename to instrumentation/servlet/servlet-common/src/test/groovy/RequestDispatcherTest.groovy diff --git a/instrumentation/servlet/src/test/groovy/RequestDispatcherUtils.java b/instrumentation/servlet/servlet-common/src/test/groovy/RequestDispatcherUtils.java similarity index 98% rename from instrumentation/servlet/src/test/groovy/RequestDispatcherUtils.java rename to instrumentation/servlet/servlet-common/src/test/groovy/RequestDispatcherUtils.java index 4f0970f6b1..20da4eebc9 100644 --- a/instrumentation/servlet/src/test/groovy/RequestDispatcherUtils.java +++ b/instrumentation/servlet/servlet-common/src/test/groovy/RequestDispatcherUtils.java @@ -68,6 +68,11 @@ public class RequestDispatcherUtils { } class TestContext implements ServletContext { + @Override + public String getContextPath() { + return null; + } + @Override public ServletContext getContext(final String s) { return null; diff --git a/instrumentation/servlet/servlet.gradle b/instrumentation/servlet/servlet.gradle deleted file mode 100644 index 2c36471595..0000000000 --- a/instrumentation/servlet/servlet.gradle +++ /dev/null @@ -1,31 +0,0 @@ -apply from: "$rootDir/gradle/instrumentation.gradle" - -muzzle { - pass { - group = "javax.servlet" - module = 'javax.servlet-api' - versions = "[,]" - assertInverse = true - } - pass { - group = "javax.servlet" - module = 'servlet-api' - versions = "[,]" - skipVersions += '0' - assertInverse = true - } -} - -dependencies { - compileOnly group: 'javax.servlet', name: 'servlet-api', version: '2.3' - - testImplementation group: 'javax.servlet', name: 'servlet-api', version: '2.3' - - // servlet request instrumentation required for linking request to response. - testImplementation project(':instrumentation:servlet:servlet-2.3') - - // Don't want to conflict with jetty from the test server. - testImplementation(project(':testing')) { - exclude group: 'org.eclipse.jetty', module: 'jetty-server' - } -} diff --git a/instrumentation/spring-webmvc-3.1/spring-webmvc-3.1.gradle b/instrumentation/spring-webmvc-3.1/spring-webmvc-3.1.gradle index a63fbcd475..bab55461c9 100644 --- a/instrumentation/spring-webmvc-3.1/spring-webmvc-3.1.gradle +++ b/instrumentation/spring-webmvc-3.1/spring-webmvc-3.1.gradle @@ -32,7 +32,6 @@ dependencies { } // Include servlet instrumentation for verifying the tomcat requests - testImplementation project(':instrumentation:servlet') testImplementation project(':instrumentation:servlet:servlet-3.0') testImplementation group: 'javax.validation', name: 'validation-api', version: '1.1.0.Final' diff --git a/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/SpringBootBasedTest.groovy b/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/SpringBootBasedTest.groovy index 84a2a2ec22..e6c0283159 100644 --- a/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/SpringBootBasedTest.groovy +++ b/instrumentation/spring-webmvc-3.1/src/test/groovy/test/boot/SpringBootBasedTest.groovy @@ -58,11 +58,6 @@ class SpringBootBasedTest extends HttpServerTest true } - @Override - boolean hasResponseSpan(ServerEndpoint endpoint) { - endpoint == REDIRECT - } - @Override boolean hasRenderSpan(ServerEndpoint endpoint) { endpoint == REDIRECT @@ -103,14 +98,8 @@ class SpringBootBasedTest extends HttpServerTest trace(0, 1) { basicSpan(it, 0, "TEST_SPAN") } - trace(1, 2) { + trace(1, 1) { serverSpan(it, 0, null, null, "POST", LOGIN) - span(1) { - operationName "HttpServletResponse.sendRedirect" - childOf span(0) - attributes { - } - } } } diff --git a/instrumentation/spring-webmvc-3.1/src/test/groovy/test/filter/ServletFilterTest.groovy b/instrumentation/spring-webmvc-3.1/src/test/groovy/test/filter/ServletFilterTest.groovy index 197e26920c..d9d9627914 100644 --- a/instrumentation/spring-webmvc-3.1/src/test/groovy/test/filter/ServletFilterTest.groovy +++ b/instrumentation/spring-webmvc-3.1/src/test/groovy/test/filter/ServletFilterTest.groovy @@ -29,7 +29,6 @@ import test.boot.SecurityConfig import static io.opentelemetry.auto.test.base.HttpServerTest.ServerEndpoint.ERROR import static io.opentelemetry.auto.test.base.HttpServerTest.ServerEndpoint.EXCEPTION import static io.opentelemetry.auto.test.base.HttpServerTest.ServerEndpoint.PATH_PARAM -import static io.opentelemetry.auto.test.base.HttpServerTest.ServerEndpoint.REDIRECT import static io.opentelemetry.auto.test.base.HttpServerTest.ServerEndpoint.SUCCESS import static io.opentelemetry.trace.Span.Kind.INTERNAL import static io.opentelemetry.trace.Span.Kind.SERVER @@ -55,11 +54,6 @@ class ServletFilterTest extends HttpServerTest { false } - @Override - boolean hasResponseSpan(ServerEndpoint endpoint) { - endpoint == REDIRECT || endpoint == ERROR - } - @Override boolean hasErrorPageSpans(ServerEndpoint endpoint) { endpoint == ERROR || endpoint == EXCEPTION @@ -83,13 +77,16 @@ class ServletFilterTest extends HttpServerTest { } @Override - void responseSpan(TraceAssert trace, int index, Object parent, String method = "GET", ServerEndpoint endpoint = SUCCESS) { + void handlerSpan(TraceAssert trace, int index, Object parent, String method = "GET", ServerEndpoint endpoint = SUCCESS) { trace.span(index) { - operationName endpoint == REDIRECT ? "HttpServletResponse.sendRedirect" : "HttpServletResponse.sendError" + operationName "TestController.${endpoint.name().toLowerCase()}" spanKind INTERNAL - errored false + errored endpoint == EXCEPTION childOf((SpanData) parent) attributes { + if (endpoint == EXCEPTION) { + errorTags(Exception, EXCEPTION.body) + } } } } @@ -130,19 +127,10 @@ class ServletFilterTest extends HttpServerTest { @Override void errorPageSpans(TraceAssert trace, int index, Object parent, String method = "GET", ServerEndpoint endpoint = SUCCESS) { trace.span(index) { - operationName "/error" - spanKind INTERNAL - errored false - childOf((SpanData) parent) - attributes { - "dispatcher.target" "/error" - } - } - trace.span(index + 1) { operationName "BasicErrorController.error" spanKind INTERNAL errored false - childOf trace.spans[index] + childOf((SpanData) parent) attributes { } } diff --git a/settings.gradle b/settings.gradle index 2c9a1f1ddb..1973946589 100644 --- a/settings.gradle +++ b/settings.gradle @@ -133,10 +133,9 @@ include ':instrumentation:reactor-3.1' include ':instrumentation:rediscala-1.8' include ':instrumentation:rmi' include ':instrumentation:rxjava-1.0' -include ':instrumentation:servlet' include ':instrumentation:servlet:glassfish-testing' include ':instrumentation:servlet:servlet-common' -include ':instrumentation:servlet:servlet-2.3' +include ':instrumentation:servlet:servlet-2.2' include ':instrumentation:servlet:servlet-3.0' // FIXME this instrumentation relied on scope listener // include ':instrumentation:slf4j-mdc' @@ -151,6 +150,7 @@ include ':instrumentation:vertx-3.0' include ':instrumentation:vertx-reactive-3.5' include ':instrumentation-core:aws-sdk:aws-sdk-2.2' +include ':instrumentation-core:servlet' include ':instrumentation-core:spring' // exporter adapters diff --git a/testing/src/main/groovy/io/opentelemetry/auto/test/base/HttpServerTest.groovy b/testing/src/main/groovy/io/opentelemetry/auto/test/base/HttpServerTest.groovy index a639938d93..d57a42a639 100644 --- a/testing/src/main/groovy/io/opentelemetry/auto/test/base/HttpServerTest.groovy +++ b/testing/src/main/groovy/io/opentelemetry/auto/test/base/HttpServerTest.groovy @@ -389,7 +389,7 @@ abstract class HttpServerTest extends AgentTestRunner { spanCount++ } if (hasErrorPageSpans(endpoint)) { - spanCount += 2 + spanCount ++ } } assertTraces(size * 2) {