From 8bf155e26122fec741902237321b1a2355faf1c6 Mon Sep 17 00:00:00 2001 From: Martin Linzmayer Date: Mon, 21 Sep 2026 13:28:32 +0200 Subject: [PATCH 1/4] feat(pii): apply data collection to guzzle middleware --- src/Tracing/GuzzleTracingMiddleware.php | 70 ++++--- tests/Tracing/GuzzleTracingMiddlewareTest.php | 180 ++++++++++++++++-- 2 files changed, 208 insertions(+), 42 deletions(-) diff --git a/src/Tracing/GuzzleTracingMiddleware.php b/src/Tracing/GuzzleTracingMiddleware.php index 480502c9c..5eafd3c5d 100644 --- a/src/Tracing/GuzzleTracingMiddleware.php +++ b/src/Tracing/GuzzleTracingMiddleware.php @@ -9,7 +9,10 @@ use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ResponseInterface; use Sentry\Breadcrumb; -use Sentry\ClientInterface; +use Sentry\DataCollection\DataCollectionPolicy; +use Sentry\DataCollection\HttpSpanDataCollector; +use Sentry\DataCollection\HttpUrlCollector; +use Sentry\Options; use Sentry\SentrySdk; use Sentry\State\HubInterface; @@ -28,24 +31,31 @@ public static function trace(?HubInterface $hub = null): \Closure $hub = $hub ?? SentrySdk::getCurrentHub(); $client = $hub->getClient(); $parentSpan = $hub->getSpan(); + $requestUri = $request->getUri(); $partialUri = Uri::fromParts([ - 'scheme' => $request->getUri()->getScheme(), - 'host' => $request->getUri()->getHost(), - 'port' => $request->getUri()->getPort(), - 'path' => $request->getUri()->getPath(), + 'scheme' => $requestUri->getScheme(), + 'host' => $requestUri->getHost(), + 'port' => $requestUri->getPort(), + 'path' => $requestUri->getPath(), ]); + $sdkOptions = $client !== null ? $client->getOptions() : null; + $policy = DataCollectionPolicy::fromOptions($sdkOptions); $spanAndBreadcrumbData = [ 'http.request.method' => $request->getMethod(), 'http.request.body.size' => $request->getBody()->getSize(), ]; - if ($request->getUri()->getQuery() !== '') { - $spanAndBreadcrumbData['http.query'] = $request->getUri()->getQuery(); + $spanAndBreadcrumbData += HttpSpanDataCollector::collectQueryData($policy, $requestUri->getQuery()); + if ($requestUri->getFragment() !== '') { + $spanAndBreadcrumbData['http.fragment'] = $requestUri->getFragment(); } - if ($request->getUri()->getFragment() !== '') { - $spanAndBreadcrumbData['http.fragment'] = $request->getUri()->getFragment(); + + $collectedUrl = (string) $partialUri; + if (!$policy->isLegacyMode()) { + $collectedUrl = HttpUrlCollector::collect($policy, (string) $requestUri); + $spanAndBreadcrumbData['url.full'] = $collectedUrl; } $childSpan = null; @@ -53,7 +63,10 @@ public static function trace(?HubInterface $hub = null): \Closure if ($parentSpan !== null && $parentSpan->getSampled()) { $spanContext = new SpanContext(); $spanContext->setOp('http.client'); - $spanContext->setData($spanAndBreadcrumbData); + $spanContext->setData(array_merge( + $spanAndBreadcrumbData, + HttpSpanDataCollector::collectPsr7RequestData($policy, $request) + )); $spanContext->setOrigin('auto.http.guzzle'); $spanContext->setDescription($request->getMethod() . ' ' . $partialUri); @@ -62,7 +75,7 @@ public static function trace(?HubInterface $hub = null): \Closure $hub->setSpan($childSpan); } - if (self::shouldAttachTracingHeaders($client, $request)) { + if (self::shouldAttachTracingHeaders($sdkOptions, $request)) { $traceParent = getTraceparent(); if ($traceParent !== '') { $request = $request->withHeader('sentry-trace', $traceParent); @@ -74,7 +87,7 @@ public static function trace(?HubInterface $hub = null): \Closure } } - $handlerPromiseCallback = static function ($responseOrException) use ($hub, $spanAndBreadcrumbData, $childSpan, $parentSpan, $partialUri) { + $handlerPromiseCallback = static function ($responseOrException) use ($hub, $spanAndBreadcrumbData, $childSpan, $parentSpan, $collectedUrl, $policy) { 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 @@ -93,21 +106,30 @@ public static function trace(?HubInterface $hub = null): \Closure $breadcrumbLevel = Breadcrumb::LEVEL_INFO; - if ($response !== null) { + if ($response instanceof ResponseInterface) { + $statusCode = $response->getStatusCode(); $spanAndBreadcrumbData['http.response.body.size'] = $response->getBody()->getSize(); - $spanAndBreadcrumbData['http.response.status_code'] = $response->getStatusCode(); + $spanAndBreadcrumbData['http.response.status_code'] = $statusCode; - if ($response->getStatusCode() >= 400 && $response->getStatusCode() < 500) { + if ($statusCode >= 400 && $statusCode < 500) { $breadcrumbLevel = Breadcrumb::LEVEL_WARNING; - } elseif ($response->getStatusCode() >= 500) { + } elseif ($statusCode >= 500) { $breadcrumbLevel = Breadcrumb::LEVEL_ERROR; } } if ($childSpan !== null) { - if ($response !== null) { + if ($response instanceof ResponseInterface) { + $spanData = array_merge( + $spanAndBreadcrumbData, + HttpSpanDataCollector::collectPsr7ResponseData($policy, $response) + ); + if (!$policy->isLegacyMode()) { + $spanData = array_merge($spanData, $childSpan->getData()); + } + $childSpan->setStatus(SpanStatus::createFromHttpStatusCode($response->getStatusCode())); - $childSpan->setData($spanAndBreadcrumbData); + $childSpan->setData($spanData); } else { $childSpan->setStatus(SpanStatus::internalError()); } @@ -119,7 +141,7 @@ public static function trace(?HubInterface $hub = null): \Closure 'http', null, array_merge([ - 'url' => (string) $partialUri, + 'url' => $collectedUrl, ], $spanAndBreadcrumbData) )); @@ -135,16 +157,14 @@ public static function trace(?HubInterface $hub = null): \Closure }; } - private static function shouldAttachTracingHeaders(?ClientInterface $client, RequestInterface $request): bool + private static function shouldAttachTracingHeaders(?Options $options, RequestInterface $request): bool { - if ($client === null) { + if ($options === null) { 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()); + return $options->getTracePropagationTargets() === null + || \in_array($request->getUri()->getHost(), $options->getTracePropagationTargets()); } } diff --git a/tests/Tracing/GuzzleTracingMiddlewareTest.php b/tests/Tracing/GuzzleTracingMiddlewareTest.php index becfa84a8..026c1625e 100644 --- a/tests/Tracing/GuzzleTracingMiddlewareTest.php +++ b/tests/Tracing/GuzzleTracingMiddlewareTest.php @@ -19,7 +19,9 @@ use Sentry\State\Hub; use Sentry\State\Scope; use Sentry\Tracing\GuzzleTracingMiddleware; +use Sentry\Tracing\Span; use Sentry\Tracing\SpanStatus; +use Sentry\Tracing\Transaction; use Sentry\Tracing\TransactionContext; final class GuzzleTracingMiddlewareTest extends TestCase @@ -27,8 +29,7 @@ final class GuzzleTracingMiddlewareTest extends TestCase public function testTraceCreatesBreadcrumbIfSpanIsNotSet(): void { $client = $this->createMock(ClientInterface::class); - $client->expects($this->atLeast(2)) - ->method('getOptions') + $client->method('getOptions') ->willReturn(new Options([ 'traces_sample_rate' => 0, ])); @@ -72,11 +73,10 @@ public function testTraceCreatesBreadcrumbIfSpanIsNotSet(): void public function testTraceCreatesBreadcrumbIfSpanIsRecorded(): void { $client = $this->createMock(ClientInterface::class); - $client->expects($this->atLeast(2)) - ->method('getOptions') - ->willReturn(new Options([ - 'traces_sample_rate' => 1, - ])); + $client->method('getOptions') + ->willReturn(new Options([ + 'traces_sample_rate' => 1, + ])); $hub = new Hub($client); SentrySdk::setCurrentHub($hub); @@ -121,8 +121,7 @@ public function testTraceCreatesBreadcrumbIfSpanIsRecorded(): void public function testTraceHeaders(Request $request, Options $options, bool $headersShouldBePresent): void { $client = $this->createMock(ClientInterface::class); - $client->expects($this->atLeastOnce()) - ->method('getOptions') + $client->method('getOptions') ->willReturn($options); $hub = new Hub($client); @@ -153,8 +152,7 @@ public function testTraceHeaders(Request $request, Options $options, bool $heade public function testTraceHeadersWithTransaction(Request $request, Options $options, bool $headersShouldBePresent): void { $client = $this->createMock(ClientInterface::class); - $client->expects($this->atLeast(2)) - ->method('getOptions') + $client->method('getOptions') ->willReturn($options); $hub = new Hub($client); @@ -195,8 +193,7 @@ public function testTraceHeadersAreNotAddedWhenExternalPropagationContextIsActiv }); $client = $this->createMock(ClientInterface::class); - $client->expects($this->atLeastOnce()) - ->method('getOptions') + $client->method('getOptions') ->willReturn(new Options([ 'trace_propagation_targets' => null, ])); @@ -320,8 +317,7 @@ public static function traceHeadersDataProvider(): iterable public function testTrace(Request $request, $expectedPromiseResult, array $expectedBreadcrumbData, array $expectedSpanData): void { $client = $this->createMock(ClientInterface::class); - $client->expects($this->atLeast(4)) - ->method('getOptions') + $client->method('getOptions') ->willReturn(new Options([ 'traces_sample_rate' => 1, 'trace_propagation_targets' => [ @@ -402,6 +398,153 @@ public function testTrace(Request $request, $expectedPromiseResult, array $expec $transaction->finish(); } + public function testTraceCollectsConfiguredMetadata(): void + { + [$spanData, $breadcrumbData] = $this->traceExchange( + ['data_collection' => []], + new Request('GET', 'https://user:password@www.example.com/path?search=hello%20world&password=secret#fragment', [ + 'Authorization' => 'Bearer secret', + 'Cookie' => 'session_id=secret; theme=dark', + ]), + new Response(200, [ + 'Content-Type' => 'application/json', + 'Set-Cookie' => ['session_id=secret; HttpOnly', 'theme=light; Path=/'], + ]) + ); + + $this->assertSame('https://www.example.com/path?search=hello%20world&password=[Filtered]', $spanData['url.full']); + $this->assertSame('search=hello%20world&password=[Filtered]', $spanData['http.query']); + $this->assertSame(['[Filtered]'], $spanData['http.request.header.authorization']); + $this->assertSame(['application/json'], $spanData['http.response.header.content-type']); + $this->assertSame('[Filtered]', $spanData['http.request.header.cookie.session_id']); + $this->assertSame('dark', $spanData['http.request.header.cookie.theme']); + $this->assertSame('[Filtered]', $spanData['http.response.header.set_cookie.session_id']); + $this->assertSame('light', $spanData['http.response.header.set_cookie.theme']); + $this->assertArrayNotHasKey('http.request.header.cookie', $spanData); + $this->assertArrayNotHasKey('http.response.header.set-cookie', $spanData); + $this->assertSame([ + 'url' => $spanData['url.full'], + 'http.request.method' => 'GET', + 'http.request.body.size' => 0, + 'http.query' => $spanData['http.query'], + 'http.fragment' => 'fragment', + 'url.full' => $spanData['url.full'], + 'http.response.body.size' => 0, + 'http.response.status_code' => 200, + ], $breadcrumbData); + } + + public function testTraceUsesConfiguredQueryFiltering(): void + { + [$spanData, $breadcrumbData] = $this->traceExchange( + ['data_collection' => ['url_query_params' => ['mode' => 'allowList', 'terms' => ['search']]]], + new Request('GET', 'https://www.example.com?search=hello%20world&custom=value'), + new Response() + ); + + $this->assertSame('search=hello%20world&custom=[Filtered]', $spanData['http.query']); + $this->assertSame('https://www.example.com?' . $spanData['http.query'], $spanData['url.full']); + $this->assertSame($spanData['http.query'], $breadcrumbData['http.query']); + $this->assertSame($spanData['url.full'], $breadcrumbData['url']); + } + + public function testTraceRespectsDisabledMetadataCollection(): void + { + [$spanData, $breadcrumbData] = $this->traceExchange( + ['data_collection' => [ + 'cookies' => ['mode' => 'off'], + 'http_headers' => ['mode' => 'off'], + 'http_bodies' => [], + 'url_query_params' => ['mode' => 'off'], + ]], + new Request('GET', 'https://www.example.com?password=secret', [ + 'Authorization' => 'Bearer secret', + 'Cookie' => 'theme=dark', + ]), + new Response(200, ['Content-Type' => 'application/json', 'Set-Cookie' => 'theme=light']) + ); + + $expected = [ + 'http.request.method' => 'GET', + 'http.request.body.size' => 0, + 'url.full' => 'https://www.example.com', + 'http.response.body.size' => 0, + 'http.response.status_code' => 200, + ]; + $this->assertSame($expected, $spanData); + $this->assertSame(array_merge(['url' => 'https://www.example.com'], $expected), $breadcrumbData); + } + + public function testTracePreservesExplicitSpanData(): void + { + $client = $this->createMock(ClientInterface::class); + $client->method('getOptions')->willReturn(new Options(['traces_sample_rate' => 1, 'data_collection' => []])); + $hub = new Hub($client); + $transaction = $hub->startTransaction(new TransactionContext()); + $hub->setSpan($transaction); + $function = (GuzzleTracingMiddleware::trace($hub))(function () use ($hub): PromiseInterface { + $span = $hub->getSpan(); + $this->assertNotNull($span); + $span->setData([ + 'http.query' => 'explicit', + 'http.response.header.authorization' => ['Bearer explicit'], + ]); + + return new FulfilledPromise(new Response(200, [ + 'Content-Type' => 'application/json', + 'Authorization' => 'Bearer automatic', + ])); + }); + + $function(new Request('GET', 'https://www.example.com/?token=secret'), [])->wait(); + + $data = $this->getHttpSpan($transaction)->getData(); + $this->assertSame('explicit', $data['http.query']); + $this->assertSame(['Bearer explicit'], $data['http.response.header.authorization']); + $this->assertSame(['application/json'], $data['http.response.header.content-type']); + } + + /** + * @param array $options + * + * @return array{array, array} + */ + private function traceExchange(array $options, Request $request, Response $response): array + { + $client = $this->createMock(ClientInterface::class); + $client->method('getOptions')->willReturn(new Options($options + ['traces_sample_rate' => 1])); + $hub = new Hub($client); + SentrySdk::setCurrentHub($hub); + $transaction = $hub->startTransaction(new TransactionContext()); + $hub->setSpan($transaction); + $function = (GuzzleTracingMiddleware::trace($hub))(function (Request $forwardedRequest) use ($request, $response): PromiseInterface { + $this->assertSame((string) $request->getUri(), (string) $forwardedRequest->getUri()); + $this->assertSame($request->getHeader('Cookie'), $forwardedRequest->getHeader('Cookie')); + + return new FulfilledPromise($response); + }); + + $this->assertSame($response, $function($request, [])->wait()); + + $event = Event::createEvent(); + $hub->configureScope(static function (Scope $scope) use ($event): void { + $scope->applyToEvent($event); + }); + $this->assertCount(1, $event->getBreadcrumbs()); + + return [$this->getHttpSpan($transaction)->getData(), $event->getBreadcrumbs()[0]->getMetadata()]; + } + + private function getHttpSpan(Transaction $transaction): Span + { + $recorder = $transaction->getSpanRecorder(); + $this->assertNotNull($recorder); + $spans = $recorder->getSpans(); + $this->assertCount(2, $spans); + + return $spans[1]; + } + public static function traceDataProvider(): iterable { yield [ @@ -423,8 +566,11 @@ public static function traceDataProvider(): iterable ]; yield [ - new Request('GET', 'https://user:password@www.example.com?query=string#fragment=1'), - new Response(), + new Request('GET', 'https://user:password@www.example.com?query=string#fragment=1', [ + 'Authorization' => 'Bearer secret', + 'Cookie' => 'theme=dark', + ]), + new Response(200, ['Content-Type' => 'application/json', 'Set-Cookie' => 'theme=light']), [ 'url' => 'https://www.example.com', 'http.request.method' => 'GET', From 1f68f377bf22aae41ce224f55fcfd3f56b318dbc Mon Sep 17 00:00:00 2001 From: Martin Linzmayer Date: Mon, 21 Sep 2026 13:28:32 +0200 Subject: [PATCH 2/4] feat(pii): apply data collection to guzzle middleware --- src/Tracing/GuzzleTracingMiddleware.php | 58 +++++++++++++++---- tests/Tracing/GuzzleTracingMiddlewareTest.php | 52 +++++++++++++++++ 2 files changed, 99 insertions(+), 11 deletions(-) diff --git a/src/Tracing/GuzzleTracingMiddleware.php b/src/Tracing/GuzzleTracingMiddleware.php index 5eafd3c5d..7f29a0633 100644 --- a/src/Tracing/GuzzleTracingMiddleware.php +++ b/src/Tracing/GuzzleTracingMiddleware.php @@ -10,8 +10,10 @@ use Psr\Http\Message\ResponseInterface; use Sentry\Breadcrumb; use Sentry\DataCollection\DataCollectionPolicy; -use Sentry\DataCollection\HttpSpanDataCollector; +use Sentry\DataCollection\HttpCookieCollector; +use Sentry\DataCollection\HttpHeaderCollector; use Sentry\DataCollection\HttpUrlCollector; +use Sentry\DataCollection\KeyValueDataFilter; use Sentry\Options; use Sentry\SentrySdk; use Sentry\State\HubInterface; @@ -47,7 +49,11 @@ public static function trace(?HubInterface $hub = null): \Closure 'http.request.body.size' => $request->getBody()->getSize(), ]; - $spanAndBreadcrumbData += HttpSpanDataCollector::collectQueryData($policy, $requestUri->getQuery()); + $queryString = HttpUrlCollector::collectQueryString($policy, $requestUri->getQuery()); + if ($queryString !== null) { + $spanAndBreadcrumbData['http.query'] = $queryString; + } + if ($requestUri->getFragment() !== '') { $spanAndBreadcrumbData['http.fragment'] = $requestUri->getFragment(); } @@ -61,12 +67,27 @@ public static function trace(?HubInterface $hub = null): \Closure $childSpan = null; if ($parentSpan !== null && $parentSpan->getSampled()) { + $spanData = $spanAndBreadcrumbData; + $dataCollection = $policy->getDataCollection(); + if ($dataCollection !== null) { + $headers = KeyValueDataFilter::filterHeaders($request->getHeaders(), $dataCollection->getHttpHeaders()['request']); + foreach ($headers ?? [] as $name => $value) { + $spanData['http.request.header.' . strtolower($name)] = $value; + } + $cookies = HttpCookieCollector::collectPsr7Request($dataCollection, $request); + if (\is_array($cookies)) { + /** @mago-ignore analysis:mixed-assignment */ + foreach ($cookies as $name => $value) { + $spanData['http.request.header.cookie.' . $name] = $value; + } + } elseif ($cookies !== null) { + $spanData['http.request.header.cookie'] = $cookies; + } + } + $spanContext = new SpanContext(); $spanContext->setOp('http.client'); - $spanContext->setData(array_merge( - $spanAndBreadcrumbData, - HttpSpanDataCollector::collectPsr7RequestData($policy, $request) - )); + $spanContext->setData($spanData); $spanContext->setOrigin('auto.http.guzzle'); $spanContext->setDescription($request->getMethod() . ' ' . $partialUri); @@ -120,11 +141,26 @@ public static function trace(?HubInterface $hub = null): \Closure if ($childSpan !== null) { if ($response instanceof ResponseInterface) { - $spanData = array_merge( - $spanAndBreadcrumbData, - HttpSpanDataCollector::collectPsr7ResponseData($policy, $response) - ); - if (!$policy->isLegacyMode()) { + $spanData = $spanAndBreadcrumbData; + $dataCollection = $policy->getDataCollection(); + if ($dataCollection !== null) { + $headers = KeyValueDataFilter::filterHeaders($response->getHeaders(), $dataCollection->getHttpHeaders()['response']); + foreach ($headers ?? [] as $name => $values) { + if ($values !== []) { + $spanData['http.response.header.' . strtolower((string) $name)] = $values; + } + } + + $cookies = HttpCookieCollector::collectPsr7Response($dataCollection, $response); + if (\is_array($cookies)) { + /** @mago-ignore analysis:mixed-assignment */ + foreach ($cookies as $name => $value) { + $spanData['http.response.header.set_cookie.' . $name] = $value; + } + } elseif ($cookies !== null) { + $spanData['http.response.header.set_cookie'] = $cookies; + } + $spanData = array_merge($spanData, $childSpan->getData()); } diff --git a/tests/Tracing/GuzzleTracingMiddlewareTest.php b/tests/Tracing/GuzzleTracingMiddlewareTest.php index 026c1625e..2923aa365 100644 --- a/tests/Tracing/GuzzleTracingMiddlewareTest.php +++ b/tests/Tracing/GuzzleTracingMiddlewareTest.php @@ -434,6 +434,58 @@ public function testTraceCollectsConfiguredMetadata(): void ], $breadcrumbData); } + public function testTraceCollectsCookiesWhenHeadersAreDisabled(): void + { + [$spanData] = $this->traceExchange( + ['data_collection' => ['http_headers' => ['mode' => 'off']]], + new Request('GET', 'https://www.example.com', [ + 'Authorization' => 'Bearer secret', + 'Cookie' => 'theme=dark', + ]), + new Response(200, [ + 'Content-Type' => 'application/json', + 'Set-Cookie' => 'theme=light; Path=/', + ]) + ); + + $this->assertSame('dark', $spanData['http.request.header.cookie.theme']); + $this->assertSame('light', $spanData['http.response.header.set_cookie.theme']); + $this->assertArrayNotHasKey('http.request.header.authorization', $spanData); + $this->assertArrayNotHasKey('http.response.header.content-type', $spanData); + } + + public function testTraceUsesSeparateRequestAndResponseHeaderRules(): void + { + [$spanData] = $this->traceExchange( + ['data_collection' => [ + 'http_headers' => [ + 'request' => ['mode' => 'allowList', 'terms' => ['x-request-id']], + 'response' => ['mode' => 'allowList', 'terms' => ['x-response-id']], + ], + 'cookies' => ['mode' => 'off'], + ]], + new Request('GET', 'https://www.example.com', [ + 'X-Request-ID' => ['request-id', 'second-request-id'], + 'X-Response-ID' => 'request-value', + 'Cookie' => 'theme=dark', + ]), + new Response(200, [ + 'X-Request-ID' => 'response-value', + 'X-Response-ID' => ['response-id', 'second-response-id'], + 'Set-Cookie' => 'theme=light', + ]) + ); + + $this->assertSame(['request-id', 'second-request-id'], $spanData['http.request.header.x-request-id']); + $this->assertSame(['[Filtered]'], $spanData['http.request.header.x-response-id']); + $this->assertSame(['[Filtered]'], $spanData['http.response.header.x-request-id']); + $this->assertSame(['response-id', 'second-response-id'], $spanData['http.response.header.x-response-id']); + $this->assertArrayNotHasKey('http.request.header.cookie', $spanData); + $this->assertArrayNotHasKey('http.request.header.cookie.theme', $spanData); + $this->assertArrayNotHasKey('http.response.header.set-cookie', $spanData); + $this->assertArrayNotHasKey('http.response.header.set_cookie.theme', $spanData); + } + public function testTraceUsesConfiguredQueryFiltering(): void { [$spanData, $breadcrumbData] = $this->traceExchange( From c46251a243e04a18fe310b3bbb5ce10b30f1151a Mon Sep 17 00:00:00 2001 From: Martin Linzmayer Date: Tue, 22 Sep 2026 17:31:56 +0200 Subject: [PATCH 3/4] CS --- src/Tracing/GuzzleTracingMiddleware.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Tracing/GuzzleTracingMiddleware.php b/src/Tracing/GuzzleTracingMiddleware.php index 7f29a0633..1e3dcb525 100644 --- a/src/Tracing/GuzzleTracingMiddleware.php +++ b/src/Tracing/GuzzleTracingMiddleware.php @@ -11,7 +11,6 @@ use Sentry\Breadcrumb; use Sentry\DataCollection\DataCollectionPolicy; use Sentry\DataCollection\HttpCookieCollector; -use Sentry\DataCollection\HttpHeaderCollector; use Sentry\DataCollection\HttpUrlCollector; use Sentry\DataCollection\KeyValueDataFilter; use Sentry\Options; From 2002d9e9d736b00a82354ad16a47c8893495940f Mon Sep 17 00:00:00 2001 From: Martin Linzmayer Date: Wed, 23 Sep 2026 13:53:43 +0200 Subject: [PATCH 4/4] refactor --- src/Tracing/GuzzleTracingMiddleware.php | 87 +++++++++---------- tests/Tracing/GuzzleTracingMiddlewareTest.php | 37 +++++++- 2 files changed, 78 insertions(+), 46 deletions(-) diff --git a/src/Tracing/GuzzleTracingMiddleware.php b/src/Tracing/GuzzleTracingMiddleware.php index 1e3dcb525..bd3c65d89 100644 --- a/src/Tracing/GuzzleTracingMiddleware.php +++ b/src/Tracing/GuzzleTracingMiddleware.php @@ -11,8 +11,9 @@ use Sentry\Breadcrumb; use Sentry\DataCollection\DataCollectionPolicy; use Sentry\DataCollection\HttpCookieCollector; +use Sentry\DataCollection\HttpHeaderCollector; +use Sentry\DataCollection\HttpMessageType; use Sentry\DataCollection\HttpUrlCollector; -use Sentry\DataCollection\KeyValueDataFilter; use Sentry\Options; use Sentry\SentrySdk; use Sentry\State\HubInterface; @@ -57,32 +58,18 @@ public static function trace(?HubInterface $hub = null): \Closure $spanAndBreadcrumbData['http.fragment'] = $requestUri->getFragment(); } - $collectedUrl = (string) $partialUri; - if (!$policy->isLegacyMode()) { - $collectedUrl = HttpUrlCollector::collect($policy, (string) $requestUri); - $spanAndBreadcrumbData['url.full'] = $collectedUrl; + $fullUrl = HttpUrlCollector::collect($policy, HttpMessageType::outgoingRequest(), $requestUri); + if ($fullUrl !== null) { + $spanAndBreadcrumbData['url.full'] = $fullUrl; } + $breadcrumbUrl = $fullUrl ?? (string) $partialUri; $childSpan = null; if ($parentSpan !== null && $parentSpan->getSampled()) { $spanData = $spanAndBreadcrumbData; - $dataCollection = $policy->getDataCollection(); - if ($dataCollection !== null) { - $headers = KeyValueDataFilter::filterHeaders($request->getHeaders(), $dataCollection->getHttpHeaders()['request']); - foreach ($headers ?? [] as $name => $value) { - $spanData['http.request.header.' . strtolower($name)] = $value; - } - $cookies = HttpCookieCollector::collectPsr7Request($dataCollection, $request); - if (\is_array($cookies)) { - /** @mago-ignore analysis:mixed-assignment */ - foreach ($cookies as $name => $value) { - $spanData['http.request.header.cookie.' . $name] = $value; - } - } elseif ($cookies !== null) { - $spanData['http.request.header.cookie'] = $cookies; - } - } + self::addHeaderData($spanData, 'http.request.header', HttpHeaderCollector::collect($policy, HttpMessageType::outgoingRequest(), $request->getHeaders())); + self::addCookieData($spanData, 'http.request.header.cookie', HttpCookieCollector::collectPsr7Request($policy, HttpMessageType::outgoingRequest(), $request)); $spanContext = new SpanContext(); $spanContext->setOp('http.client'); @@ -107,7 +94,7 @@ public static function trace(?HubInterface $hub = null): \Closure } } - $handlerPromiseCallback = static function ($responseOrException) use ($hub, $spanAndBreadcrumbData, $childSpan, $parentSpan, $collectedUrl, $policy) { + $handlerPromiseCallback = static function ($responseOrException) use ($hub, $spanAndBreadcrumbData, $childSpan, $parentSpan, $breadcrumbUrl, $policy) { 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 @@ -141,30 +128,11 @@ public static function trace(?HubInterface $hub = null): \Closure if ($childSpan !== null) { if ($response instanceof ResponseInterface) { $spanData = $spanAndBreadcrumbData; - $dataCollection = $policy->getDataCollection(); - if ($dataCollection !== null) { - $headers = KeyValueDataFilter::filterHeaders($response->getHeaders(), $dataCollection->getHttpHeaders()['response']); - foreach ($headers ?? [] as $name => $values) { - if ($values !== []) { - $spanData['http.response.header.' . strtolower((string) $name)] = $values; - } - } - - $cookies = HttpCookieCollector::collectPsr7Response($dataCollection, $response); - if (\is_array($cookies)) { - /** @mago-ignore analysis:mixed-assignment */ - foreach ($cookies as $name => $value) { - $spanData['http.response.header.set_cookie.' . $name] = $value; - } - } elseif ($cookies !== null) { - $spanData['http.response.header.set_cookie'] = $cookies; - } - - $spanData = array_merge($spanData, $childSpan->getData()); - } + self::addHeaderData($spanData, 'http.response.header', HttpHeaderCollector::collect($policy, HttpMessageType::incomingResponse(), $response->getHeaders())); + self::addCookieData($spanData, 'http.response.header.set_cookie', HttpCookieCollector::collectPsr7Response($policy, HttpMessageType::incomingResponse(), $response)); $childSpan->setStatus(SpanStatus::createFromHttpStatusCode($response->getStatusCode())); - $childSpan->setData($spanData); + $childSpan->setData(array_merge($spanData, $childSpan->getData())); } else { $childSpan->setStatus(SpanStatus::internalError()); } @@ -176,7 +144,7 @@ public static function trace(?HubInterface $hub = null): \Closure 'http', null, array_merge([ - 'url' => $collectedUrl, + 'url' => $breadcrumbUrl, ], $spanAndBreadcrumbData) )); @@ -192,6 +160,35 @@ public static function trace(?HubInterface $hub = null): \Closure }; } + /** + * @param array $data + * @param array|null $headers + */ + private static function addHeaderData(array &$data, string $prefix, ?array $headers): void + { + foreach ($headers ?? [] as $name => $values) { + $data[$prefix . '.' . strtolower((string) $name)] = $values; + } + } + + /** + * @param array $data + * @param array|string|null $cookies Cookies grouped by name, or `[Filtered]` if they could not be parsed + */ + private static function addCookieData(array &$data, string $prefix, $cookies): void + { + if (\is_string($cookies)) { + $data[$prefix] = $cookies; + + return; + } + + /** @mago-ignore analysis:mixed-assignment */ + foreach ($cookies ?? [] as $name => $value) { + $data[$prefix . '.' . $name] = $value; + } + } + private static function shouldAttachTracingHeaders(?Options $options, RequestInterface $request): bool { if ($options === null) { diff --git a/tests/Tracing/GuzzleTracingMiddlewareTest.php b/tests/Tracing/GuzzleTracingMiddlewareTest.php index 2923aa365..8c2ad955c 100644 --- a/tests/Tracing/GuzzleTracingMiddlewareTest.php +++ b/tests/Tracing/GuzzleTracingMiddlewareTest.php @@ -412,7 +412,7 @@ public function testTraceCollectsConfiguredMetadata(): void ]) ); - $this->assertSame('https://www.example.com/path?search=hello%20world&password=[Filtered]', $spanData['url.full']); + $this->assertSame('https://[Filtered]:[Filtered]@www.example.com/path?search=hello%20world&password=[Filtered]#fragment', $spanData['url.full']); $this->assertSame('search=hello%20world&password=[Filtered]', $spanData['http.query']); $this->assertSame(['[Filtered]'], $spanData['http.request.header.authorization']); $this->assertSame(['application/json'], $spanData['http.response.header.content-type']); @@ -527,6 +527,41 @@ public function testTraceRespectsDisabledMetadataCollection(): void $this->assertSame(array_merge(['url' => 'https://www.example.com'], $expected), $breadcrumbData); } + public function testTraceSupportsNumericHeaderNames(): void + { + [$spanData] = $this->traceExchange( + ['data_collection' => []], + new Request('GET', 'https://www.example.com/', ['123' => 'request']), + new Response(200, ['456' => 'response']) + ); + + $this->assertSame(['request'], $spanData['http.request.header.123']); + $this->assertSame(['response'], $spanData['http.response.header.456']); + } + + public function testTracePreservesExplicitSpanDataInLegacyMode(): void + { + $client = $this->createMock(ClientInterface::class); + $client->method('getOptions')->willReturn(new Options(['traces_sample_rate' => 1])); + $hub = new Hub($client); + $transaction = $hub->startTransaction(new TransactionContext()); + $hub->setSpan($transaction); + $function = (GuzzleTracingMiddleware::trace($hub))(function () use ($hub): PromiseInterface { + $span = $hub->getSpan(); + $this->assertNotNull($span); + $span->setData(['http.query' => 'explicit']); + + return new FulfilledPromise(new Response(200)); + }); + + $function(new Request('GET', 'https://www.example.com/?token=secret'), [])->wait(); + + $data = $this->getHttpSpan($transaction)->getData(); + $this->assertSame('explicit', $data['http.query']); + $this->assertSame(200, $data['http.response.status_code']); + $this->assertArrayNotHasKey('url.full', $data); + } + public function testTracePreservesExplicitSpanData(): void { $client = $this->createMock(ClientInterface::class);