From 38f70b70e8e2d22beb5624814d61e3c2943754bf Mon Sep 17 00:00:00 2001 From: Arun Philip Date: Thu, 24 Nov 2022 04:48:49 -0500 Subject: [PATCH] Add support for HTTP Client baggage propagation (#675) Co-authored-by: Michi Hoffmann --- CHANGELOG.md | 1 + src/DependencyInjection/Configuration.php | 5 + src/Resources/config/schema/sentry-1.0.xsd | 1 + .../AbstractTraceableHttpClient.php | 13 +++ .../DependencyInjection/ConfigurationTest.php | 1 + .../DependencyInjection/Fixtures/php/full.php | 1 + .../DependencyInjection/Fixtures/xml/full.xml | 1 + .../DependencyInjection/Fixtures/yml/full.yml | 2 + .../SentryExtensionTest.php | 1 + .../HttpClient/TraceableHttpClientTest.php | 99 +++++++++++++++++++ 10 files changed, 125 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 41e37db3..5c27d5ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ - feat: Add support for tracing of the Symfony HTTP client requests (#606) - 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/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 05d06550..b1c1bdf3 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -48,6 +48,7 @@ public function getConfigTreeBuilder(): TreeBuilder ->arrayNode('options') ->addDefaultsIfNotSet() ->fixXmlConfig('integration') + ->fixXmlConfig('trace_propagation_target') ->fixXmlConfig('tag') ->fixXmlConfig('class_serializer') ->fixXmlConfig('prefix', 'prefixes') @@ -72,6 +73,10 @@ public function getConfigTreeBuilder(): TreeBuilder ->info('The sampling factor to apply to transactions. A value of 0 will deny sending any transaction, and a value of 1 will send all transactions.') ->end() ->scalarNode('traces_sampler')->end() + ->arrayNode('trace_propagation_targets') + ->scalarPrototype()->end() + ->beforeNormalization()->castToArray()->end() + ->end() ->booleanNode('attach_stacktrace')->end() ->integerNode('context_lines')->min(0)->end() ->booleanNode('enable_compression')->end() diff --git a/src/Resources/config/schema/sentry-1.0.xsd b/src/Resources/config/schema/sentry-1.0.xsd index 9240c5c6..a140d46c 100644 --- a/src/Resources/config/schema/sentry-1.0.xsd +++ b/src/Resources/config/schema/sentry-1.0.xsd @@ -23,6 +23,7 @@ + diff --git a/src/Tracing/HttpClient/AbstractTraceableHttpClient.php b/src/Tracing/HttpClient/AbstractTraceableHttpClient.php index 3ad27ad4..651ad69a 100644 --- a/src/Tracing/HttpClient/AbstractTraceableHttpClient.php +++ b/src/Tracing/HttpClient/AbstractTraceableHttpClient.php @@ -4,6 +4,7 @@ namespace Sentry\SentryBundle\Tracing\HttpClient; +use GuzzleHttp\Psr7\Uri; use Psr\Log\LoggerAwareInterface; use Psr\Log\LoggerInterface; use Sentry\State\HubInterface; @@ -49,6 +50,18 @@ public function request(string $method, string $url, array $options = []): Respo if (null !== $parent) { $headers = $options['headers'] ?? []; $headers['sentry-trace'] = $parent->toTraceparent(); + + // 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(); + } + } + $options['headers'] = $headers; $context = new SpanContext(); diff --git a/tests/DependencyInjection/ConfigurationTest.php b/tests/DependencyInjection/ConfigurationTest.php index 9b853130..76aaeaf8 100644 --- a/tests/DependencyInjection/ConfigurationTest.php +++ b/tests/DependencyInjection/ConfigurationTest.php @@ -26,6 +26,7 @@ public function testProcessConfigurationWithDefaultConfiguration(): void 'options' => [ 'integrations' => [], 'prefixes' => array_merge(['%kernel.project_dir%'], array_filter(explode(\PATH_SEPARATOR, get_include_path() ?: ''))), + 'trace_propagation_targets' => [], 'environment' => '%kernel.environment%', 'release' => PrettyVersions::getRootPackageVersion()->getPrettyVersion(), 'tags' => [], diff --git a/tests/DependencyInjection/Fixtures/php/full.php b/tests/DependencyInjection/Fixtures/php/full.php index 82c71ef8..cf52ebf9 100644 --- a/tests/DependencyInjection/Fixtures/php/full.php +++ b/tests/DependencyInjection/Fixtures/php/full.php @@ -17,6 +17,7 @@ 'sample_rate' => 1, 'traces_sample_rate' => 1, 'traces_sampler' => 'App\\Sentry\\Tracing\\TracesSampler', + 'trace_propagation_targets' => ['website.invalid'], 'attach_stacktrace' => true, 'context_lines' => 0, 'enable_compression' => true, diff --git a/tests/DependencyInjection/Fixtures/xml/full.xml b/tests/DependencyInjection/Fixtures/xml/full.xml index e9240f89..bab488bb 100644 --- a/tests/DependencyInjection/Fixtures/xml/full.xml +++ b/tests/DependencyInjection/Fixtures/xml/full.xml @@ -36,6 +36,7 @@ max-request-body-size="none" > App\Sentry\Integration\FooIntegration + website.invalid %kernel.project_dir% development %kernel.cache_dir% diff --git a/tests/DependencyInjection/Fixtures/yml/full.yml b/tests/DependencyInjection/Fixtures/yml/full.yml index e3c86290..d0799679 100644 --- a/tests/DependencyInjection/Fixtures/yml/full.yml +++ b/tests/DependencyInjection/Fixtures/yml/full.yml @@ -12,6 +12,8 @@ sentry: sample_rate: 1 traces_sample_rate: 1 traces_sampler: App\Sentry\Tracing\TracesSampler + trace_propagation_targets: + - 'website.invalid' attach_stacktrace: true context_lines: 0 enable_compression: true diff --git a/tests/DependencyInjection/SentryExtensionTest.php b/tests/DependencyInjection/SentryExtensionTest.php index 30a17193..d2055e70 100644 --- a/tests/DependencyInjection/SentryExtensionTest.php +++ b/tests/DependencyInjection/SentryExtensionTest.php @@ -202,6 +202,7 @@ public function testClientIsCreatedFromOptions(): void 'sample_rate' => 1, 'traces_sample_rate' => 1, 'traces_sampler' => new Reference('App\\Sentry\\Tracing\\TracesSampler'), + 'trace_propagation_targets' => ['website.invalid'], 'attach_stacktrace' => true, 'context_lines' => 0, 'enable_compression' => true, diff --git a/tests/Tracing/HttpClient/TraceableHttpClientTest.php b/tests/Tracing/HttpClient/TraceableHttpClientTest.php index ba9da822..5c724713 100644 --- a/tests/Tracing/HttpClient/TraceableHttpClientTest.php +++ b/tests/Tracing/HttpClient/TraceableHttpClientTest.php @@ -8,6 +8,8 @@ use PHPUnit\Framework\TestCase; use Psr\Log\LoggerAwareInterface; use Psr\Log\NullLogger; +use Sentry\ClientInterface; +use Sentry\Options; use Sentry\SentryBundle\Tracing\HttpClient\AbstractTraceableResponse; use Sentry\SentryBundle\Tracing\HttpClient\TraceableHttpClient; use Sentry\State\HubInterface; @@ -68,6 +70,7 @@ public function testRequest(): void $this->assertSame('GET', $response->getInfo('http_method')); $this->assertSame('https://www.example.com/test-page', $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()); $spans = $transaction->getSpanRecorder()->getSpans(); @@ -83,6 +86,102 @@ public function testRequest(): void $this->assertSame($expectedTags, $spans[1]->getTags()); } + public function testRequestDoesNotContainBaggageHeader(): void + { + $options = new Options([ + 'dsn' => 'http://public:secret@example.com/sentry/1', + 'trace_propagation_targets' => ['non-matching-host.invalid'], + ]); + $client = $this->createMock(ClientInterface::class); + $client + ->expects($this->once()) + ->method('getOptions') + ->willReturn($options); + + $transaction = new Transaction(new TransactionContext()); + $transaction->initSpanRecorder(); + + $this->hub->expects($this->once()) + ->method('getSpan') + ->willReturn($transaction); + $this->hub->expects($this->once()) + ->method('getClient') + ->willReturn($client); + + $mockResponse = new MockResponse(); + $decoratedHttpClient = new MockHttpClient($mockResponse); + $httpClient = new TraceableHttpClient($decoratedHttpClient, $this->hub); + $response = $httpClient->request('PUT', 'https://www.example.com/test-page'); + + $this->assertInstanceOf(AbstractTraceableResponse::class, $response); + $this->assertSame(200, $response->getStatusCode()); + $this->assertSame('PUT', $response->getInfo('http_method')); + $this->assertSame('https://www.example.com/test-page', $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()); + + $spans = $transaction->getSpanRecorder()->getSpans(); + $expectedTags = [ + 'http.method' => 'PUT', + 'http.url' => 'https://www.example.com/test-page', + ]; + + $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($expectedTags, $spans[1]->getTags()); + } + + public function testRequestDoesContainBaggageHeader(): void + { + $options = new Options([ + 'dsn' => 'http://public:secret@example.com/sentry/1', + 'trace_propagation_targets' => ['www.example.com'], + ]); + $client = $this->createMock(ClientInterface::class); + $client + ->expects($this->once()) + ->method('getOptions') + ->willReturn($options); + + $transaction = new Transaction(new TransactionContext()); + $transaction->initSpanRecorder(); + + $this->hub->expects($this->once()) + ->method('getSpan') + ->willReturn($transaction); + $this->hub->expects($this->once()) + ->method('getClient') + ->willReturn($client); + + $mockResponse = new MockResponse(); + $decoratedHttpClient = new MockHttpClient($mockResponse); + $httpClient = new TraceableHttpClient($decoratedHttpClient, $this->hub); + $response = $httpClient->request('POST', 'https://www.example.com/test-page'); + + $this->assertInstanceOf(AbstractTraceableResponse::class, $response); + $this->assertSame(200, $response->getStatusCode()); + $this->assertSame('POST', $response->getInfo('http_method')); + $this->assertSame('https://www.example.com/test-page', $response->getInfo('url')); + $this->assertSame(['sentry-trace: ' . $transaction->toTraceparent()], $mockResponse->getRequestOptions()['normalized_headers']['sentry-trace']); + $this->assertSame(['baggage: ' . $transaction->toBaggage()], $mockResponse->getRequestOptions()['normalized_headers']['baggage']); + $this->assertNotNull($transaction->getSpanRecorder()); + + $spans = $transaction->getSpanRecorder()->getSpans(); + $expectedTags = [ + 'http.method' => 'POST', + 'http.url' => 'https://www.example.com/test-page', + ]; + + $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($expectedTags, $spans[1]->getTags()); + } + public function testStream(): void { $transaction = new Transaction(new TransactionContext());