feat(laravel): inject trace context into outbound HTTP client requests#656
feat(laravel): inject trace context into outbound HTTP client requests#656zigzagdev wants to merge 7 commits into
HTTP client requests#656Conversation
Setter for injecting W3C trace-context headers into an immutable PSR-7 RequestInterface, following the same assert()+singleton pattern as the other PropagationSetterInterface implementations in this monorepo.
Hooks PendingRequest::runBeforeSendingCallbacks(), where Laravel finalizes the PSR-7 request just before handing it to Guzzle, and injects the active span's trace context into the request headers. Previously no traceparent/tracestate was ever sent, so downstream services had no way to link back into the calling trace.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #656 +/- ##
============================================
- Coverage 80.09% 80.02% -0.07%
- Complexity 1597 1605 +8
============================================
Files 118 120 +2
Lines 6179 6204 +25
============================================
+ Hits 4949 4965 +16
- Misses 1230 1239 +9 Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
ChrisLightfootWild
left a comment
ChrisLightfootWild
left a comment
There was a problem hiding this comment.
We should opt into this behaviour rather than it be the default. There is no mechanism to prevent leaking to external endpoints (we do not know what a developer may be sending in baggage etc)
The target of an outbound Http::client request may be a third-party service outside the application's control, so trace context (and, in future, baggage) should not be sent there by default. Gate injection behind OTEL_PHP_INSTRUMENTATION_LARAVEL_HTTP_CLIENT_PROPAGATION_ENABLED, off unless explicitly opted into.
|
I fixed it. |
PuvaanRaaj
left a comment
PuvaanRaaj
left a comment
There was a problem hiding this comment.
Reviewed the updated opt-in propagation implementation and its Laravel CI results. The opt-in addresses the earlier leakage concern, but the current head has a directly introduced Phan failure that blocks the quality matrix.
| return $params; | ||
| } | ||
|
|
||
| TraceContextPropagator::getInstance()->inject($request, RequestPropagationSetter::instance(), Context::getCurrent()); |
There was a problem hiding this comment.
This call currently fails the Laravel quality jobs with PhanAccessMethodInternal: RequestPropagationSetter is marked @internal and is being called from the Hooks subnamespace. Please adjust the annotation/scope or use the project’s established suppression pattern so the required quality checks pass.
There was a problem hiding this comment.
Suppressed via the same @phan-suppress-next-line PhanAccessMethodInternal pattern used in Kernel.php, rather than removing @internal from
▎ RequestPropagationSetter (it's intentionally internal). phan passes locally now.
3 participants
Summary
Motivation
ClientRequestWatcherrecords spans for outboundIlluminate\Http\Clientrequests, but never injectedtraceparent/tracestateheaders into them.Since the watcher only listens on the
RequestSendingevent, which carries an immutable PSR-7 request that never reaches the request Guzzle actually sends, distributed tracing was completely broken for any downstream service called via Laravel's HTTP client — there was no way to link the receiving service's trace back to the calling trace.What I have done
RequestPropagationSetter(src/Propagators/RequestPropagationSetter.php), aPropagationSetterInterfaceimplementation for PSR-7RequestInterface, following the same pattern as the existingResponsePropagationSetterand the otherPropagationSetterInterfaceimplementations across this monorepo.Illuminate\Http\Client\PendingRequest::runBeforeSendingCallbacks()(src/Hooks/Illuminate/Http/Client/PendingRequest.php), the point where Laravel finalizes the PSR-7 request justbefore handing it to Guzzle. Injects the active span's trace context into the request headers there, since that's
the last point the actual outgoing request can still be mutated.
LaravelInstrumentation.php.test_it_injects_trace_context_into_outbound_requeststoClientTest.php, asserting the injectedtraceparentheader matches an active parent span's trace/span id.Test Results