From 61e7b53bb8e8d1016e5b459c74fc042f7bdf9d4d Mon Sep 17 00:00:00 2001 From: Ago Allikmaa Date: Tue, 21 Mar 2023 05:22:47 +0200 Subject: [PATCH] Add response customizer support for Servlet and Jetty --- .../groovy/AbstractJaxRsHttpServerTest.groovy | 5 +++++ .../jetty/v11_0/Jetty11HandlerAdvice.java | 4 ++++ .../jetty/v11_0/Jetty11ResponseMutator.java | 18 ++++++++++++++++++ .../jetty/v11_0/JettyHandlerTest.java | 1 + .../jetty/v8_0/Jetty8HandlerAdvice.java | 4 ++++ .../jetty/v8_0/Jetty8ResponseMutator.java | 18 ++++++++++++++++++ .../jetty/v8_0/JettyHandlerTest.java | 1 + .../servlet/v2_2/Servlet2Accessor.java | 9 ++++++++- .../servlet/v2_2/Servlet2Advice.java | 4 ++++ .../src/test/groovy/JettyServlet2Test.groovy | 5 +++++ .../servlet/v3_0/Servlet3Accessor.java | 9 ++++++++- .../servlet/v3_0/Servlet3Advice.java | 7 +++++++ .../test/groovy/AbstractServlet3Test.groovy | 5 +++++ .../servlet/v5_0/Servlet5Accessor.java | 10 +++++++++- .../service/JakartaServletServiceAdvice.java | 8 ++++++++ .../test/groovy/AbstractServlet5Test.groovy | 5 +++++ 16 files changed, 110 insertions(+), 3 deletions(-) create mode 100644 instrumentation/jetty/jetty-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v11_0/Jetty11ResponseMutator.java rename instrumentation/jetty/jetty-11.0/javaagent/src/test/{Java => java}/io/opentelemetry/javaagent/instrumentation/jetty/v11_0/JettyHandlerTest.java (99%) create mode 100644 instrumentation/jetty/jetty-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v8_0/Jetty8ResponseMutator.java diff --git a/instrumentation/jaxrs/jaxrs-common/testing/src/main/groovy/AbstractJaxRsHttpServerTest.groovy b/instrumentation/jaxrs/jaxrs-common/testing/src/main/groovy/AbstractJaxRsHttpServerTest.groovy index ab0dfe5849d4..59a8f36aebc8 100644 --- a/instrumentation/jaxrs/jaxrs-common/testing/src/main/groovy/AbstractJaxRsHttpServerTest.groovy +++ b/instrumentation/jaxrs/jaxrs-common/testing/src/main/groovy/AbstractJaxRsHttpServerTest.groovy @@ -219,6 +219,11 @@ abstract class AbstractJaxRsHttpServerTest extends HttpServerTest implemen Boolean.getBoolean("testLatestDeps") } + @Override + boolean hasResponseCustomizer(ServerEndpoint endpoint) { + true + } + @Override void serverSpan(TraceAssert trace, int index, diff --git a/instrumentation/jetty/jetty-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v11_0/Jetty11HandlerAdvice.java b/instrumentation/jetty/jetty-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v11_0/Jetty11HandlerAdvice.java index b55ffda470e0..60a368077314 100644 --- a/instrumentation/jetty/jetty-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v11_0/Jetty11HandlerAdvice.java +++ b/instrumentation/jetty/jetty-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v11_0/Jetty11HandlerAdvice.java @@ -10,6 +10,7 @@ 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 static void onEnter( // 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) diff --git a/instrumentation/jetty/jetty-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v11_0/Jetty11ResponseMutator.java b/instrumentation/jetty/jetty-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v11_0/Jetty11ResponseMutator.java new file mode 100644 index 000000000000..1b2cab6d5fe0 --- /dev/null +++ b/instrumentation/jetty/jetty-11.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v11_0/Jetty11ResponseMutator.java @@ -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 { + INSTANCE; + + @Override + public void appendHeader(HttpServletResponse response, String name, String value) { + response.addHeader(name, value); + } +} diff --git a/instrumentation/jetty/jetty-11.0/javaagent/src/test/Java/io/opentelemetry/javaagent/instrumentation/jetty/v11_0/JettyHandlerTest.java b/instrumentation/jetty/jetty-11.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jetty/v11_0/JettyHandlerTest.java similarity index 99% rename from instrumentation/jetty/jetty-11.0/javaagent/src/test/Java/io/opentelemetry/javaagent/instrumentation/jetty/v11_0/JettyHandlerTest.java rename to instrumentation/jetty/jetty-11.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jetty/v11_0/JettyHandlerTest.java index 4ec07e39f3a5..2c995bd6af35 100644 --- a/instrumentation/jetty/jetty-11.0/javaagent/src/test/Java/io/opentelemetry/javaagent/instrumentation/jetty/v11_0/JettyHandlerTest.java +++ b/instrumentation/jetty/jetty-11.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jetty/v11_0/JettyHandlerTest.java @@ -88,6 +88,7 @@ protected void configure(HttpServerTestOptions options) { 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 diff --git a/instrumentation/jetty/jetty-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v8_0/Jetty8HandlerAdvice.java b/instrumentation/jetty/jetty-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v8_0/Jetty8HandlerAdvice.java index 9bd6f311f15b..a72e957f26c8 100644 --- a/instrumentation/jetty/jetty-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v8_0/Jetty8HandlerAdvice.java +++ b/instrumentation/jetty/jetty-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v8_0/Jetty8HandlerAdvice.java @@ -10,6 +10,7 @@ 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 static void onEnter( // 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) diff --git a/instrumentation/jetty/jetty-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v8_0/Jetty8ResponseMutator.java b/instrumentation/jetty/jetty-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v8_0/Jetty8ResponseMutator.java new file mode 100644 index 000000000000..31b8423c13d6 --- /dev/null +++ b/instrumentation/jetty/jetty-8.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/v8_0/Jetty8ResponseMutator.java @@ -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 { + INSTANCE; + + @Override + public void appendHeader(HttpServletResponse response, String name, String value) { + response.addHeader(name, value); + } +} diff --git a/instrumentation/jetty/jetty-8.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jetty/v8_0/JettyHandlerTest.java b/instrumentation/jetty/jetty-8.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jetty/v8_0/JettyHandlerTest.java index 00f3986c0706..3c6ba7ad8ea0 100644 --- a/instrumentation/jetty/jetty-8.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jetty/v8_0/JettyHandlerTest.java +++ b/instrumentation/jetty/jetty-8.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jetty/v8_0/JettyHandlerTest.java @@ -88,6 +88,7 @@ protected void configure(HttpServerTestOptions options) { 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 diff --git a/instrumentation/servlet/servlet-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v2_2/Servlet2Accessor.java b/instrumentation/servlet/servlet-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v2_2/Servlet2Accessor.java index 91e1c278e7f1..2003822fb7e9 100644 --- a/instrumentation/servlet/servlet-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v2_2/Servlet2Accessor.java +++ b/instrumentation/servlet/servlet-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v2_2/Servlet2Accessor.java @@ -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 javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -public class Servlet2Accessor extends JavaxServletAccessor { +public class Servlet2Accessor extends JavaxServletAccessor + implements HttpServerResponseMutator { public static final Servlet2Accessor INSTANCE = new Servlet2Accessor(); private Servlet2Accessor() {} @@ -55,4 +57,9 @@ public List getResponseHeaderValues( public boolean isResponseCommitted(HttpServletResponse httpServletResponse) { return httpServletResponse.isCommitted(); } + + @Override + public void appendHeader(HttpServletResponse response, String name, String value) { + response.addHeader(name, value); + } } diff --git a/instrumentation/servlet/servlet-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v2_2/Servlet2Advice.java b/instrumentation/servlet/servlet-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v2_2/Servlet2Advice.java index 5c4279cb7e70..6afe0ffc8a2a 100644 --- a/instrumentation/servlet/servlet-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v2_2/Servlet2Advice.java +++ b/instrumentation/servlet/servlet-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v2_2/Servlet2Advice.java @@ -12,6 +12,7 @@ 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 static void onEnter( // 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) diff --git a/instrumentation/servlet/servlet-2.2/javaagent/src/test/groovy/JettyServlet2Test.groovy b/instrumentation/servlet/servlet-2.2/javaagent/src/test/groovy/JettyServlet2Test.groovy index 53ffbe24d9ca..3a3529362558 100644 --- a/instrumentation/servlet/servlet-2.2/javaagent/src/test/groovy/JettyServlet2Test.groovy +++ b/instrumentation/servlet/servlet-2.2/javaagent/src/test/groovy/JettyServlet2Test.groovy @@ -111,6 +111,11 @@ class JettyServlet2Test extends HttpServerTest 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" diff --git a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3Accessor.java b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3Accessor.java index a5a22189764d..2a6b1be02fcb 100644 --- a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3Accessor.java +++ b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3Accessor.java @@ -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.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -public class Servlet3Accessor extends JavaxServletAccessor { +public class Servlet3Accessor extends JavaxServletAccessor + implements HttpServerResponseMutator { public static final Servlet3Accessor INSTANCE = new Servlet3Accessor(); private Servlet3Accessor() {} @@ -70,6 +72,11 @@ public boolean isResponseCommitted(HttpServletResponse response) { 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 listener; diff --git a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3Advice.java b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3Advice.java index 9da2aa307eda..25569ee6d1d0 100644 --- a/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3Advice.java +++ b/instrumentation/servlet/servlet-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v3_0/Servlet3Advice.java @@ -11,6 +11,7 @@ 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 @@ && helper().needsRescoping(currentContext, attachedContext)) { 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) diff --git a/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/AbstractServlet3Test.groovy b/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/AbstractServlet3Test.groovy index 6dbdb60eb1b7..fd905ddc1f6a 100644 --- a/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/AbstractServlet3Test.groovy +++ b/instrumentation/servlet/servlet-3.0/javaagent/src/test/groovy/AbstractServlet3Test.groovy @@ -79,6 +79,11 @@ abstract class AbstractServlet3Test extends HttpServerTest { +public class Servlet5Accessor + implements ServletAccessor, + HttpServerResponseMutator { public static final Servlet5Accessor INSTANCE = new Servlet5Accessor(); private Servlet5Accessor() {} @@ -172,6 +175,11 @@ public boolean isServletException(Throwable throwable) { 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 listener; diff --git a/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/service/JakartaServletServiceAdvice.java b/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/service/JakartaServletServiceAdvice.java index a32ad87b15e0..ced8758574df 100644 --- a/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/service/JakartaServletServiceAdvice.java +++ b/instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/service/JakartaServletServiceAdvice.java @@ -11,9 +11,11 @@ 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 @@ && helper().needsRescoping(currentContext, attachedContext)) { 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) diff --git a/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/AbstractServlet5Test.groovy b/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/AbstractServlet5Test.groovy index 99a64cd5ac38..bb6175902824 100644 --- a/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/AbstractServlet5Test.groovy +++ b/instrumentation/servlet/servlet-5.0/javaagent/src/test/groovy/AbstractServlet5Test.groovy @@ -78,6 +78,11 @@ abstract class AbstractServlet5Test extends HttpServerTest