Remove disabled by default servlet instrumentation
MODIFY
README.md
README.md
@@ -283,7 +283,6 @@ Because the automatic instrumentation runs in a different classpath than the ins
Some instrumentations can produce too many spans and make traces very noisy.
For this reason, the following instrumentations are disabled by default:
- `jdbc-datasource` which creates spans whenever the `java.sql.DataSource#getConnection` method is called.
-- `servlet-filter` which creates spans around Servlet Filter methods.
- `servlet-service` which creates spans around Servlet methods.
To enable them, add the `otel.instrumentation.<name>.enabled` system property:

MODIFY
instrumentation/servlet/README.md
instrumentation/servlet/README.md
@@ -30,7 +30,7 @@ at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServ
<b>at org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:883)</b>
<b>at javax.servlet.http.HttpServlet.service(HttpServlet.java:741)</b>
...
-<b>at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)</b>
+<b>at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)</b>
...
at org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
@@ -40,40 +40,23 @@ at java.base/java.lang.Thread.run(Thread.java:834)
</pre>
Everything starts when HTTP request processing reaches the first class from Servlet specification.
-In the example above this is `ApplicationFilterChain.doFilter(ServletRequest, ServletResponse)` method.
+In the example above this is the
+`OncePerRequestFilter.doFilter(ServletRequest, ServletResponse, FilterChain)` method.
Let us call this first servlet specific method an "entry point".
-This is the main target for `Servlet3Instrumentation` and `Servlet2Instrumentation` instruments:
+This is the main target for `Servlet3Instrumentation` and `Servlet2Instrumentation`:
-`public void javax.servlet.FilterChain#doFilter(ServletRequest, ServletResponse)`
+`public void javax.servlet.Filter#doFilter(ServletRequest, ServletResponse, FilterChain)`
`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.
+These instrumentations are located in two separate submodules `servlet-3.0` and `servlet-2.2`,
+because they and corresponding tests depend on different versions of the 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 `HttpServletInstrumentationModule`.
-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 `HttpServletInstrumentationModule` is disabled by default.
-In rare cases when you need it, you can enable it using configuration property `otel.instrumentation.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 `FilterInstrumentationModule` which is also disabled by default.
-You can enable it with the configuration property `otel.instrumentation.servlet-filter.enabled`.
At last, request processing may reach the specific framework that your 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,
+All spans from Servlet API will have `kind=SERVER` and name based on corresponding class and method names,
such as `ApplicationFilterChain.doFilter` or `FrameworkServlet.doGet`.
Span created by Spring MVC instrumentation will have `kind=INTERNAL` and named `OwnerController.initCreationForm`.

MODIFY
instrumentation/servlet/servlet-2.2/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v2_2/ServletAndFilterInstrumentation.java
instrumentation/servlet/servlet-2.2/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v2_2/ServletAndFilterInstrumentation.java
@@ -32,11 +32,6 @@ final class ServletAndFilterInstrumentation implements TypeInstrumentation {
return safeHasSuperType(namedOneOf("javax.servlet.Filter", "javax.servlet.http.HttpServlet"));
}
- /**
- * Here we are instrumenting the public method for HttpServlet. This should ensure that this
- * advice is always called before HttpServletInstrumentation which is instrumenting the protected
- * method.
- */
@Override
public Map<? extends ElementMatcher<? super MethodDescription>, String> transformers() {
return singletonMap(

MODIFY
instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/ServletAndFilterInstrumentation.java
instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/ServletAndFilterInstrumentation.java
@@ -23,7 +23,7 @@ final class ServletAndFilterInstrumentation implements TypeInstrumentation {
@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
// Optimization for expensive typeMatcher.
- return hasClassesNamed("javax.servlet.http.HttpServlet");
+ return hasClassesNamed("javax.servlet.Filter");
}
@Override
@@ -31,11 +31,6 @@ final class ServletAndFilterInstrumentation implements TypeInstrumentation {
return safeHasSuperType(namedOneOf("javax.servlet.Filter", "javax.servlet.http.HttpServlet"));
}
- /**
- * Here we are instrumenting the public method for HttpServlet. This should ensure that this
- * advice is always called before HttpServletInstrumentation which is instrumenting the protected
- * method.
- */
@Override
public Map<? extends ElementMatcher<? super MethodDescription>, String> transformers() {
return singletonMap(

DELETE
instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/filter/FilterInstrumentationModule.java
-/*
- * Copyright The OpenTelemetry Authors
- * SPDX-License-Identifier: Apache-2.0
- */
-
-package io.opentelemetry.javaagent.instrumentation.servlet.filter;
-
-import static io.opentelemetry.javaagent.instrumentation.servlet.filter.FilterTracer.tracer;
-import static io.opentelemetry.javaagent.tooling.ClassLoaderMatcher.hasClassesNamed;
-import static io.opentelemetry.javaagent.tooling.bytebuddy.matcher.AgentElementMatchers.implementsInterface;
-import static java.util.Collections.singletonList;
-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 com.google.auto.service.AutoService;
-import io.opentelemetry.api.trace.Span;
-import io.opentelemetry.api.trace.Span.Kind;
-import io.opentelemetry.context.Scope;
-import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge;
-import io.opentelemetry.javaagent.tooling.InstrumentationModule;
-import io.opentelemetry.javaagent.tooling.TypeInstrumentation;
-import java.util.List;
-import java.util.Map;
-import javax.servlet.Filter;
-import net.bytebuddy.asm.Advice;
-import net.bytebuddy.description.method.MethodDescription;
-import net.bytebuddy.description.type.TypeDescription;
-import net.bytebuddy.matcher.ElementMatcher;
-
-/**
- * Instruments all filter invocations in filter chain.
- *
- * <p>See README.md for more information about different servlet instrumentations.
- */
-@AutoService(InstrumentationModule.class)
-public final class FilterInstrumentationModule extends InstrumentationModule {
- public FilterInstrumentationModule() {
- super("servlet-filter");
- }
-
- @Override
- public boolean defaultEnabled() {
- return false;
- }
-
- @Override
- public String[] helperClassNames() {
- return new String[] {
- packageName + ".FilterTracer",
- };
- }
-
- @Override
- public List<TypeInstrumentation> typeInstrumentations() {
- return singletonList(new FilterInstrumentation());
- }
-
- private static final class FilterInstrumentation implements TypeInstrumentation {
- @Override
- public ElementMatcher<ClassLoader> classLoaderMatcher() {
- // Optimization for expensive typeMatcher.
- return hasClassesNamed("javax.servlet.Filter");
- }
-
- @Override
- public ElementMatcher<TypeDescription> typeMatcher() {
- return implementsInterface(named("javax.servlet.Filter"));
- }
-
- @Override
- public Map<? extends ElementMatcher<? super MethodDescription>, String> transformers() {
- return singletonMap(
- named("doFilter")
- .and(takesArgument(0, named("javax.servlet.ServletRequest")))
- .and(takesArgument(1, named("javax.servlet.ServletResponse")))
- .and(isPublic()),
- FilterInstrumentationModule.class.getName() + "$FilterAdvice");
- }
- }
-
- public static class FilterAdvice {
-
- @Advice.OnMethodEnter(suppress = Throwable.class)
- public static void start(
- @Advice.This Filter filter,
- @Advice.Local("otelSpan") Span span,
- @Advice.Local("otelScope") Scope scope) {
- if (!Java8BytecodeBridge.currentSpan().getSpanContext().isValid()) {
- // Don't want to generate a new top-level span
- return;
- }
-
- // Here we use "this" instead of "the method target" to distinguish abstract filter instances.
- span = tracer().startSpan(filter.getClass().getSimpleName() + ".doFilter", Kind.INTERNAL);
- scope = tracer().startScope(span);
- }
-
- @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
- public static void stopSpan(
- @Advice.Thrown Throwable throwable,
- @Advice.Local("otelSpan") Span span,
- @Advice.Local("otelScope") Scope scope) {
- if (scope == null) {
- return;
- }
- if (throwable != null) {
- tracer().endExceptionally(span, throwable);
- } else {
- tracer().end(span);
- }
- }
- }
-}

DELETE
instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/filter/FilterTracer.java
-/*
- * Copyright The OpenTelemetry Authors
- * SPDX-License-Identifier: Apache-2.0
- */
-
-package io.opentelemetry.javaagent.instrumentation.servlet.filter;
-
-import io.opentelemetry.instrumentation.api.tracer.BaseTracer;
-
-public class FilterTracer extends BaseTracer {
- private static final FilterTracer TRACER = new FilterTracer();
-
- public static FilterTracer tracer() {
- return TRACER;
- }
-
- @Override
- protected String getInstrumentationName() {
- return "io.opentelemetry.javaagent.servlet";
- }
-}

DELETE
instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/http/HttpServletInstrumentationModule.java
-/*
- * Copyright The OpenTelemetry Authors
- * SPDX-License-Identifier: Apache-2.0
- */
-
-package io.opentelemetry.javaagent.instrumentation.servlet.http;
-
-import static io.opentelemetry.javaagent.instrumentation.servlet.http.HttpServletTracer.tracer;
-import static io.opentelemetry.javaagent.tooling.ClassLoaderMatcher.hasClassesNamed;
-import static io.opentelemetry.javaagent.tooling.bytebuddy.matcher.AgentElementMatchers.extendsClass;
-import static java.util.Collections.singletonList;
-import static java.util.Collections.singletonMap;
-import static net.bytebuddy.matcher.ElementMatchers.isProtected;
-import static net.bytebuddy.matcher.ElementMatchers.isPublic;
-import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith;
-import static net.bytebuddy.matcher.ElementMatchers.named;
-import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
-
-import com.google.auto.service.AutoService;
-import io.opentelemetry.api.trace.Span;
-import io.opentelemetry.context.Scope;
-import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge;
-import io.opentelemetry.javaagent.tooling.InstrumentationModule;
-import io.opentelemetry.javaagent.tooling.TypeInstrumentation;
-import java.lang.reflect.Method;
-import java.util.List;
-import java.util.Map;
-import net.bytebuddy.asm.Advice;
-import net.bytebuddy.description.method.MethodDescription;
-import net.bytebuddy.description.type.TypeDescription;
-import net.bytebuddy.matcher.ElementMatcher;
-
-// Please read README.md of this subproject to understand what this instrumentation is and why it
-// requires a separate module.
-@AutoService(InstrumentationModule.class)
-public final class HttpServletInstrumentationModule extends InstrumentationModule {
- public HttpServletInstrumentationModule() {
- super("servlet-service");
- }
-
- @Override
- public boolean defaultEnabled() {
- return false;
- }
-
- @Override
- public String[] helperClassNames() {
- return new String[] {
- packageName + ".HttpServletTracer",
- };
- }
-
- @Override
- public List<TypeInstrumentation> typeInstrumentations() {
- return singletonList(new HttpServletInstrumentation());
- }
-
- private static final class HttpServletInstrumentation implements TypeInstrumentation {
- @Override
- public ElementMatcher<ClassLoader> classLoaderMatcher() {
- // Optimization for expensive typeMatcher.
- return hasClassesNamed("javax.servlet.http.HttpServlet");
- }
-
- @Override
- public ElementMatcher<TypeDescription> typeMatcher() {
- return extendsClass(named("javax.servlet.http.HttpServlet"));
- }
-
- /**
- * Here we are instrumenting the protected method for HttpServlet. This should ensure that this
- * advice is always called after Servlet3Instrumentation which is instrumenting the public
- * method.
- */
- @Override
- public Map<? extends ElementMatcher<? super MethodDescription>, String> transformers() {
- return singletonMap(
- named("service")
- .or(nameStartsWith("do")) // doGet, doPost, etc
- .and(takesArgument(0, named("javax.servlet.http.HttpServletRequest")))
- .and(takesArgument(1, named("javax.servlet.http.HttpServletResponse")))
- .and(isProtected().or(isPublic())),
- HttpServletInstrumentationModule.class.getName() + "$HttpServletAdvice");
- }
- }
-
- public static class HttpServletAdvice {
-
- @Advice.OnMethodEnter(suppress = Throwable.class)
- public static void start(
- @Advice.Origin Method method,
- @Advice.Local("otelSpan") Span span,
- @Advice.Local("otelScope") Scope scope) {
-
- if (!Java8BytecodeBridge.currentSpan().getSpanContext().isValid()) {
- // Don't want to generate a new top-level span
- return;
- }
-
- span = tracer().startSpan(method);
- scope = tracer().startScope(span);
- }
-
- @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
- public static void stopSpan(
- @Advice.Thrown Throwable throwable,
- @Advice.Local("otelSpan") Span span,
- @Advice.Local("otelScope") Scope scope) {
- if (scope == null) {
- return;
- }
- scope.close();
-
- if (throwable != null) {
- tracer().endExceptionally(span, throwable);
- } else {
- tracer().end(span);
- }
- }
- }
-}

DELETE
instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/http/HttpServletTracer.java
-/*
- * Copyright The OpenTelemetry Authors
- * SPDX-License-Identifier: Apache-2.0
- */
-
-package io.opentelemetry.javaagent.instrumentation.servlet.http;
-
-import io.opentelemetry.instrumentation.api.tracer.BaseTracer;
-
-public class HttpServletTracer extends BaseTracer {
- private static final HttpServletTracer TRACER = new HttpServletTracer();
-
- public static HttpServletTracer tracer() {
- return TRACER;
- }
-
- @Override
- protected String getInstrumentationName() {
- return "io.opentelemetry.javaagent.servlet";
- }
-}

DELETE
instrumentation/servlet/servlet-common/src/test/groovy/FilterTest.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.instrumentation.test.AgentTestRunner
-import io.opentelemetry.instrumentation.test.utils.ConfigUtils
-import javax.servlet.Filter
-import javax.servlet.FilterChain
-import javax.servlet.FilterConfig
-import javax.servlet.ServletException
-import javax.servlet.ServletRequest
-import javax.servlet.ServletResponse
-
-class FilterTest extends AgentTestRunner {
- static final PREVIOUS_CONFIG = ConfigUtils.updateConfigAndResetInstrumentation {
- it.setProperty("otel.instrumentation.servlet-filter.enabled", "true")
- }
-
- def cleanupSpec() {
- ConfigUtils.setConfig(PREVIOUS_CONFIG)
- }
-
- def "test doFilter no-parent"() {
- when:
- filter.doFilter(null, null, null)
-
- then:
- assertTraces(0) {}
-
- where:
- filter = new TestFilter()
- }
-
- def "test doFilter with parent"() {
- when:
- runUnderTrace("parent") {
- filter.doFilter(null, null, null)
- }
-
- then:
- assertTraces(1) {
- trace(0, 2) {
- basicSpan(it, 0, "parent")
- span(1) {
- name "${filter.class.simpleName}.doFilter"
- childOf span(0)
- attributes {
- }
- }
- }
- }
-
- where:
- filter << [new TestFilter(), new TestFilter() {}]
- }
-
- def "test doFilter exception"() {
- setup:
- def ex = new Exception("some error")
- def filter = new TestFilter() {
- @Override
- void doFilter(ServletRequest servletRequest, ServletResponse servletResponse, FilterChain filterChain) {
- throw ex
- }
- }
-
- when:
- runUnderTrace("parent") {
- filter.doFilter(null, null, null)
- }
-
- then:
- def th = thrown(Exception)
- th == ex
-
- assertTraces(1) {
- trace(0, 2) {
- basicSpan(it, 0, "parent", null, ex)
- span(1) {
- name "${filter.class.simpleName}.doFilter"
- childOf span(0)
- errored true
- errorEvent(ex.class, ex.message)
- }
- }
- }
- }
-
- static class TestFilter implements Filter {
-
- @Override
- void init(FilterConfig filterConfig) throws ServletException {
- }
-
- @Override
- void doFilter(ServletRequest servletRequest, ServletResponse servletResponse, FilterChain filterChain) throws IOException, ServletException {
- }
-
- @Override
- void destroy() {
- }
- }
-}

DELETE
instrumentation/servlet/servlet-common/src/test/groovy/HttpServletTest.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 groovy.servlet.AbstractHttpServlet
-import io.opentelemetry.instrumentation.test.AgentTestRunner
-import io.opentelemetry.instrumentation.test.utils.ConfigUtils
-import javax.servlet.http.HttpServletRequest
-import javax.servlet.http.HttpServletResponse
-
-class HttpServletTest extends AgentTestRunner {
- static final PREVIOUS_CONFIG = ConfigUtils.updateConfigAndResetInstrumentation {
- it.setProperty("otel.instrumentation.servlet-service.enabled", "true")
- }
-
- def specCleanup() {
- ConfigUtils.setConfig(PREVIOUS_CONFIG)
- }
-
- def req = Mock(HttpServletRequest) {
- getMethod() >> "GET"
- getProtocol() >> "TEST"
- }
- def resp = Mock(HttpServletResponse)
-
- def "test service no-parent"() {
- when:
- servlet.service(req, resp)
-
- then:
- assertTraces(0) {}
-
- where:
- servlet = new TestServlet()
- }
-
- def "test service with parent"() {
- when:
- runUnderTrace("parent") {
- servlet.service(req, resp)
- }
-
- then:
- assertTraces(1) {
- trace(0, 3) {
- basicSpan(it, 0, "parent")
- span(1) {
- name "HttpServlet.service"
- childOf span(0)
- attributes {
- }
- }
- span(2) {
- name "${expectedSpanName}.doGet"
- childOf span(1)
- attributes {
- }
- }
- }
- }
-
- where:
- servlet << [new TestServlet(), new TestServlet() {
- @Override
- protected void doGet(HttpServletRequest req, HttpServletResponse resp) {
- }
- }]
-
- expectedSpanName = servlet.class.anonymousClass ? servlet.class.name : servlet.class.simpleName
- }
-
- def "test service exception"() {
- setup:
- def ex = new Exception("some error")
- def servlet = new TestServlet() {
- @Override
- protected void doGet(HttpServletRequest req, HttpServletResponse resp) {
- throw ex
- }
- }
-
- when:
- runUnderTrace("parent") {
- servlet.service(req, resp)
- }
-
- then:
- def th = thrown(Exception)
- th == ex
-
- assertTraces(1) {
- trace(0, 3) {
- basicSpan(it, 0, "parent", null, ex)
- span(1) {
- name "HttpServlet.service"
- childOf span(0)
- errored true
- errorEvent(ex.class, ex.message)
- }
- span(2) {
- name "${servlet.class.name}.doGet"
- childOf span(1)
- errored true
- errorEvent(ex.class, ex.message)
- }
- }
- }
- }
-
- static class TestServlet extends AbstractHttpServlet {
- @Override
- protected void doGet(HttpServletRequest req, HttpServletResponse resp) {
- }
- }
-}

Remove disabled by default servlet instrumentation (#1684)