Skip to content

Commit 693449b

Browse files
bm1549jordan-wongdevflow.devflow-routing-intake
authored
Fix Synapse 3.0 instrumentation bugs and harden span propagation (#10892)
fix: fix Synapse 3.0 instrumentation bugs and harden span propagation Three deterministic bugs fixed: 1. Key mismatch in PassthruInstrumentation: PR #9422 renamed the context key constant but forgot to update the hardcoded string literal in SynapsePassthruInstrumentation, breaking parent span propagation across the passthru mechanism. 2. http.url test assertion: QueryObfuscator recombines http.url with the query string after obfuscation. The test expected path-only but the actual value correctly includes the query string. 3. Missing peer.service in V1 client spans: SynapseClientDecorator returns relative URIs (no host), so peer.hostname was never set. Added peer hostname/IP/port extraction from NHttpClientConnection. Additionally, hardened the passthru span propagation to read the server span directly from the source connection context rather than relying solely on activeSpan(). SourceHandler.requestReceived() dispatches to a worker thread pool, and while java-concurrent instrumentation normally handles context propagation across ThreadPoolExecutor, the connection- based lookup is more robust. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> fix: delegate peer tagging to BaseDecorator for IPv6 and config support Use DECORATE.onPeerConnection() and setPeerPort() instead of manually setting peer tags. This correctly classifies IPv6 addresses as peer.ipv6 (instead of forcing peer.ipv4) and honors the trace.peer.hostname.enabled config toggle for peer.hostname. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Merge branch 'master' into brian.marks/fix-synapse-instrumentation-bugs Co-authored-by: jordan-wong <61242306+jordan-wong@users.noreply.github.com> Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>
1 parent 4666c89 commit 693449b

File tree

3 files changed

+35
-6
lines changed

3 files changed

+35
-6
lines changed

dd-java-agent/instrumentation/synapse-3.0/src/main/java/datadog/trace/instrumentation/synapse3/SynapseClientInstrumentation.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
2525
import net.bytebuddy.asm.Advice;
2626
import org.apache.axis2.context.MessageContext;
27+
import org.apache.http.HttpInetConnection;
2728
import org.apache.http.HttpResponse;
2829
import org.apache.http.nio.NHttpClientConnection;
2930
import org.apache.synapse.transport.passthru.TargetContext;
@@ -107,6 +108,13 @@ public static void requestSubmitted(
107108
// populate span using details from the submitted HttpRequest (resolved URI, etc.)
108109
AgentSpan span = spanFromContext(scope.context());
109110
DECORATE.onRequest(span, TargetContext.getRequest(connection).getRequest());
111+
112+
// set peer info from the connection since request URIs are relative paths
113+
if (connection instanceof HttpInetConnection) {
114+
HttpInetConnection inetConn = (HttpInetConnection) connection;
115+
DECORATE.onPeerConnection(span, inetConn.getRemoteAddress());
116+
DECORATE.setPeerPort(span, inetConn.getRemotePort());
117+
}
110118
scope.close();
111119
}
112120
}

dd-java-agent/instrumentation/synapse-3.0/src/main/java/datadog/trace/instrumentation/synapse3/SynapsePassthruInstrumentation.java

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,13 @@
22

33
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
44
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeSpan;
5+
import static datadog.trace.bootstrap.instrumentation.api.Java8BytecodeBridge.spanFromContext;
6+
import static datadog.trace.instrumentation.synapse3.SynapseClientDecorator.SYNAPSE_CONTEXT_KEY;
57
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
68
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
79

810
import com.google.auto.service.AutoService;
11+
import datadog.context.Context;
912
import datadog.trace.agent.tooling.Instrumenter;
1013
import datadog.trace.agent.tooling.InstrumenterModule;
1114
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
@@ -14,6 +17,7 @@
1417
import java.util.Map;
1518
import net.bytebuddy.asm.Advice;
1619
import org.apache.axis2.context.MessageContext;
20+
import org.apache.http.nio.NHttpServerConnection;
1721

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

56-
// use message context to propagate active spans across Synapse's 'passthru' mechanism
57-
AgentSpan span = activeSpan();
60+
// Propagate the server span to the client via the message context.
61+
// Prefer reading the span directly from the source connection's context (where
62+
// SynapseServerInstrumentation stored it) over activeSpan(). SourceHandler dispatches
63+
// request processing to a worker thread pool, and while java-concurrent instrumentation
64+
// normally propagates context across ThreadPoolExecutor, the connection-based lookup is
65+
// more robust as it doesn't depend on automatic context propagation.
66+
AgentSpan span = null;
67+
Object sourceConn = message.getProperty("pass-through.Source-Connection");
68+
if (sourceConn instanceof NHttpServerConnection) {
69+
Object ctx =
70+
((NHttpServerConnection) sourceConn).getContext().getAttribute(SYNAPSE_CONTEXT_KEY);
71+
if (ctx instanceof Context) {
72+
span = spanFromContext((Context) ctx);
73+
}
74+
}
75+
if (null == span) {
76+
span = activeSpan();
77+
}
5878
if (null != span) {
59-
message.setNonReplicableProperty("dd.trace.synapse.span", span);
79+
message.setNonReplicableProperty(SYNAPSE_CONTEXT_KEY, span);
6080
}
6181
}
6282
}

dd-java-agent/instrumentation/synapse-3.0/src/test/groovy/datadog/trace/instrumentation/synapse3/SynapseTest.groovy

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ abstract class SynapseTest extends VersionedNamingTestBase {
247247
"$Tags.SPAN_KIND" Tags.SPAN_KIND_SERVER
248248
"$Tags.PEER_HOST_IPV4" "127.0.0.1"
249249
"$Tags.PEER_PORT" Integer
250-
"$Tags.HTTP_URL" "/services/SimpleStockQuoteService"
250+
"$Tags.HTTP_URL" query ? "/services/SimpleStockQuoteService?${query}" : "/services/SimpleStockQuoteService"
251251
"$DDTags.HTTP_QUERY" query
252252
"$Tags.HTTP_METHOD" method
253253
"$Tags.HTTP_STATUS" statusCode
@@ -302,6 +302,9 @@ abstract class SynapseTest extends VersionedNamingTestBase {
302302
tags {
303303
"$Tags.COMPONENT" "synapse-client"
304304
"$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT
305+
"$Tags.PEER_HOSTNAME" String
306+
"$Tags.PEER_HOST_IPV4" "127.0.0.1"
307+
"$Tags.PEER_PORT" Integer
305308
"$Tags.HTTP_URL" "/services/SimpleStockQuoteService"
306309
"$Tags.HTTP_METHOD" method
307310
"$Tags.HTTP_STATUS" statusCode
@@ -311,7 +314,6 @@ abstract class SynapseTest extends VersionedNamingTestBase {
311314
}
312315
}
313316

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

317319

@@ -321,7 +323,6 @@ class SynapseV0ForkedTest extends SynapseTest implements TestingGenericHttpNamin
321323
}
322324
}
323325

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

327328
@Override

0 commit comments

Comments
 (0)