Skip to content

Commit

Permalink
Fix missing data on HTTP spans (#1735)
Browse files Browse the repository at this point in the history 
Co-authored-by: Michi Hoffmann <cleptric@users.noreply.github.com>
  • Loading branch information
@stayallive @cleptric
stayallive and cleptric authored May 10, 2024
1 parent 9dcd4ca commit d12ed37
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 25 deletions.
45 changes: 22 additions & 23 deletions src/Tracing/GuzzleTracingMiddleware.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,22 @@ public static function trace(?HubInterface $hub = null): \Closure
'path' => $request->getUri()->getPath(),
]);

$spanAndBreadcrumbData = [
'http.request.method' => $request->getMethod(),
'http.request.body.size' => $request->getBody()->getSize(),
];

if ($request->getUri()->getQuery() !== '') {
$spanAndBreadcrumbData['http.query'] = $request->getUri()->getQuery();
}
if ($request->getUri()->getFragment() !== '') {
$spanAndBreadcrumbData['http.fragment'] = $request->getUri()->getFragment();
}

$spanContext = new SpanContext();
$spanContext->setOp('http.client');
$spanContext->setDescription($request->getMethod() . ' ' . (string) $partialUri);
$spanContext->setData([
'http.request.method' => $request->getMethod(),
'http.query' => $request->getUri()->getQuery(),
'http.fragment' => $request->getUri()->getFragment(),
]);
$spanContext->setDescription($request->getMethod() . ' ' . $partialUri);
$spanContext->setData($spanAndBreadcrumbData);

$childSpan = $span->startChild($spanContext);

Expand All @@ -66,7 +74,7 @@ public static function trace(?HubInterface $hub = null): \Closure
->withHeader('baggage', $childSpan->toBaggage());
}

$handlerPromiseCallback = static function ($responseOrException) use ($hub, $request, $childSpan, $partialUri) {
$handlerPromiseCallback = static function ($responseOrException) use ($hub, $spanAndBreadcrumbData, $childSpan, $partialUri) {
// We finish the span (which means setting the span end timestamp) first to ensure the measured time
// the span spans is as close to only the HTTP request time and do the data collection afterwards
$childSpan->finish();
Expand All @@ -80,23 +88,12 @@ public static function trace(?HubInterface $hub = null): \Closure
$response = $responseOrException->getResponse();
}

$breadcrumbData = [
'url' => (string) $partialUri,
'http.request.method' => $request->getMethod(),
'http.request.body.size' => $request->getBody()->getSize(),
];
if ($request->getUri()->getQuery() !== '') {
$breadcrumbData['http.query'] = $request->getUri()->getQuery();
}
if ($request->getUri()->getFragment() !== '') {
$breadcrumbData['http.fragment'] = $request->getUri()->getFragment();
}

if ($response !== null) {
$childSpan->setStatus(SpanStatus::createFromHttpStatusCode($response->getStatusCode()));
$spanAndBreadcrumbData['http.response.body.size'] = $response->getBody()->getSize();
$spanAndBreadcrumbData['http.response.status_code'] = $response->getStatusCode();

$breadcrumbData['http.response.status_code'] = $response->getStatusCode();
$breadcrumbData['http.response.body.size'] = $response->getBody()->getSize();
$childSpan->setStatus(SpanStatus::createFromHttpStatusCode($response->getStatusCode()));
$childSpan->setData($spanAndBreadcrumbData);
} else {
$childSpan->setStatus(SpanStatus::internalError());
}
Expand All @@ -106,7 +103,9 @@ public static function trace(?HubInterface $hub = null): \Closure
Breadcrumb::TYPE_HTTP,
'http',
null,
$breadcrumbData
array_merge([
'url' => (string) $partialUri,
], $spanAndBreadcrumbData)
));

if ($responseOrException instanceof \Throwable) {
Expand Down
29 changes: 27 additions & 2 deletions tests/Tracing/GuzzleTracingMiddlewareTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ public static function traceHeadersDataProvider(): iterable
/**
* @dataProvider traceDataProvider
*/
public function testTrace(Request $request, $expectedPromiseResult, array $expectedBreadcrumbData): void
public function testTrace(Request $request, $expectedPromiseResult, array $expectedBreadcrumbData, array $expectedSpanData): void
{
$client = $this->createMock(ClientInterface::class);
$client->expects($this->exactly(4))
Expand All @@ -201,7 +201,7 @@ public function testTrace(Request $request, $expectedPromiseResult, array $expec

$client->expects($this->once())
->method('captureEvent')
->with($this->callback(function (Event $eventArg) use ($hub, $request, $expectedPromiseResult, $expectedBreadcrumbData): bool {
->with($this->callback(function (Event $eventArg) use ($hub, $request, $expectedPromiseResult, $expectedBreadcrumbData, $expectedSpanData): bool {
$this->assertSame(EventType::transaction(), $eventArg->getType());

$hub->configureScope(static function (Scope $scope) use ($eventArg): void {
Expand Down Expand Up @@ -233,6 +233,7 @@ public function testTrace(Request $request, $expectedPromiseResult, array $expec
$this->assertSame(SpanStatus::internalError(), $guzzleSpan->getStatus());
}

$this->assertSame($expectedSpanData, $guzzleSpan->getData());
$this->assertSame($expectedBreadcrumbData, $guzzleBreadcrumb->getMetadata());

return true;
Expand Down Expand Up @@ -277,8 +278,14 @@ public static function traceDataProvider(): iterable
'url' => 'https://www.example.com',
'http.request.method' => 'GET',
'http.request.body.size' => 0,
'http.response.body.size' => 0,
'http.response.status_code' => 200,
],
[
'http.request.method' => 'GET',
'http.request.body.size' => 0,
'http.response.body.size' => 0,
'http.response.status_code' => 200,
],
];

Expand All @@ -291,8 +298,16 @@ public static function traceDataProvider(): iterable
'http.request.body.size' => 0,
'http.query' => 'query=string',
'http.fragment' => 'fragment=1',
'http.response.body.size' => 0,
'http.response.status_code' => 200,
],
[
'http.request.method' => 'GET',
'http.request.body.size' => 0,
'http.query' => 'query=string',
'http.fragment' => 'fragment=1',
'http.response.body.size' => 0,
'http.response.status_code' => 200,
],
];

Expand All @@ -303,8 +318,14 @@ public static function traceDataProvider(): iterable
'url' => 'https://www.example.com',
'http.request.method' => 'POST',
'http.request.body.size' => 10,
'http.response.body.size' => 6,
'http.response.status_code' => 403,
],
[
'http.request.method' => 'POST',
'http.request.body.size' => 10,
'http.response.body.size' => 6,
'http.response.status_code' => 403,
],
];

Expand All @@ -316,6 +337,10 @@ public static function traceDataProvider(): iterable
'http.request.method' => 'GET',
'http.request.body.size' => 0,
],
[
'http.request.method' => 'GET',
'http.request.body.size' => 0,
],
];
}
}

0 comments on commit d12ed37

Please sign in to comment.