Skip to content

Commit 09682bd

Browse files
jandro996devflow.devflow-routing-intake
andauthored
Add http.status_code, error, and http.useragent to inferred proxy spans (#10749)
WIP WIP WIP Fix service-entry spans inheriting gateway domain name as service name under inferred proxy spans When an inferred proxy span is the parent, the service-entry span was inheriting its service name (the gateway domain name, e.g. system-tests-api-gateway.com) instead of using the application's configured service name. Reset the service name to the configured DD_SERVICE value in HttpServerDecorator.startSpan() when an inferred proxy parent is detected. Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>
1 parent 5a62ccc commit 09682bd

5 files changed

Lines changed: 109 additions & 21 deletions

File tree

dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,14 @@ public Context startSpan(REQUEST_CARRIER carrier, Context parentContext) {
174174
extracted = startInferredProxySpan(parentContext, extracted);
175175
AgentSpan span =
176176
tracer().startSpan(instrumentationName, spanName(), extracted).setMeasured(true);
177+
// Register service-entry span with inferred proxy span (if present) so that premature
178+
// finish calls from child spans (e.g., Spring MVC handler) are deferred until the
179+
// service-entry span finishes (after the response status is known).
180+
registerServiceEntrySpanInInferredProxy(parentContext, span);
181+
// Reset service name inherited from inferred proxy parent: the inferred span uses the
182+
// gateway domain name as service name, but the service-entry span should identify
183+
// the application (configured DD_SERVICE), not the upstream gateway.
184+
resetServiceNameIfUnderInferredProxy(parentContext, span);
177185
// Apply RequestBlockingAction if any
178186
Flow<Void> flow = callIGCallbackRequestHeaders(span, carrier);
179187
if (flow.getAction() instanceof RequestBlockingAction) {
@@ -193,6 +201,20 @@ protected AgentSpanContext startInferredProxySpan(Context context, AgentSpanCont
193201
return span.start(extracted);
194202
}
195203

204+
private void registerServiceEntrySpanInInferredProxy(
205+
Context parentContext, AgentSpan serviceEntrySpan) {
206+
InferredProxySpan inferredProxy = InferredProxySpan.fromContext(parentContext);
207+
if (inferredProxy != null) {
208+
inferredProxy.registerServiceEntrySpan(serviceEntrySpan);
209+
}
210+
}
211+
212+
private void resetServiceNameIfUnderInferredProxy(Context parentContext, AgentSpan span) {
213+
if (InferredProxySpan.fromContext(parentContext) != null) {
214+
span.setServiceName(Config.get().getServiceName());
215+
}
216+
}
217+
196218
private final DataStreamsTransactionTracker.TransactionSourceReader
197219
DSM_TRANSACTION_SOURCE_READER =
198220
(source, headerName) -> {

dd-java-agent/instrumentation/spring/spring-webmvc/spring-webmvc-3.1/src/test/groovy/test/boot/SpringBootBasedTest.groovy

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -523,6 +523,8 @@ class SpringBootBasedTest extends HttpServerTest<ConfigurableApplicationContext>
523523
"$Tags.HTTP_ROUTE" "/success"
524524
"stage" "test"
525525
"_dd.inferred_span" 1
526+
"$Tags.HTTP_STATUS" SUCCESS.status
527+
"$Tags.HTTP_USER_AGENT" String
526528
// Standard tags that are automatically added
527529
"_dd.agent_psr" Number
528530
"_dd.base_service" String
@@ -541,10 +543,8 @@ class SpringBootBasedTest extends HttpServerTest<ConfigurableApplicationContext>
541543
}
542544
}
543545
// Server span should be a child of the inferred proxy span
544-
// When there's an inferred proxy span parent, the server span inherits the parent's service name
545546
span {
546-
// Service name is inherited from the inferred proxy span parent
547-
serviceName "api.example.com"
547+
serviceName expectedServiceName()
548548
operationName operation()
549549
resourceName expectedResourceName(SUCCESS, "GET", address)
550550
spanType DDSpanTypes.HTTP_SERVER
@@ -568,9 +568,8 @@ class SpringBootBasedTest extends HttpServerTest<ConfigurableApplicationContext>
568568
}
569569
}
570570
if (hasHandlerSpan()) {
571-
// Handler span inherits service name from inferred proxy span parent
572571
it.span {
573-
serviceName "api.example.com"
572+
serviceName expectedServiceName()
574573
operationName "spring.handler"
575574
resourceName "TestController.success"
576575
spanType DDSpanTypes.HTTP_SERVER
@@ -583,9 +582,8 @@ class SpringBootBasedTest extends HttpServerTest<ConfigurableApplicationContext>
583582
}
584583
}
585584
}
586-
// Controller span also inherits service name
587585
it.span {
588-
serviceName "api.example.com"
586+
serviceName expectedServiceName()
589587
operationName "controller"
590588
resourceName "controller"
591589
errored false

dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/TraceMapperV0_4.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,10 @@ public void accept(Metadata metadata) {
7171

7272
TagMap tags = metadata.getTags();
7373

74-
final boolean writeSamplingPriority = firstSpanInTrace || lastSpanInTrace;
74+
// Also write on top-level spans so that inferred proxy spans (which may be in the middle
75+
// of the serialized list due to phased-finish ordering) always carry the sampling decision.
76+
final boolean writeSamplingPriority =
77+
firstSpanInTrace || lastSpanInTrace || metadata.topLevel();
7578
final UTF8BytesString processTags = firstSpanInPayload ? metadata.processTags() : null;
7679
int metaSize =
7780
metadata.getBaggage().size()

dd-trace-core/src/main/java/datadog/trace/common/writer/ddagent/TraceMapperV0_5.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,10 @@ MetaWriter forSpan(boolean firstInTrace, boolean lastInTrace, boolean firstInPay
202202

203203
@Override
204204
public void accept(Metadata metadata) {
205-
final boolean writeSamplingPriority = firstSpanInTrace || lastSpanInTrace;
205+
// Also write on top-level spans so that inferred proxy spans (which may be in the middle
206+
// of the serialized list due to phased-finish ordering) always carry the sampling decision.
207+
final boolean writeSamplingPriority =
208+
firstSpanInTrace || lastSpanInTrace || metadata.topLevel();
206209
final UTF8BytesString processTags = firstSpanInPayload ? metadata.processTags() : null;
207210

208211
TagMap tags = metadata.getTags();

internal-api/src/main/java/datadog/trace/api/gateway/InferredProxySpan.java

Lines changed: 74 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@
22

33
import static datadog.context.ContextKey.named;
44
import static datadog.trace.api.DDTags.SPAN_TYPE;
5+
import static datadog.trace.bootstrap.instrumentation.api.ErrorPriorities.HTTP_SERVER_DECORATOR;
56
import static datadog.trace.bootstrap.instrumentation.api.ResourceNamePriorities.MANUAL_INSTRUMENTATION;
67
import static datadog.trace.bootstrap.instrumentation.api.Tags.COMPONENT;
78
import static datadog.trace.bootstrap.instrumentation.api.Tags.HTTP_METHOD;
89
import static datadog.trace.bootstrap.instrumentation.api.Tags.HTTP_ROUTE;
910
import static datadog.trace.bootstrap.instrumentation.api.Tags.HTTP_URL;
11+
import static datadog.trace.bootstrap.instrumentation.api.Tags.HTTP_USER_AGENT;
1012
import static datadog.trace.bootstrap.instrumentation.api.Tags.SPAN_KIND;
1113
import static datadog.trace.bootstrap.instrumentation.api.Tags.SPAN_KIND_SERVER;
1214

@@ -46,6 +48,10 @@ public class InferredProxySpan implements ImplicitContextKeyed {
4648

4749
private final Map<String, String> headers;
4850
private AgentSpan span;
51+
// Service-entry span registered at startSpan() time; used to guard against premature finishing
52+
// by child spans (e.g., Spring MVC handler spans) before the response status is known.
53+
private AgentSpan registeredServiceEntrySpan;
54+
private boolean phasedFinished;
4955

5056
public static InferredProxySpan fromHeaders(Map<String, String> values) {
5157
return new InferredProxySpan(values);
@@ -191,31 +197,75 @@ private String computeArn(String proxySystem, String region, String apiId) {
191197
return String.format("arn:%s:apigateway:%s::/%s/%s", partition, region, resourceType, apiId);
192198
}
193199

200+
/**
201+
* Registers the service-entry span for this inferred proxy span. This allows {@link
202+
* #finish(AgentSpan)} to distinguish between premature finish calls from child handler spans
203+
* (e.g., Spring MVC) and the final finish call from the service-entry span after the response is
204+
* written.
205+
*/
206+
public void registerServiceEntrySpan(AgentSpan serviceEntrySpan) {
207+
this.registeredServiceEntrySpan = serviceEntrySpan;
208+
}
209+
194210
public void finish() {
195211
finish(null);
196212
}
197213

198214
/**
199-
* Finishes this inferred proxy span and copies AppSec tags from the service-entry span to this
200-
* span as required by RFC-1081. AppSec detection occurs in the service-entry span context, so its
201-
* tags must be propagated to the inferred proxy span for endpoint correlation.
215+
* Finishes this inferred proxy span, copying relevant tags from the given span.
202216
*
203-
* @param serviceEntrySpan the service-entry child span, or null if not available
217+
* <p>When a service-entry span is registered (via {@link #registerServiceEntrySpan}), this method
218+
* distinguishes between two callers:
219+
*
220+
* <ul>
221+
* <li><b>Non-service-entry caller</b> (e.g., Spring MVC handler span): copies available tags
222+
* (AppSec) and calls {@link AgentSpan#phasedFinish()} to record duration without
223+
* publishing. The span stays alive so HTTP tags can be added later.
224+
* <li><b>Service-entry caller</b>: copies all tags including HTTP status/error/useragent, then
225+
* publishes the span (via {@link AgentSpan#publish()} if phasedFinished, or {@link
226+
* AgentSpan#finish()} otherwise).
227+
* </ul>
228+
*
229+
* @param callerSpan the span calling finish, used to copy tags and determine caller type
204230
*/
205-
public void finish(AgentSpan serviceEntrySpan) {
206-
if (this.span != null) {
207-
copyAppSecTagsFromServiceEntry(serviceEntrySpan);
208-
this.span.finish();
231+
public void finish(AgentSpan callerSpan) {
232+
if (this.span == null) {
233+
return;
234+
}
235+
236+
boolean isServiceEntryOrFallback =
237+
registeredServiceEntrySpan == null
238+
|| callerSpan == null
239+
|| callerSpan == registeredServiceEntrySpan;
240+
241+
if (isServiceEntryOrFallback) {
242+
// Final call: copy all tags (AppSec + HTTP status/error/useragent) and close the span
243+
copyTagsFromServiceEntry(callerSpan);
244+
if (phasedFinished) {
245+
this.span.publish();
246+
} else {
247+
this.span.finish();
248+
}
209249
this.span = null;
250+
this.registeredServiceEntrySpan = null;
251+
this.phasedFinished = false;
252+
} else if (!phasedFinished) {
253+
// First non-service-entry call (e.g., Spring MVC handler span fires beforeFinish() before
254+
// the response is written): copy available tags (AppSec) and phase-finish to record
255+
// duration, but keep the span alive so the service-entry call can add HTTP tags later.
256+
copyTagsFromServiceEntry(callerSpan);
257+
this.span.phasedFinish();
258+
this.phasedFinished = true;
210259
}
260+
// If already phasedFinished and caller is not service-entry: ignore duplicate calls
211261
}
212262

213263
/**
214-
* Copies AppSec tags from the service-entry span to this inferred proxy span as required by
215-
* RFC-1081: the inferred span must carry {@code _dd.appsec.enabled} and {@code _dd.appsec.json}
216-
* so that security activity can be correlated with the API Gateway endpoint.
264+
* Copies relevant tags from the service-entry span to this inferred proxy span. This includes
265+
* AppSec tags required by RFC-1081, plus HTTP tags that are only known after the request
266+
* completes ({@code http.status_code}, {@code error}, {@code http.useragent}).
217267
*/
218-
private void copyAppSecTagsFromServiceEntry(AgentSpan serviceEntrySpan) {
268+
private void copyTagsFromServiceEntry(AgentSpan serviceEntrySpan) {
219269
if (serviceEntrySpan == null || serviceEntrySpan == this.span) {
220270
return;
221271
}
@@ -229,6 +279,18 @@ private void copyAppSecTagsFromServiceEntry(AgentSpan serviceEntrySpan) {
229279
if (appsecJson != null) {
230280
this.span.setTag("_dd.appsec.json", appsecJson.toString());
231281
}
282+
283+
short statusCode = serviceEntrySpan.getHttpStatusCode();
284+
if (statusCode > 0) {
285+
this.span.setHttpStatusCode(statusCode);
286+
boolean isError = Config.get().getHttpServerErrorStatuses().get(statusCode);
287+
this.span.setError(isError, HTTP_SERVER_DECORATOR);
288+
}
289+
290+
Object userAgent = serviceEntrySpan.getTag(HTTP_USER_AGENT);
291+
if (userAgent != null) {
292+
this.span.setTag(HTTP_USER_AGENT, userAgent.toString());
293+
}
232294
}
233295

234296
@Override

0 commit comments

Comments
 (0)