open-telemetry
/
opentelemetry-java-instrumentation
mirror of https://github.com/open-telemetry/opentelemetry-java-instrumentation.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Remove disabled by default servlet instrumentation (#1684)

This commit is contained in:
Trask Stalnaker 2020-11-19 11:22:02 -08:00 committed by GitHub
parent d183692699
commit e092e87d6b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 9 additions and 541 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
README.md
View File

@ -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:

33
instrumentation/servlet/README.md
View File

@ -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`.

5
instrumentation/servlet/servlet-2.2/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v2_2/ServletAndFilterInstrumentation.java
View File

@ -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(

7
instrumentation/servlet/servlet-3.0/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/ServletAndFilterInstrumentation.java
View File

@ -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(

115
instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/filter/FilterInstrumentationModule.java
View File

@ -1,115 +0,0 @@
/*
* 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);
}
}
}
}

21
instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/filter/FilterTracer.java
View File

@ -1,21 +0,0 @@
/*
* 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";
}
}

121
instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/http/HttpServletInstrumentationModule.java
View File

@ -1,121 +0,0 @@
/*
* 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);
}
}
}
}

21
instrumentation/servlet/servlet-common/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/http/HttpServletTracer.java
View File

@ -1,21 +0,0 @@
/*
* 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";
}
}

107
instrumentation/servlet/servlet-common/src/test/groovy/FilterTest.groovy
View File

@ -1,107 +0,0 @@
/*
* 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() {
}
}
}

119
instrumentation/servlet/servlet-common/src/test/groovy/HttpServletTest.groovy
View File

@ -1,119 +0,0 @@
/*
* 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) {
}
}
}