Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add HttpServerResponseCustomizer support for Servlet and Jetty #8095

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,16 @@

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;
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() {}
Expand Down Expand Up @@ -55,4 +57,9 @@ public List<String> getResponseHeaderValues(
public boolean isResponseCommitted(HttpServletResponse httpServletResponse) {
return httpServletResponse.isCommitted();
}

@Override
public void appendHeader(HttpServletResponse response, String name, String value) {
response.addHeader(name, value);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -16,7 +17,8 @@
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() {}
Expand Down Expand Up @@ -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<HttpServletResponse> listener;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -20,7 +21,9 @@
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() {}
Expand Down Expand Up @@ -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<HttpServletResponse> listener;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down