Add support for HTTP Client baggage propagation (#675)

DAcodedBEAT · cleptric · web-flow · commit 38f70b70e8e2 · 2022-11-24T10:48:49.000+01:00
Co-authored-by: Michi Hoffmann &lt;cleptric@users.noreply.github.com&gt;
diff --git 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
@@ -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
@@ -23,6 +23,7 @@
     <xsd:complexType name="options">
         <xsd:sequence>
             <xsd:element name="integration" type="xsd:string" minOccurs="0" maxOccurs="unbounded" />
+            <xsd:element name="trace-propagation-target" type="xsd:string" minOccurs="0" maxOccurs="unbounded" />
             <xsd:element name="prefix" type="xsd:string" minOccurs="0" maxOccurs="unbounded" />
             <xsd:element name="tag" type="tag" minOccurs="0" maxOccurs="unbounded" />
             <xsd:element name="in-app-exclude" type="xsd:string" minOccurs="0" maxOccurs="unbounded" />
diff --git 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
@@ -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
@@ -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
@@ -36,6 +36,7 @@
                         max-request-body-size="none"
         >
             <sentry:integration>App\Sentry\Integration\FooIntegration</sentry:integration>
+            <sentry:trace-propagation-target>website.invalid</sentry:trace-propagation-target>
             <sentry:prefix>%kernel.project_dir%</sentry:prefix>
             <sentry:tag name="context">development</sentry:tag>
             <sentry:in-app-exclude>%kernel.cache_dir%</sentry:in-app-exclude>
diff --git 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
@@ -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
@@ -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());

Original file line number	Diff line number	Diff line change
`@@ -36,6 +36,7 @@`
`36`	`36`	`max-request-body-size="none"`
`37`	`37`	`>`
`38`	`38`	`<sentry:integration>App\Sentry\Integration\FooIntegration</sentry:integration>`
	`39`	`+ <sentry:trace-propagation-target>website.invalid</sentry:trace-propagation-target>`
`39`	`40`	`<sentry:prefix>%kernel.project_dir%</sentry:prefix>`
`40`	`41`	`<sentry:tag name="context">development</sentry:tag>`
`41`	`42`	`<sentry:in-app-exclude>%kernel.cache_dir%</sentry:in-app-exclude>`