diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c27d5ac..25ed5e7a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,10 +2,11 @@ ## Unreleased -- feat: Add support for tracing of the Symfony HTTP client requests (#606) +- feat: Add support for tracing of Symfony HTTP client requests (#606) + - feat: Add support for HTTP client baggage propagation (#663) + - ref: Add proper HTTP client span descriptions (#680) - feat: Support logging the impersonator user, if any (#647) - ref: Use constant for the SDK version (#662) -- Add support for HTTP client baggage propagation (#663) ## 4.4.0 (2022-10-20) diff --git a/src/Tracing/HttpClient/AbstractTraceableHttpClient.php b/src/Tracing/HttpClient/AbstractTraceableHttpClient.php index 651ad69a..f4a2a35e 100644 --- a/src/Tracing/HttpClient/AbstractTraceableHttpClient.php +++ b/src/Tracing/HttpClient/AbstractTraceableHttpClient.php @@ -51,11 +51,12 @@ public function request(string $method, string $url, array $options = []): Respo $headers = $options['headers'] ?? []; $headers['sentry-trace'] = $parent->toTraceparent(); + $uri = new Uri($url); + // Check if the request destination is allow listed in the trace_propagation_targets option. $client = $this->hub->getClient(); if (null !== $client) { $sdkOptions = $client->getOptions(); - $uri = new Uri($url); if (\in_array($uri->getHost(), $sdkOptions->getTracePropagationTargets())) { $headers['baggage'] = $parent->toBaggage(); @@ -64,12 +65,14 @@ public function request(string $method, string $url, array $options = []): Respo $options['headers'] = $headers; + $formattedUri = $this->formatUri($uri); + $context = new SpanContext(); $context->setOp('http.client'); - $context->setDescription('HTTP ' . $method); + $context->setDescription($method . ' ' . $formattedUri); $context->setTags([ 'http.method' => $method, - 'http.url' => $url, + 'http.url' => $formattedUri, ]); $span = $parent->startChild($context); @@ -108,4 +111,10 @@ public function setLogger(LoggerInterface $logger): void $this->client->setLogger($logger); } } + + private function formatUri(Uri $uri): string + { + // Instead of relying on Uri::__toString, we only use a sub set of the URI + return Uri::composeComponents($uri->getScheme(), $uri->getHost(), $uri->getPath(), null, null); + } } diff --git a/tests/Tracing/HttpClient/TraceableHttpClientTest.php b/tests/Tracing/HttpClient/TraceableHttpClientTest.php index 5c724713..06636d26 100644 --- a/tests/Tracing/HttpClient/TraceableHttpClientTest.php +++ b/tests/Tracing/HttpClient/TraceableHttpClientTest.php @@ -63,12 +63,12 @@ public function testRequest(): void $mockResponse = new MockResponse(); $decoratedHttpClient = new MockHttpClient($mockResponse); $httpClient = new TraceableHttpClient($decoratedHttpClient, $this->hub); - $response = $httpClient->request('GET', 'https://www.example.com/test-page'); + $response = $httpClient->request('GET', 'https://username:password@www.example.com/test-page?foo=bar#baz'); $this->assertInstanceOf(AbstractTraceableResponse::class, $response); $this->assertSame(200, $response->getStatusCode()); $this->assertSame('GET', $response->getInfo('http_method')); - $this->assertSame('https://www.example.com/test-page', $response->getInfo('url')); + $this->assertSame('https://username:password@www.example.com/test-page?foo=bar#baz', $response->getInfo('url')); $this->assertSame(['sentry-trace: ' . $transaction->toTraceparent()], $mockResponse->getRequestOptions()['normalized_headers']['sentry-trace']); $this->assertArrayNotHasKey('baggage', $mockResponse->getRequestOptions()['normalized_headers']); $this->assertNotNull($transaction->getSpanRecorder()); @@ -82,7 +82,7 @@ public function testRequest(): void $this->assertCount(2, $spans); $this->assertNull($spans[1]->getEndTimestamp()); $this->assertSame('http.client', $spans[1]->getOp()); - $this->assertSame('HTTP GET', $spans[1]->getDescription()); + $this->assertSame('GET https://www.example.com/test-page', $spans[1]->getDescription()); $this->assertSame($expectedTags, $spans[1]->getTags()); } @@ -130,7 +130,7 @@ public function testRequestDoesNotContainBaggageHeader(): void $this->assertCount(2, $spans); $this->assertNull($spans[1]->getEndTimestamp()); $this->assertSame('http.client', $spans[1]->getOp()); - $this->assertSame('HTTP PUT', $spans[1]->getDescription()); + $this->assertSame('PUT https://www.example.com/test-page', $spans[1]->getDescription()); $this->assertSame($expectedTags, $spans[1]->getTags()); } @@ -178,7 +178,7 @@ public function testRequestDoesContainBaggageHeader(): void $this->assertCount(2, $spans); $this->assertNull($spans[1]->getEndTimestamp()); $this->assertSame('http.client', $spans[1]->getOp()); - $this->assertSame('HTTP POST', $spans[1]->getDescription()); + $this->assertSame('POST https://www.example.com/test-page', $spans[1]->getDescription()); $this->assertSame($expectedTags, $spans[1]->getTags()); } @@ -214,7 +214,7 @@ public function testStream(): void $this->assertCount(2, $spans); $this->assertNotNull($spans[1]->getEndTimestamp()); $this->assertSame('http.client', $spans[1]->getOp()); - $this->assertSame('HTTP GET', $spans[1]->getDescription()); + $this->assertSame('GET https://www.example.com/test-page', $spans[1]->getDescription()); $this->assertSame($expectedTags, $spans[1]->getTags()); $loopIndex = 0;