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

Add HttpServerResponseCustomizer support for Servlet and Jetty (#8095) 

Add `HttpServerResponseCustomizer` support for Servlet 2.2/3.0/5.0 and
Jetty 8/11 instrumentations. Enabled testing for it in JaxRs tests as
well since those should now all be covered due to servlet
instrumentations. Fixed Jetty 11 test source set directory name.

Known limitation - response headers do not work on Jetty 8 for internal
exception pages caused by throwing an exception that is handled outside
of instrumentation scope, working around this would require an
additional instrumentation and/or keeping an expired `Context` instance
referenced by the response object. This does not appear to be an issue
on Jetty 11. Additionally, calling `ServletResponse#reset` can wipe
headers as well, for which there is no workaround (yet?) in this PR.
This commit is contained in:
Ago Allikmaa 2023-03-23 13:01:07 +02:00 committed by GitHub
parent 079e0faa2d
commit e466dc439a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 110 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
instrumentation/jaxrs/jaxrs-common/testing/src/main/groovy/AbstractJaxRsHttpServerTest.groovy
View File

@ -219,6 +219,11 @@ abstract class AbstractJaxRsHttpServerTest<S> extends HttpServerTest<S> implemen
Boolean.getBoolean("testLatestDeps")
}
@Override
boolean hasResponseCustomizer(ServerEndpoint endpoint) {
true
}
@Override
void serverSpan(TraceAssert trace,
int index,

4
instrumentation/jetty/jetty-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v11_0/Jetty11HandlerAdvice.java
View File

@ -10,6 +10,7 @@ import static io.opentelemetry.javaagent.instrumentation.jetty.v11_0.Jetty11Sing
import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge;
import io.opentelemetry.javaagent.bootstrap.http.HttpServerResponseCustomizerHolder;
import io.opentelemetry.javaagent.instrumentation.servlet.ServletRequestContext;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
@ -45,6 +46,9 @@ public class Jetty11HandlerAdvice {
// Must be set here since Jetty handlers can use startAsync outside of servlet scope.
helper().setAsyncListenerResponse(request, response);
HttpServerResponseCustomizerHolder.getCustomizer()
.customize(context, response, Jetty11ResponseMutator.INSTANCE);
}
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)

18
instrumentation/jetty/jetty-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v11_0/Jetty11ResponseMutator.java Normal file
View File

@ -0,0 +1,18 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.javaagent.instrumentation.jetty.v11_0;
import io.opentelemetry.javaagent.bootstrap.http.HttpServerResponseMutator;
import jakarta.servlet.http.HttpServletResponse;
public enum Jetty11ResponseMutator implements HttpServerResponseMutator<HttpServletResponse> {
INSTANCE;
@Override
public void appendHeader(HttpServletResponse response, String name, String value) {
response.addHeader(name, value);
}
}

1
instrumentation/jetty/jetty-11.0/javaagent/src/test/Java/io/opentelemetry/javaagent/instrumentation/jetty/v11_0/JettyHandlerTest.java → instrumentation/jetty/jetty-11.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jetty/v11_0/JettyHandlerTest.java
View File

@ -88,6 +88,7 @@ public class JettyHandlerTest extends AbstractHttpServerTest<Server> {
DEFAULT_HTTP_ATTRIBUTES, Collections.singleton(SemanticAttributes.HTTP_ROUTE)));
options.setHasResponseSpan(endpoint -> endpoint == REDIRECT || endpoint == ERROR);
options.setExpectedException(new IllegalStateException(EXCEPTION.getBody()));
options.setHasResponseCustomizer(endpoint -> true);
}
@Override

4
instrumentation/jetty/jetty-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v8_0/Jetty8HandlerAdvice.java
View File

@ -10,6 +10,7 @@ import static io.opentelemetry.javaagent.instrumentation.jetty.v8_0.Jetty8Single
import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge;
import io.opentelemetry.javaagent.bootstrap.http.HttpServerResponseCustomizerHolder;
import io.opentelemetry.javaagent.instrumentation.servlet.ServletRequestContext;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
@ -45,6 +46,9 @@ public class Jetty8HandlerAdvice {
// Must be set here since Jetty handlers can use startAsync outside of servlet scope.
helper().setAsyncListenerResponse(request, response);
HttpServerResponseCustomizerHolder.getCustomizer()
.customize(context, response, Jetty8ResponseMutator.INSTANCE);
}
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)

18
instrumentation/jetty/jetty-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v8_0/Jetty8ResponseMutator.java Normal file
View File

@ -0,0 +1,18 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.javaagent.instrumentation.jetty.v8_0;
import io.opentelemetry.javaagent.bootstrap.http.HttpServerResponseMutator;
import javax.servlet.http.HttpServletResponse;
public enum Jetty8ResponseMutator implements HttpServerResponseMutator<HttpServletResponse> {
INSTANCE;
@Override
public void appendHeader(HttpServletResponse response, String name, String value) {
response.addHeader(name, value);
}
}

1
instrumentation/jetty/jetty-8.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jetty/v8_0/JettyHandlerTest.java
View File

@ -88,6 +88,7 @@ public class JettyHandlerTest extends AbstractHttpServerTest<Server> {
DEFAULT_HTTP_ATTRIBUTES, Collections.singleton(SemanticAttributes.HTTP_ROUTE)));
options.setHasResponseSpan(endpoint -> endpoint == REDIRECT || endpoint == ERROR);
options.setExpectedException(new IllegalStateException(EXCEPTION.getBody()));
options.setHasResponseCustomizer(endpoint -> endpoint != EXCEPTION);
}
@Override

9
instrumentation/servlet/servlet-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v2_2/Servlet2Accessor.java
View File

@ -5,6 +5,7 @@
package io.opentelemetry.javaagent.instrumentation.servlet.v2_2;
import io.opentelemetry.javaagent.bootstrap.http.HttpServerResponseMutator;
import io.opentelemetry.javaagent.instrumentation.servlet.ServletAsyncListener;
import io.opentelemetry.javaagent.instrumentation.servlet.javax.JavaxServletAccessor;
import java.util.Collections;
@ -12,7 +13,8 @@ import java.util.List;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
public class Servlet2Accessor extends JavaxServletAccessor<HttpServletResponse> {
public class Servlet2Accessor extends JavaxServletAccessor<HttpServletResponse>
implements HttpServerResponseMutator<HttpServletResponse> {
public static final Servlet2Accessor INSTANCE = new Servlet2Accessor();
private Servlet2Accessor() {}
@ -55,4 +57,9 @@ public class Servlet2Accessor extends JavaxServletAccessor<HttpServletResponse>
public boolean isResponseCommitted(HttpServletResponse httpServletResponse) {
return httpServletResponse.isCommitted();
}
@Override
public void appendHeader(HttpServletResponse response, String name, String value) {
response.addHeader(name, value);
}
}

4
instrumentation/servlet/servlet-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v2_2/Servlet2Advice.java
View File

@ -12,6 +12,7 @@ import io.opentelemetry.context.Scope;
import io.opentelemetry.instrumentation.api.util.VirtualField;
import io.opentelemetry.javaagent.bootstrap.CallDepth;
import io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge;
import io.opentelemetry.javaagent.bootstrap.http.HttpServerResponseCustomizerHolder;
import io.opentelemetry.javaagent.bootstrap.servlet.AppServerBridge;
import io.opentelemetry.javaagent.instrumentation.servlet.ServletRequestContext;
import javax.servlet.ServletRequest;
@ -64,6 +65,9 @@ public class Servlet2Advice {
// reset response status from previous request
// (some servlet containers reuse response objects to reduce memory allocations)
VirtualField.find(ServletResponse.class, Integer.class).set(response, null);
HttpServerResponseCustomizerHolder.getCustomizer()
.customize(context, (HttpServletResponse) response, Servlet2Accessor.INSTANCE);
}
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)

5
instrumentation/servlet/servlet-2.2/javaagent/src/test/groovy/JettyServlet2Test.groovy
View File

@ -111,6 +111,11 @@ class JettyServlet2Test extends HttpServerTest<Server> implements AgentTestTrait
endpoint == REDIRECT || endpoint == ERROR
}
@Override
boolean hasResponseCustomizer(ServerEndpoint endpoint) {
true
}
@Override
void responseSpan(TraceAssert trace, int index, Object parent, String method = "GET", ServerEndpoint endpoint = SUCCESS) {
def responseMethod = endpoint == REDIRECT ? "sendRedirect" : "sendError"

9
instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3Accessor.java
View File

@ -5,6 +5,7 @@
package io.opentelemetry.javaagent.instrumentation.servlet.v3_0;
import io.opentelemetry.javaagent.bootstrap.http.HttpServerResponseMutator;
import io.opentelemetry.javaagent.instrumentation.servlet.ServletAsyncListener;
import io.opentelemetry.javaagent.instrumentation.servlet.javax.JavaxServletAccessor;
import java.util.ArrayList;
@ -16,7 +17,8 @@ import javax.servlet.AsyncListener;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
public class Servlet3Accessor extends JavaxServletAccessor<HttpServletResponse> {
public class Servlet3Accessor extends JavaxServletAccessor<HttpServletResponse>
implements HttpServerResponseMutator<HttpServletResponse> {
public static final Servlet3Accessor INSTANCE = new Servlet3Accessor();
private Servlet3Accessor() {}
@ -70,6 +72,11 @@ public class Servlet3Accessor extends JavaxServletAccessor<HttpServletResponse>
return response.isCommitted();
}
@Override
public void appendHeader(HttpServletResponse response, String name, String value) {
response.addHeader(name, value);
}
private static class Listener implements AsyncListener {
private final ServletAsyncListener<HttpServletResponse> listener;

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

@ -11,6 +11,7 @@ import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import io.opentelemetry.javaagent.bootstrap.CallDepth;
import io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge;
import io.opentelemetry.javaagent.bootstrap.http.HttpServerResponseCustomizerHolder;
import io.opentelemetry.javaagent.bootstrap.servlet.AppServerBridge;
import io.opentelemetry.javaagent.bootstrap.servlet.MappingResolver;
import io.opentelemetry.javaagent.instrumentation.servlet.ServletRequestContext;
@ -73,6 +74,12 @@ public class Servlet3Advice {
contextToUpdate =
helper().updateContext(contextToUpdate, httpServletRequest, mappingResolver, servlet);
scope = contextToUpdate.makeCurrent();
if (context != null) {
// Only trigger response customizer once, so only if server span was created here
HttpServerResponseCustomizerHolder.getCustomizer()
.customize(contextToUpdate, (HttpServletResponse) response, Servlet3Accessor.INSTANCE);
}
}
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)

5
instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/AbstractServlet3Test.groovy
View File

@ -79,6 +79,11 @@ abstract class AbstractServlet3Test<SERVER, CONTEXT> extends HttpServerTest<SERV
true
}
@Override
boolean hasResponseCustomizer(ServerEndpoint endpoint) {
true
}
@Override
boolean hasResponseSpan(ServerEndpoint endpoint) {
endpoint == REDIRECT || (endpoint == ERROR && errorEndpointUsesSendError())

10
instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/Servlet5Accessor.java
View File

@ -5,6 +5,7 @@
package io.opentelemetry.javaagent.instrumentation.servlet.v5_0;
import io.opentelemetry.javaagent.bootstrap.http.HttpServerResponseMutator;
import io.opentelemetry.javaagent.instrumentation.servlet.ServletAccessor;
import io.opentelemetry.javaagent.instrumentation.servlet.ServletAsyncListener;
import jakarta.servlet.AsyncEvent;
@ -20,7 +21,9 @@ import java.util.Collections;
import java.util.Enumeration;
import java.util.List;
public class Servlet5Accessor implements ServletAccessor<HttpServletRequest, HttpServletResponse> {
public class Servlet5Accessor
implements ServletAccessor<HttpServletRequest, HttpServletResponse>,
HttpServerResponseMutator<HttpServletResponse> {
public static final Servlet5Accessor INSTANCE = new Servlet5Accessor();
private Servlet5Accessor() {}
@ -172,6 +175,11 @@ public class Servlet5Accessor implements ServletAccessor<HttpServletRequest, Htt
return throwable instanceof ServletException;
}
@Override
public void appendHeader(HttpServletResponse response, String name, String value) {
response.addHeader(name, value);
}
private static class Listener implements AsyncListener {
private final ServletAsyncListener<HttpServletResponse> listener;

8
instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/service/JakartaServletServiceAdvice.java
View File

@ -11,9 +11,11 @@ import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import io.opentelemetry.javaagent.bootstrap.CallDepth;
import io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge;
import io.opentelemetry.javaagent.bootstrap.http.HttpServerResponseCustomizerHolder;
import io.opentelemetry.javaagent.bootstrap.servlet.AppServerBridge;
import io.opentelemetry.javaagent.bootstrap.servlet.MappingResolver;
import io.opentelemetry.javaagent.instrumentation.servlet.ServletRequestContext;
import io.opentelemetry.javaagent.instrumentation.servlet.v5_0.Servlet5Accessor;
import io.opentelemetry.javaagent.instrumentation.servlet.v5_0.Servlet5Singletons;
import jakarta.servlet.Servlet;
import jakarta.servlet.ServletRequest;
@ -74,6 +76,12 @@ public class JakartaServletServiceAdvice {
contextToUpdate =
helper().updateContext(contextToUpdate, httpServletRequest, mappingResolver, servlet);
scope = contextToUpdate.makeCurrent();
if (context != null) {
// Only trigger response customizer once, so only if server span was created here
HttpServerResponseCustomizerHolder.getCustomizer()
.customize(contextToUpdate, (HttpServletResponse) response, Servlet5Accessor.INSTANCE);
}
}
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)

5
instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/AbstractServlet5Test.groovy
View File

@ -78,6 +78,11 @@ abstract class AbstractServlet5Test<SERVER, CONTEXT> extends HttpServerTest<SERV
}
}
@Override
boolean hasResponseCustomizer(ServerEndpoint endpoint) {
true
}
@Override
boolean hasResponseSpan(ServerEndpoint endpoint) {
endpoint == REDIRECT || (endpoint == ERROR && errorEndpointUsesSendError())