Skip to content

Commit

Permalink
ref: Add proper HTTP client span descriptions (#680)
Browse files Browse the repository at this point in the history
  • Loading branch information
cleptric committed Nov 24, 2022
1 parent 38f70b7 commit 795376f
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 11 deletions.
5 changes: 3 additions & 2 deletions CHANGELOG.md
Expand Up @@ -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)

Expand Down
15 changes: 12 additions & 3 deletions src/Tracing/HttpClient/AbstractTraceableHttpClient.php
Expand Up @@ -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();
Expand All @@ -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);
Expand Down Expand Up @@ -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);
}
}
12 changes: 6 additions & 6 deletions tests/Tracing/HttpClient/TraceableHttpClientTest.php
Expand Up @@ -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());
Expand All @@ -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());
}

Expand Down Expand Up @@ -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());
}

Expand Down Expand Up @@ -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());
}

Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 795376f

Please sign in to comment.