search

DataDog / datadog-lambda-go

The Datadog AWS Lambda package for Go
Apache License 2.0
59 stars 40 forks source link

Cross-service tracing breaks when `x-datadog-sampling-priority` header is not present #74

Closed jamestelfer closed 3 years ago

jamestelfer commented 3 years ago

Expected Behavior

The x-datadog-sampling-priority header should (?) be optional; cross-service traces and X-ray merging should still work when this header is missing.

The ddtrace library header injector treats the field as optional, so it may not always be present: https://github.com/DataDog/dd-trace-go/blob/v1/ddtrace/tracer/textmap.go#L225-L227

I came across this behaviour when there was an intermediate service whose runtime doesn't have a first-party library. The traceID and parentID headers were forwarded, seemingly satisfying the requirements of the ddtrace-go library's extractor, but this library implements its own behaviour as part of support for Xray trace merging.

Actual Behavior

If the x-datadog-trace-id and x-datadog-parent-id headers are present, but the x-datadog-sampling-priority header is missing, the trace context is not propagated and the traces aren't joined in the Datadog interface.

Specifications

jamestelfer commented 3 years ago

I'm not 100% certain that this header is being sent by the JS API BTW. It might be, but I haven't yet confirmed it.

hghotra commented 3 years ago

@jamestelfer you're right, even in the JS tracer, it seems like x-datadog-sampling-priority may not always be there.

https://github.com/DataDog/dd-trace-js/blob/7b27059fe3b9de6788c0517a2b499ff6b78da5ff/packages/dd-trace/src/opentracing/propagation/text_map.js#L70-L76

But in the Lambda library, we're not treating it as optional.

https://github.com/DataDog/datadog-lambda-layer-js/blob/f2558a53a86fd8ed040df1fe27b949801b265cbb/src/trace/context.ts#L207-L211

If the sampling priority is not set, the traces get sampled out. I'm not sure why this is the case but perhaps we should set the sampling priority to a default if it's not present (or not send it altogether).

cc @Hesperide @DarcyRaynerDD