From 5db149e1faa886c4cdc6dd528a2e90acd42234ff Mon Sep 17 00:00:00 2001 From: pellmont Date: Mon, 3 Apr 2023 12:06:51 +0200 Subject: [PATCH] fix order of cxf handlers to enable symmetric tracing around jaxws handler chain (#8160) the current implementation of Start and End around the invocation of a Jax WS is asymmetric around the JAX-WS Handler Chain. Current behavior: (execution of incoming MessageHandlers) -> (TracingStartInInterceptor) -> (WebService Invocation) -> (execution of outgoing MessageHandlers) -> (TracingEndInInterceptor) if I understood the code of this cxf instrumentation correctly, the intent was to build the span close around the WebService Invocation (without Handler Chains). So the desired behavior would look like this: (execution of incoming MessageHandlers) -> (TracingStartInInterceptor) -> (WebService Invocation) -> (TracingEndInInterceptor) -> (execution of outgoing MessageHandlers) Unfortunately CXF is calling the Outgoing Chain inside the POST_INVOKE Phase of Cxf (so the outgoing chain is technically a sub-chain in the incoming chain... which is documented but quite surprising...). So the solution in the fix at least guarantees the the outgoing chain is invoked AFTER end of tracing. For any extra Interceptors in the POST_INVOKE Phase there is still no guarantee of ordering, but I think this is not a opentelemetry issue but a design-flaw of CXF... --------- Co-authored-by: Trask Stalnaker Co-authored-by: Lauri Tulmin --- .../cxf/TracingEndInInterceptor.java | 3 +++ .../src/test/groovy/TestWsServlet.groovy | 15 +++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/instrumentation/jaxws/jaxws-2.0-cxf-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/cxf/TracingEndInInterceptor.java b/instrumentation/jaxws/jaxws-2.0-cxf-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/cxf/TracingEndInInterceptor.java index d6ed962b52..19e810cf9f 100644 --- a/instrumentation/jaxws/jaxws-2.0-cxf-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/cxf/TracingEndInInterceptor.java +++ b/instrumentation/jaxws/jaxws-2.0-cxf-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/cxf/TracingEndInInterceptor.java @@ -5,6 +5,7 @@ package io.opentelemetry.javaagent.instrumentation.cxf; +import org.apache.cxf.interceptor.OutgoingChainInterceptor; import org.apache.cxf.message.Message; import org.apache.cxf.phase.AbstractPhaseInterceptor; import org.apache.cxf.phase.Phase; @@ -12,6 +13,8 @@ import org.apache.cxf.phase.Phase; public class TracingEndInInterceptor extends AbstractPhaseInterceptor { public TracingEndInInterceptor() { super(Phase.POST_INVOKE); + // end the span before the OutInterceptors (callbacks) are executed + addBefore(OutgoingChainInterceptor.class.getName()); } @Override diff --git a/instrumentation/jaxws/jaxws-2.0-cxf-3.0/javaagent/src/test/groovy/TestWsServlet.groovy b/instrumentation/jaxws/jaxws-2.0-cxf-3.0/javaagent/src/test/groovy/TestWsServlet.groovy index d8cd8064fc..6e40c049ec 100644 --- a/instrumentation/jaxws/jaxws-2.0-cxf-3.0/javaagent/src/test/groovy/TestWsServlet.groovy +++ b/instrumentation/jaxws/jaxws-2.0-cxf-3.0/javaagent/src/test/groovy/TestWsServlet.groovy @@ -4,7 +4,13 @@ */ import hello.HelloServiceImpl +import io.opentelemetry.api.trace.Span +import io.opentelemetry.context.Context +import io.opentelemetry.instrumentation.api.instrumenter.LocalRootSpan import org.apache.cxf.jaxws.EndpointImpl +import org.apache.cxf.message.Message +import org.apache.cxf.phase.AbstractPhaseInterceptor +import org.apache.cxf.phase.Phase import org.apache.cxf.transport.servlet.CXFNonSpringServlet import javax.servlet.ServletConfig @@ -18,5 +24,14 @@ class TestWsServlet extends CXFNonSpringServlet { Object implementor = new HelloServiceImpl() EndpointImpl endpoint = new EndpointImpl(bus, implementor) endpoint.publish("/HelloService") + endpoint.getOutInterceptors().add(new AbstractPhaseInterceptor(Phase.SETUP) { + @Override + void handleMessage(Message message) { + Context context = Context.current() + if (LocalRootSpan.fromContext(context) != Span.fromContext(context)) { + throw new IllegalStateException("handler span should be ended before outgoing interceptors") + } + } + }) } }