diff --git a/src/Tracing/GuzzleTracingMiddleware.php b/src/Tracing/GuzzleTracingMiddleware.php index 08e640359..6dfde7840 100644 --- a/src/Tracing/GuzzleTracingMiddleware.php +++ b/src/Tracing/GuzzleTracingMiddleware.php @@ -28,18 +28,7 @@ public static function trace(?HubInterface $hub = null): \Closure return static function (RequestInterface $request, array $options) use ($hub, $handler) { $hub = $hub ?? SentrySdk::getCurrentHub(); $client = $hub->getClient(); - $span = $hub->getSpan(); - - if ($span === null) { - if (self::shouldAttachTracingHeaders($client, $request)) { - $request = $request - ->withHeader('sentry-trace', getTraceparent()) - ->withHeader('traceparent', getW3CTraceparent()) - ->withHeader('baggage', getBaggage()); - } - - return $handler($request, $options); - } + $parentSpan = $hub->getSpan(); $partialUri = Uri::fromParts([ 'scheme' => $request->getUri()->getScheme(), @@ -60,24 +49,30 @@ public static function trace(?HubInterface $hub = null): \Closure $spanAndBreadcrumbData['http.fragment'] = $request->getUri()->getFragment(); } - $spanContext = new SpanContext(); - $spanContext->setOp('http.client'); - $spanContext->setDescription($request->getMethod() . ' ' . $partialUri); - $spanContext->setData($spanAndBreadcrumbData); + $childSpan = null; + + if ($parentSpan !== null && $parentSpan->getSampled()) { + $spanContext = new SpanContext(); + $spanContext->setOp('http.client'); + $spanContext->setDescription($request->getMethod() . ' ' . $partialUri); + $spanContext->setData($spanAndBreadcrumbData); - $childSpan = $span->startChild($spanContext); + $childSpan = $parentSpan->startChild($spanContext); + } if (self::shouldAttachTracingHeaders($client, $request)) { $request = $request - ->withHeader('sentry-trace', $childSpan->toTraceparent()) - ->withHeader('traceparent', $childSpan->toW3CTraceparent()) - ->withHeader('baggage', $childSpan->toBaggage()); + ->withHeader('sentry-trace', getTraceparent()) + ->withHeader('traceparent', getW3CTraceparent()) + ->withHeader('baggage', getBaggage()); } $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(); + if ($childSpan !== null) { + // 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(); + } $response = null; @@ -91,11 +86,15 @@ public static function trace(?HubInterface $hub = null): \Closure if ($response !== null) { $spanAndBreadcrumbData['http.response.body.size'] = $response->getBody()->getSize(); $spanAndBreadcrumbData['http.response.status_code'] = $response->getStatusCode(); + } - $childSpan->setStatus(SpanStatus::createFromHttpStatusCode($response->getStatusCode())); - $childSpan->setData($spanAndBreadcrumbData); - } else { - $childSpan->setStatus(SpanStatus::internalError()); + if ($childSpan !== null) { + if ($response !== null) { + $childSpan->setStatus(SpanStatus::createFromHttpStatusCode($response->getStatusCode())); + $childSpan->setData($spanAndBreadcrumbData); + } else { + $childSpan->setStatus(SpanStatus::internalError()); + } } $hub->addBreadcrumb(new Breadcrumb( @@ -122,18 +121,14 @@ public static function trace(?HubInterface $hub = null): \Closure private static function shouldAttachTracingHeaders(?ClientInterface $client, RequestInterface $request): bool { - if ($client !== null) { - $sdkOptions = $client->getOptions(); - - // Check if the request destination is allow listed in the trace_propagation_targets option. - if ( - $sdkOptions->getTracePropagationTargets() === null - || \in_array($request->getUri()->getHost(), $sdkOptions->getTracePropagationTargets()) - ) { - return true; - } + if ($client === null) { + return false; } - return false; + $sdkOptions = $client->getOptions(); + + // Check if the request destination is allow listed in the trace_propagation_targets option. + return $sdkOptions->getTracePropagationTargets() === null + || \in_array($request->getUri()->getHost(), $sdkOptions->getTracePropagationTargets()); } } diff --git a/tests/Tracing/GuzzleTracingMiddlewareTest.php b/tests/Tracing/GuzzleTracingMiddlewareTest.php index ae3c30e30..349adadf0 100644 --- a/tests/Tracing/GuzzleTracingMiddlewareTest.php +++ b/tests/Tracing/GuzzleTracingMiddlewareTest.php @@ -23,10 +23,10 @@ final class GuzzleTracingMiddlewareTest extends TestCase { - public function testTraceDoesNothingIfSpanIsNotSet(): void + public function testTraceCreatesBreadcrumbIfSpanIsNotSet(): void { $client = $this->createMock(ClientInterface::class); - $client->expects($this->once()) + $client->expects($this->atLeast(2)) ->method('getOptions') ->willReturn(new Options()); @@ -55,7 +55,7 @@ public function testTraceDoesNothingIfSpanIsNotSet(): void $scope->applyToEvent($event); - $this->assertCount(0, $event->getBreadcrumbs()); + $this->assertCount(1, $event->getBreadcrumbs()); }); } @@ -95,7 +95,7 @@ public function testTraceHeaders(Request $request, Options $options, bool $heade /** * @dataProvider traceHeadersDataProvider */ - public function testTraceHeadersWithTransacttion(Request $request, Options $options, bool $headersShouldBePresent): void + public function testTraceHeadersWithTransaction(Request $request, Options $options, bool $headersShouldBePresent): void { $client = $this->createMock(ClientInterface::class); $client->expects($this->atLeast(2))