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

Merge pull request #1387 from DataDog/devinsba/dont-change-servletresponse-type 

Rework servlet 2 to not wrap HttpServletResponse objects
This commit is contained in:
Brian Devins-Suresh 2020-04-21 14:39:48 -04:00 committed by GitHub
parent 55030ab74a aec2fad9db
commit f908859442
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 104 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
dd-java-agent/instrumentation/servlet/request-2/request-2.gradle
View File

@ -2,10 +2,10 @@ muzzle {
pass {
group = "javax.servlet"
module = "servlet-api"
versions = "[2.3, 3.0)"
versions = "[2.2, 3.0)"
assertInverse = true
}
fail {
group = "javax.servlet"
module = 'javax.servlet-api'
@ -24,7 +24,7 @@ testSets {
}
dependencies {
compileOnly group: 'javax.servlet', name: 'servlet-api', version: '2.3'
compileOnly group: 'javax.servlet', name: 'servlet-api', version: '2.2'
testCompile(project(':dd-java-agent:testing')) {
exclude group: 'org.eclipse.jetty', module: 'jetty-server'

20
dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Advice.java
View File

@ -28,8 +28,8 @@ public class Servlet2Advice {
public static AgentScope onEnter(
@Advice.This final Object servlet,
@Advice.Argument(0) final ServletRequest request,
@Advice.Argument(value = 1, readOnly = false, typing = Assigner.Typing.DYNAMIC)
ServletResponse response) {
@Advice.Argument(value = 1, typing = Assigner.Typing.DYNAMIC)
final ServletResponse response) {
final boolean hasServletTrace = request.getAttribute(DD_SPAN_ATTRIBUTE) instanceof AgentSpan;
final boolean invalidRequest = !(request instanceof HttpServletRequest);
@ -45,7 +45,8 @@ public class Servlet2Advice {
InstrumentationContext.get(HttpServletResponse.class, HttpServletRequest.class)
.put((HttpServletResponse) response, httpServletRequest);
response = new StatusSavingHttpServletResponseWrapper((HttpServletResponse) response);
// Default value for checking for uncaught error later
InstrumentationContext.get(ServletResponse.class, Integer.class).put(response, 200);
}
final AgentSpan.Context extractedContext = propagate().extract(httpServletRequest, GETTER);
@ -89,10 +90,17 @@ public class Servlet2Advice {
return;
}
final AgentSpan span = scope.span();
DECORATE.onResponse(span, response);
if (response instanceof HttpServletResponse) {
DECORATE.onResponse(
span, InstrumentationContext.get(ServletResponse.class, Integer.class).get(response));
} else {
DECORATE.onResponse(span, null);
}
if (throwable != null) {
if (response instanceof StatusSavingHttpServletResponseWrapper
&& ((StatusSavingHttpServletResponseWrapper) response).status
if (response instanceof HttpServletResponse
&& InstrumentationContext.get(ServletResponse.class, Integer.class).get(response)
== HttpServletResponse.SC_OK) {
// exception was thrown but status code wasn't set
span.setTag(Tags.HTTP_STATUS, 500);

12
dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Decorator.java
View File

@ -4,11 +4,10 @@ import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import datadog.trace.bootstrap.instrumentation.decorator.HttpServerDecorator;
import java.net.URI;
import java.net.URISyntaxException;
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
public class Servlet2Decorator
extends HttpServerDecorator<HttpServletRequest, HttpServletRequest, ServletResponse> {
extends HttpServerDecorator<HttpServletRequest, HttpServletRequest, Integer> {
public static final Servlet2Decorator DECORATE = new Servlet2Decorator();
@Override
@ -50,13 +49,8 @@ public class Servlet2Decorator
}
@Override
protected Integer status(final ServletResponse httpServletResponse) {
if (httpServletResponse instanceof StatusSavingHttpServletResponseWrapper) {
return ((StatusSavingHttpServletResponseWrapper) httpServletResponse).status;
} else {
// HttpServletResponse doesn't have accessor for status code.
return null;
}
protected Integer status(final Integer status) {
return status;
}
@Override

10
dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2Instrumentation.java
View File

@ -10,6 +10,7 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
import com.google.auto.service.AutoService;
import datadog.trace.agent.tooling.Instrumenter;
import java.util.HashMap;
import java.util.Map;
import net.bytebuddy.description.method.MethodDescription;
import net.bytebuddy.description.type.TypeDescription;
@ -37,16 +38,17 @@ public final class Servlet2Instrumentation extends Instrumenter.Default {
@Override
public String[] helperClassNames() {
return new String[] {
packageName + ".Servlet2Decorator",
packageName + ".HttpServletRequestExtractAdapter",
packageName + ".StatusSavingHttpServletResponseWrapper",
packageName + ".Servlet2Decorator", packageName + ".HttpServletRequestExtractAdapter",
};
}
@Override
public Map<String, String> contextStore() {
return singletonMap(
final Map<String, String> contextStores = new HashMap<>();
contextStores.put(
"javax.servlet.http.HttpServletResponse", "javax.servlet.http.HttpServletRequest");
contextStores.put("javax.servlet.ServletResponse", Integer.class.getName());
return contextStores;
}
/**

13
dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2ResponseRedirectAdvice.java Normal file
View File

@ -0,0 +1,13 @@
package datadog.trace.instrumentation.servlet2;
import datadog.trace.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);
}
}

14
dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2ResponseStatusAdvice.java Normal file
View File

@ -0,0 +1,14 @@
package datadog.trace.instrumentation.servlet2;
import datadog.trace.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);
}
}

51
dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/Servlet2ResponseStatusInstrumentation.java Normal file
View File

@ -0,0 +1,51 @@
package datadog.trace.instrumentation.servlet2;
import static datadog.trace.agent.tooling.ClassLoaderMatcher.hasClassesNamed;
import static datadog.trace.agent.tooling.bytebuddy.matcher.DDElementMatchers.safeHasSuperType;
import static java.util.Collections.singletonMap;
import static net.bytebuddy.matcher.ElementMatchers.named;
import static net.bytebuddy.matcher.ElementMatchers.not;
import com.google.auto.service.AutoService;
import datadog.trace.agent.tooling.Instrumenter;
import java.util.HashMap;
import java.util.Map;
import net.bytebuddy.description.method.MethodDescription;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.matcher.ElementMatcher;
@AutoService(Instrumenter.class)
public final class Servlet2ResponseStatusInstrumentation extends Instrumenter.Default {
public Servlet2ResponseStatusInstrumentation() {
super("servlet", "servlet-2");
}
// this is required to make sure servlet 2 instrumentation won't apply to servlet 3
@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
return not(hasClassesNamed("javax.servlet.AsyncEvent", "javax.servlet.AsyncListener"));
}
@Override
public ElementMatcher<? super TypeDescription> typeMatcher() {
return safeHasSuperType(named("javax.servlet.http.HttpServletResponse"));
}
@Override
public Map<String, String> contextStore() {
return singletonMap("javax.servlet.ServletResponse", Integer.class.getName());
}
/**
* Unlike Servlet2Instrumentation it doesn't matter if the HttpServletResponseInstrumentation
* applies first
*/
@Override
public Map<? extends ElementMatcher<? super MethodDescription>, String> transformers() {
final Map<ElementMatcher<? super MethodDescription>, String> transformers = new HashMap<>();
transformers.put(
named("sendError").or(named("setStatus")), packageName + ".Servlet2ResponseStatusAdvice");
transformers.put(named("sendRedirect"), packageName + ".Servlet2ResponseRedirectAdvice");
return transformers;
}
}

43
dd-java-agent/instrumentation/servlet/request-2/src/main/java/datadog/trace/instrumentation/servlet2/StatusSavingHttpServletResponseWrapper.java
View File

@ -1,43 +0,0 @@
package datadog.trace.instrumentation.servlet2;
import java.io.IOException;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpServletResponseWrapper;
public class StatusSavingHttpServletResponseWrapper extends HttpServletResponseWrapper {
public int status = 200;
public StatusSavingHttpServletResponseWrapper(final HttpServletResponse response) {
super(response);
}
@Override
public void sendError(final int status) throws IOException {
this.status = status;
super.sendError(status);
}
@Override
public void sendError(final int status, final String message) throws IOException {
this.status = status;
super.sendError(status, message);
}
@Override
public void sendRedirect(final String location) throws IOException {
status = 302;
super.sendRedirect(location);
}
@Override
public void setStatus(final int status) {
this.status = status;
super.setStatus(status);
}
@Override
public void setStatus(final int status, final String message) {
this.status = status;
super.setStatus(status, message);
}
}