Skip to content

Fix Synapse 3.0 instrumentation bugs and harden span propagation #10892

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.

Sign up for GitHub

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

Draft
bm1549 wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Fix Synapse 3.0 instrumentation bugs and harden span propagation #10892

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 15 additions & 0 deletions ....0/src/main/java/datadog/trace/instrumentation/synapse3/SynapseClientInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@
import datadog.trace.agent.tooling.InstrumenterModule;
import datadog.trace.agent.tooling.annotation.AppliesOn;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import datadog.trace.bootstrap.instrumentation.api.Tags;
import net.bytebuddy.asm.Advice;
import org.apache.axis2.context.MessageContext;
import org.apache.http.HttpInetConnection;
import org.apache.http.HttpResponse;
import org.apache.http.nio.NHttpClientConnection;
import org.apache.synapse.transport.passthru.TargetContext;
Expand Down Expand Up @@ -107,6 +109,19 @@ public static void requestSubmitted(
// populate span using details from the submitted HttpRequest (resolved URI, etc.)
AgentSpan span = spanFromContext(scope.context());
DECORATE.onRequest(span, TargetContext.getRequest(connection).getRequest());

// set peer hostname from the connection since request URIs are relative paths
if (connection instanceof HttpInetConnection) {
java.net.InetAddress remoteAddress = ((HttpInetConnection) connection).getRemoteAddress();
if (remoteAddress != null) {
span.setTag(Tags.PEER_HOSTNAME, remoteAddress.getHostName());
span.setTag(Tags.PEER_HOST_IPV4, remoteAddress.getHostAddress());
int remotePort = ((HttpInetConnection) connection).getRemotePort();
if (remotePort > 0) {
span.setTag(Tags.PEER_PORT, remotePort);
}
}
}
scope.close();
}
}
Expand Down
26 changes: 23 additions & 3 deletions .../src/main/java/datadog/trace/instrumentation/synapse3/SynapsePassthruInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,13 @@

import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeSpan;
import static datadog.trace.bootstrap.instrumentation.api.Java8BytecodeBridge.spanFromContext;
import static datadog.trace.instrumentation.synapse3.SynapseClientDecorator.SYNAPSE_CONTEXT_KEY;
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;

import com.google.auto.service.AutoService;
import datadog.context.Context;
import datadog.trace.agent.tooling.Instrumenter;
import datadog.trace.agent.tooling.InstrumenterModule;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
Expand All @@ -14,6 +17,7 @@
import java.util.Map;
import net.bytebuddy.asm.Advice;
import org.apache.axis2.context.MessageContext;
import org.apache.http.nio.NHttpServerConnection;

/** Helps propagate parent spans over 'passthru' mechanism to synapse-client instrumentation. */
@AutoService(InstrumenterModule.class)
Expand Down Expand Up @@ -53,10 +57,26 @@ public static void submit(@Advice.Argument(0) final MessageContext message) {
}
}

// use message context to propagate active spans across Synapse's 'passthru' mechanism
AgentSpan span = activeSpan();
// Propagate the server span to the client via the message context.
// Prefer reading the span directly from the source connection's context (where
// SynapseServerInstrumentation stored it) over activeSpan(). SourceHandler dispatches
// request processing to a worker thread pool, and while java-concurrent instrumentation
// normally propagates context across ThreadPoolExecutor, the connection-based lookup is
// more robust as it doesn't depend on automatic context propagation.
AgentSpan span = null;
Object sourceConn = message.getProperty("pass-through.Source-Connection");
if (sourceConn instanceof NHttpServerConnection) {
Object ctx =
((NHttpServerConnection) sourceConn).getContext().getAttribute(SYNAPSE_CONTEXT_KEY);
if (ctx instanceof Context) {
span = spanFromContext((Context) ctx);
}
}
if (null == span) {
span = activeSpan();
}
if (null != span) {
message.setNonReplicableProperty("dd.trace.synapse.span", span);
message.setNonReplicableProperty(SYNAPSE_CONTEXT_KEY, span);
}
}
}
Expand Down
7 changes: 4 additions & 3 deletions ...ion/synapse-3.0/src/test/groovy/datadog/trace/instrumentation/synapse3/SynapseTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ abstract class SynapseTest extends VersionedNamingTestBase {
"$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER
"$Tags.PEER_HOST_IPV4" "127.0.0.1"
"$Tags.PEER_PORT" Integer
"$Tags.HTTP_URL" "/services/SimpleStockQuoteService"
"$Tags.HTTP_URL" query ? "/services/SimpleStockQuoteService?${query}" : "/services/SimpleStockQuoteService"
"$DDTags.HTTP_QUERY" query
"$Tags.HTTP_METHOD" method
"$Tags.HTTP_STATUS" statusCode
Expand Down Expand Up @@ -302,6 +302,9 @@ abstract class SynapseTest extends VersionedNamingTestBase {
tags {
"$Tags.COMPONENT" "synapse-client"
"$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT
"$Tags.PEER_HOSTNAME" String
"$Tags.PEER_HOST_IPV4" "127.0.0.1"
"$Tags.PEER_PORT" Integer
"$Tags.HTTP_URL" "/services/SimpleStockQuoteService"
"$Tags.HTTP_METHOD" method
"$Tags.HTTP_STATUS" statusCode
Expand All @@ -311,7 +314,6 @@ abstract class SynapseTest extends VersionedNamingTestBase {
}
}

@Flaky("Occasionally times out when receiving traces")
class SynapseV0ForkedTest extends SynapseTest implements TestingGenericHttpNamingConventions.ClientV0 {


Expand All @@ -321,7 +323,6 @@ class SynapseV0ForkedTest extends SynapseTest implements TestingGenericHttpNamin
}
}

@Flaky("Occasionally times out when receiving traces")
class SynapseV1ForkedTest extends SynapseTest implements TestingGenericHttpNamingConventions.ClientV1 {

@Override
Expand Down
Loading