Skip to content

Commit 413d894

Browse files
cleptriccleptric
authored
Tracing without Performance (#742)
1 parent 216f8f0 commit 413d894

File tree

10 files changed

+202
-161
lines changed

10 files changed

+202
-161
lines changed

‎composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
"guzzlehttp/psr7": "^1.7 || ^2.0",
2929
"jean85/pretty-package-versions": "^1.5 || ^2.0",
3030
"sentry/sdk": "^3.4",
31-
"sentry/sentry": "^3.19",
31+
"sentry/sentry": "^3.20.1",
3232
"symfony/cache-contracts": "^1.1||^2.4||^3.0",
3333
"symfony/config": "^4.4.20||^5.0.11||^6.0",
3434
"symfony/console": "^4.4.20||^5.0.11||^6.0",

‎phpstan-baseline.neon

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,11 @@ parameters:
265265
count: 1
266266
path: src/Tracing/HttpClient/TraceableResponseForV6.php
267267

268+
-
269+
message: "#^Constructor of class Sentry\\\\SentryBundle\\\\Twig\\\\SentryExtension has an unused parameter \\$hub\\.$#"
270+
count: 1
271+
path: src/Twig/SentryExtension.php
272+
268273
-
269274
message: "#^Cannot access offset 'profiles_sample_rate' on mixed\\.$#"
270275
count: 1

‎src/DependencyInjection/Configuration.php

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,7 @@ public function getConfigTreeBuilder(): TreeBuilder
9191
->info('The sampling factor to apply to profiles. A value of 0 will deny sending any profiles, and a value of 1 will send all profiles. Profiles are sampled in relation to traces_sample_rate')
9292
->end()
9393
->scalarNode('traces_sampler')->end()
94-
->arrayNode('trace_propagation_targets')
95-
->scalarPrototype()->end()
96-
->beforeNormalization()->castToArray()->end()
97-
->end()
94+
->variableNode('trace_propagation_targets')->end()
9895
->booleanNode('attach_stacktrace')->end()
9996
->integerNode('context_lines')->min(0)->end()
10097
->booleanNode('enable_compression')->end()

‎src/EventListener/TracingRequestListener.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,13 @@
44

55
namespace Sentry\SentryBundle\EventListener;
66

7-
use Sentry\Tracing\Transaction;
8-
use Sentry\Tracing\TransactionContext;
97
use Sentry\Tracing\TransactionSource;
108
use Symfony\Component\HttpFoundation\Request;
119
use Symfony\Component\HttpKernel\Event\RequestEvent;
1210
use Symfony\Component\HttpKernel\Event\TerminateEvent;
1311

12+
use function Sentry\continueTrace;
13+
1414
/**
1515
* This event listener acts on the master requests and starts a transaction
1616
* to report performance data to Sentry. It gathers useful data like the
@@ -37,7 +37,7 @@ public function handleKernelRequestEvent(RequestEvent $event): void
3737
/** @var float $requestStartTime */
3838
$requestStartTime = $request->server->get('REQUEST_TIME_FLOAT', microtime(true));
3939

40-
$context = TransactionContext::fromHeaders(
40+
$context = continueTrace(
4141
$request->headers->get('sentry-trace', ''),
4242
$request->headers->get('baggage', '')
4343
);

‎src/Resources/config/services.xml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,8 +132,6 @@
132132
</service>
133133

134134
<service id="Sentry\SentryBundle\Twig\SentryExtension" class="Sentry\SentryBundle\Twig\SentryExtension">
135-
<argument type="service" id="Sentry\State\HubInterface" />
136-
137135
<tag name="twig.extension" />
138136
</service>
139137
</services>

‎src/Tracing/HttpClient/AbstractTraceableHttpClient.php

Lines changed: 63 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use GuzzleHttp\Psr7\Uri;
88
use Psr\Log\LoggerAwareInterface;
99
use Psr\Log\LoggerInterface;
10+
use Sentry\ClientInterface;
1011
use Sentry\State\HubInterface;
1112
use Sentry\Tracing\SpanContext;
1213
use Symfony\Component\HttpClient\Response\ResponseStream;
@@ -15,6 +16,9 @@
1516
use Symfony\Contracts\HttpClient\ResponseStreamInterface;
1617
use Symfony\Contracts\Service\ResetInterface;
1718

19+
use function Sentry\getBaggage;
20+
use function Sentry\getTraceparent;
21+
1822
/**
1923
* This is an implementation of the {@see HttpClientInterface} that decorates
2024
* an existing http client to support distributed tracing capabilities.
@@ -44,49 +48,52 @@ public function __construct(HttpClientInterface $client, HubInterface $hub)
4448
*/
4549
public function request(string $method, string $url, array $options = []): ResponseInterface
4650
{
47-
$span = null;
48-
$parent = $this->hub->getSpan();
49-
50-
if (null !== $parent) {
51-
$headers = $options['headers'] ?? [];
52-
$headers['sentry-trace'] = $parent->toTraceparent();
51+
$uri = new Uri($url);
52+
$headers = $options['headers'] ?? [];
5353

54-
$uri = new Uri($url);
55-
$partialUri = Uri::fromParts([
56-
'scheme' => $uri->getScheme(),
57-
'host' => $uri->getHost(),
58-
'port' => $uri->getPort(),
59-
'path' => $uri->getPath(),
60-
]);
61-
62-
// Check if the request destination is allow listed in the trace_propagation_targets option.
63-
$client = $this->hub->getClient();
64-
if (null !== $client) {
65-
$sdkOptions = $client->getOptions();
54+
$span = $this->hub->getSpan();
55+
$client = $this->hub->getClient();
6656

67-
if (\in_array($uri->getHost(), (array) $sdkOptions->getTracePropagationTargets())) {
68-
$headers['baggage'] = $parent->toBaggage();
69-
}
57+
if (null === $span) {
58+
if (self::shouldAttachTracingHeaders($client, $uri)) {
59+
$headers['baggage'] = getBaggage();
60+
$headers['sentry-trace'] = getTraceparent();
7061
}
7162

7263
$options['headers'] = $headers;
7364

74-
$context = new SpanContext();
75-
$context->setOp('http.client');
76-
$context->setDescription($method . ' ' . (string) $partialUri);
77-
$context->setTags([
78-
'http.method' => $method,
79-
'http.url' => (string) $partialUri,
80-
]);
81-
$context->setData([
82-
'http.query' => $uri->getQuery(),
83-
'http.fragment' => $uri->getFragment(),
84-
]);
85-
86-
$span = $parent->startChild($context);
65+
return new TraceableResponse($this->client, $this->client->request($method, $url, $options), $span);
8766
}
8867

89-
return new TraceableResponse($this->client, $this->client->request($method, $url, $options), $span);
68+
$partialUri = Uri::fromParts([
69+
'scheme' => $uri->getScheme(),
70+
'host' => $uri->getHost(),
71+
'port' => $uri->getPort(),
72+
'path' => $uri->getPath(),
73+
]);
74+
75+
$context = new SpanContext();
76+
$context->setOp('http.client');
77+
$context->setDescription($method . ' ' . (string) $partialUri);
78+
$context->setTags([
79+
'http.method' => $method,
80+
'http.url' => (string) $partialUri,
81+
]);
82+
$context->setData([
83+
'http.query' => $uri->getQuery(),
84+
'http.fragment' => $uri->getFragment(),
85+
]);
86+
87+
$childSpan = $span->startChild($context);
88+
89+
if (self::shouldAttachTracingHeaders($client, $uri)) {
90+
$headers['baggage'] = $childSpan->toBaggage();
91+
$headers['sentry-trace'] = $childSpan->toTraceparent();
92+
}
93+
94+
$options['headers'] = $headers;
95+
96+
return new TraceableResponse($this->client, $this->client->request($method, $url, $options), $childSpan);
9097
}
9198

9299
/**
@@ -119,4 +126,25 @@ public function setLogger(LoggerInterface $logger): void
119126
$this->client->setLogger($logger);
120127
}
121128
}
129+
130+
private static function shouldAttachTracingHeaders(?ClientInterface $client, Uri $uri): bool
131+
{
132+
if (null !== $client) {
133+
$sdkOptions = $client->getOptions();
134+
135+
// Check if the request destination is allow listed in the trace_propagation_targets option.
136+
if (
137+
null !== $sdkOptions->getTracePropagationTargets() &&
138+
// Due to BC, we treat an empty array (the default) as all hosts are allow listed
139+
(
140+
[] === $sdkOptions->getTracePropagationTargets() ||
141+
\in_array($uri->getHost(), $sdkOptions->getTracePropagationTargets())
142+
)
143+
) {
144+
return true;
145+
}
146+
}
147+
148+
return false;
149+
}
122150
}

‎src/Twig/SentryExtension.php

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,19 +8,16 @@
88
use Twig\Extension\AbstractExtension;
99
use Twig\TwigFunction;
1010

11+
use function Sentry\getBaggage;
12+
use function Sentry\getTraceparent;
13+
1114
final class SentryExtension extends AbstractExtension
1215
{
13-
/**
14-
* @var HubInterface The current hub
15-
*/
16-
private $hub;
17-
1816
/**
1917
* @param HubInterface $hub The current hub
2018
*/
21-
public function __construct(HubInterface $hub)
19+
public function __construct(HubInterface $hub = null)
2220
{
23-
$this->hub = $hub;
2421
}
2522

2623
/**
@@ -39,18 +36,14 @@ public function getFunctions(): array
3936
*/
4037
public function getTraceMeta(): string
4138
{
42-
$span = $this->hub->getSpan();
43-
44-
return sprintf('<meta name="sentry-trace" content="%s" />', null !== $span ? $span->toTraceparent() : '');
39+
return sprintf('<meta name="sentry-trace" content="%s" />', getTraceparent());
4540
}
4641

4742
/**
4843
* Returns an HTML meta tag named `baggage`.
4944
*/
5045
public function getBaggageMeta(): string
5146
{
52-
$span = $this->hub->getSpan();
53-
54-
return sprintf('<meta name="baggage" content="%s" />', null !== $span ? $span->toBaggage() : '');
47+
return sprintf('<meta name="baggage" content="%s" />', getBaggage());
5548
}
5649
}

‎tests/DependencyInjection/ConfigurationTest.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ public function testProcessConfigurationWithDefaultConfiguration(): void
2828
'options' => [
2929
'integrations' => [],
3030
'prefixes' => array_merge(['%kernel.project_dir%'], array_filter(explode(\PATH_SEPARATOR, get_include_path() ?: ''))),
31-
'trace_propagation_targets' => [],
3231
'environment' => '%kernel.environment%',
3332
'release' => PrettyVersions::getRootPackageVersion()->getPrettyVersion(),
3433
'tags' => [],

0 commit comments

Comments
 (0)