Skip to content

Commit c34bdc2

Browse files
LitarnusLitarnus
authored
feat: add strict_trace_continuation support (#2016)
1 parent e086e51 commit c34bdc2

9 files changed

Lines changed: 483 additions & 36 deletions

‎phpstan-baseline.neon‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -260,11 +260,6 @@ parameters:
260260
count: 1
261261
path: src/Options.php
262262

263-
-
264-
message: "#^Method Sentry\\\\Options\\:\\:isStrictTracePropagationEnabled\\(\\) should return bool but returns mixed\\.$#"
265-
count: 1
266-
path: src/Options.php
267-
268263
-
269264
message: "#^Method Sentry\\\\Options\\:\\:shouldAttachMetricCodeLocations\\(\\) should return bool but returns mixed\\.$#"
270265
count: 1

‎src/Options.php‎

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,12 @@ public function __construct(array $options = [])
6161

6262
$this->configureOptions($this->resolver);
6363

64+
// Migrate `strict_trace_propagation` over to `strict_trace_continuation` if not set.
65+
// If both are set, then `strict_trace_continuation` will take precedence.
66+
if (isset($options['strict_trace_propagation']) && !isset($options['strict_trace_continuation'])) {
67+
$options['strict_trace_continuation'] = $options['strict_trace_propagation'];
68+
}
69+
6470
$this->options = $this->resolver->resolve($options);
6571

6672
if ($this->options['enable_tracing'] === true && $this->options['traces_sample_rate'] === null) {
@@ -775,25 +781,50 @@ public function setTracePropagationTargets(array $tracePropagationTargets): self
775781
}
776782

777783
/**
778-
* Returns whether strict trace propagation is enabled or not.
784+
* Returns whether strict trace continuation is enabled or not.
779785
*/
780-
public function isStrictTracePropagationEnabled(): bool
786+
public function isStrictTraceContinuationEnabled(): bool
781787
{
782-
return $this->options['strict_trace_propagation'];
788+
/**
789+
* @var bool $result
790+
*/
791+
$result = $this->options['strict_trace_continuation'];
792+
793+
return $result;
783794
}
784795

785796
/**
786-
* Sets if strict trace propagation should be enabled or not.
797+
* Sets if strict trace continuation should be enabled or not.
787798
*/
788-
public function enableStrictTracePropagation(bool $strictTracePropagation): self
799+
public function enableStrictTraceContinuation(bool $strictTraceContinuation): self
789800
{
790-
$options = array_merge($this->options, ['strict_trace_propagation' => $strictTracePropagation]);
801+
$options = array_merge($this->options, ['strict_trace_continuation' => $strictTraceContinuation]);
791802

792803
$this->options = $this->resolver->resolve($options);
793804

794805
return $this;
795806
}
796807

808+
/**
809+
* Returns whether strict trace propagation is enabled or not.
810+
*
811+
* @deprecated since version 4.21. To be removed in version 5.0. Use `isStrictTraceContinuationEnabled` instead.
812+
*/
813+
public function isStrictTracePropagationEnabled(): bool
814+
{
815+
return $this->isStrictTraceContinuationEnabled();
816+
}
817+
818+
/**
819+
* Sets if strict trace propagation should be enabled or not.
820+
*
821+
* @deprecated since version 4.21. To be removed in version 5.0. Use `enableStrictTraceContinuation` instead.
822+
*/
823+
public function enableStrictTracePropagation(bool $strictTracePropagation): self
824+
{
825+
return $this->enableStrictTraceContinuation($strictTracePropagation);
826+
}
827+
797828
/**
798829
* Gets a list of default tags for events.
799830
*
@@ -1352,6 +1383,7 @@ private function configureOptions(OptionsResolver $resolver): void
13521383
return $metric;
13531384
},
13541385
'trace_propagation_targets' => null,
1386+
'strict_trace_continuation' => false,
13551387
'strict_trace_propagation' => false,
13561388
'tags' => [],
13571389
'error_types' => null,
@@ -1406,6 +1438,7 @@ private function configureOptions(OptionsResolver $resolver): void
14061438
$resolver->setAllowedTypes('ignore_exceptions', 'string[]');
14071439
$resolver->setAllowedTypes('ignore_transactions', 'string[]');
14081440
$resolver->setAllowedTypes('trace_propagation_targets', ['null', 'string[]']);
1441+
$resolver->setAllowedTypes('strict_trace_continuation', 'bool');
14091442
$resolver->setAllowedTypes('strict_trace_propagation', 'bool');
14101443
$resolver->setAllowedTypes('tags', 'string[]');
14111444
$resolver->setAllowedTypes('error_types', ['null', 'int']);

‎src/Tracing/DynamicSamplingContext.php‎

Lines changed: 13 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -167,22 +167,7 @@ public static function fromTransaction(Transaction $transaction, HubInterface $h
167167
$client = $hub->getClient();
168168

169169
if ($client !== null) {
170-
$options = $client->getOptions();
171-
172-
if ($options->getDsn() !== null && $options->getDsn()->getPublicKey() !== null) {
173-
$samplingContext->set('public_key', $options->getDsn()->getPublicKey());
174-
}
175-
if ($options->getDsn() !== null && $options->getDsn()->getOrgId() !== null) {
176-
$samplingContext->set('org_id', (string) $options->getDsn()->getOrgId());
177-
}
178-
179-
if ($options->getRelease() !== null) {
180-
$samplingContext->set('release', $options->getRelease());
181-
}
182-
183-
if ($options->getEnvironment() !== null) {
184-
$samplingContext->set('environment', $options->getEnvironment());
185-
}
170+
self::setOrgOptions($client->getOptions(), $samplingContext);
186171
}
187172

188173
if ($transaction->getSampled() !== null) {
@@ -208,11 +193,22 @@ public static function fromOptions(Options $options, Scope $scope): self
208193
$samplingContext->set('sample_rate', (string) $options->getTracesSampleRate());
209194
}
210195

196+
self::setOrgOptions($options, $samplingContext);
197+
198+
$samplingContext->freeze();
199+
200+
return $samplingContext;
201+
}
202+
203+
private static function setOrgOptions(Options $options, DynamicSamplingContext $samplingContext): void
204+
{
211205
if ($options->getDsn() !== null && $options->getDsn()->getPublicKey() !== null) {
212206
$samplingContext->set('public_key', $options->getDsn()->getPublicKey());
213207
}
214208

215-
if ($options->getDsn() !== null && $options->getDsn()->getOrgId() !== null) {
209+
if ($options->getOrgId() !== null) {
210+
$samplingContext->set('org_id', (string) $options->getOrgId());
211+
} elseif ($options->getDsn() !== null && $options->getDsn()->getOrgId() !== null) {
216212
$samplingContext->set('org_id', (string) $options->getDsn()->getOrgId());
217213
}
218214

@@ -223,10 +219,6 @@ public static function fromOptions(Options $options, Scope $scope): self
223219
if ($options->getEnvironment() !== null) {
224220
$samplingContext->set('environment', $options->getEnvironment());
225221
}
226-
227-
$samplingContext->freeze();
228-
229-
return $samplingContext;
230222
}
231223

232224
/**

‎src/Tracing/Traits/TraceHeaderParserTrait.php‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
namespace Sentry\Tracing\Traits;
66

7+
use Sentry\SentrySdk;
78
use Sentry\Tracing\DynamicSamplingContext;
89
use Sentry\Tracing\SpanId;
910
use Sentry\Tracing\TraceId;
@@ -65,6 +66,14 @@ protected static function parseTraceAndBaggageHeaders(string $sentryTrace, strin
6566

6667
$samplingContext = DynamicSamplingContext::fromHeader($baggage);
6768

69+
if ($hasSentryTrace && !self::shouldContinueTrace($samplingContext)) {
70+
$result['traceId'] = null;
71+
$result['parentSpanId'] = null;
72+
$result['parentSampled'] = null;
73+
74+
return $result;
75+
}
76+
6877
if ($hasSentryTrace) {
6978
// The request comes from an old SDK which does not support Dynamic Sampling.
7079
// Propagate the Dynamic Sampling Context as is, but frozen, even without sentry-* entries.
@@ -102,4 +111,51 @@ protected static function parseTraceAndBaggageHeaders(string $sentryTrace, strin
102111

103112
return $result;
104113
}
114+
115+
private static function shouldContinueTrace(DynamicSamplingContext $samplingContext): bool
116+
{
117+
$hub = SentrySdk::getCurrentHub();
118+
$client = $hub->getClient();
119+
120+
if ($client === null) {
121+
return true;
122+
}
123+
124+
$options = $client->getOptions();
125+
$clientOrgId = $options->getOrgId();
126+
if ($clientOrgId === null && $options->getDsn() !== null) {
127+
$clientOrgId = $options->getDsn()->getOrgId();
128+
}
129+
130+
$baggageOrgId = $samplingContext->get('org_id');
131+
$logger = $options->getLoggerOrNullLogger();
132+
133+
// both org IDs are set but are not equals
134+
if ($clientOrgId !== null && $baggageOrgId !== null && ((string) $clientOrgId !== $baggageOrgId)) {
135+
$logger->debug(
136+
\sprintf(
137+
"Starting a new trace because org IDs don't match (incoming baggage org_id: %s, SDK org_id: %s)",
138+
$baggageOrgId,
139+
$clientOrgId
140+
)
141+
);
142+
143+
return false;
144+
}
145+
146+
// One org ID is not set and strict trace continuation is enabled
147+
if ($options->isStrictTraceContinuationEnabled() && ($clientOrgId === null) !== ($baggageOrgId === null)) {
148+
$logger->debug(
149+
\sprintf(
150+
'Starting a new trace because strict trace continuation is enabled and one org ID is missing (incoming baggage org_id: %s, SDK org_id: %s)',
151+
$baggageOrgId !== null ? $baggageOrgId : 'none',
152+
$clientOrgId !== null ? (string) $clientOrgId : 'none'
153+
)
154+
);
155+
156+
return false;
157+
}
158+
159+
return true;
160+
}
105161
}

‎src/functions.php‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@
6262
* server_name?: string,
6363
* spotlight?: bool,
6464
* spotlight_url?: string,
65-
* strict_trace_propagation?: bool,
65+
* strict_trace_continuation?: bool,
6666
* tags?: array<string>,
6767
* trace_propagation_targets?: array<string>|null,
6868
* traces_sample_rate?: float|int|null,
@@ -389,13 +389,32 @@ function getBaggage(): string
389389
*/
390390
function continueTrace(string $sentryTrace, string $baggage): TransactionContext
391391
{
392+
// With the new `strict_trace_continuation`, it's possible that we start two new
393+
// traces if we parse the TransactionContext and PropagationContext from the same
394+
// headers. To make sure the trace is the same, we will create one transaction
395+
// context from headers and copy relevant information over.
396+
$transactionContext = TransactionContext::fromHeaders($sentryTrace, $baggage);
397+
$propagationContext = PropagationContext::fromDefaults();
398+
$metadata = $transactionContext->getMetadata();
399+
400+
$traceId = $transactionContext->getTraceId() ?? $propagationContext->getTraceId();
401+
$transactionContext->setTraceId($traceId);
402+
$propagationContext->setTraceId($traceId);
403+
404+
$propagationContext->setParentSpanId($transactionContext->getParentSpanId());
405+
$propagationContext->setSampleRand($metadata->getSampleRand());
406+
407+
$dynamicSamplingContext = $metadata->getDynamicSamplingContext();
408+
if ($dynamicSamplingContext !== null) {
409+
$propagationContext->setDynamicSamplingContext($dynamicSamplingContext);
410+
}
411+
392412
$hub = SentrySdk::getCurrentHub();
393-
$hub->configureScope(static function (Scope $scope) use ($sentryTrace, $baggage) {
394-
$propagationContext = PropagationContext::fromHeaders($sentryTrace, $baggage);
413+
$hub->configureScope(static function (Scope $scope) use ($propagationContext): void {
395414
$scope->setPropagationContext($propagationContext);
396415
});
397416

398-
return TransactionContext::fromHeaders($sentryTrace, $baggage);
417+
return $transactionContext;
399418
}
400419

401420
/**

‎tests/FunctionsTest.php‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -637,4 +637,77 @@ public function testContinueTrace(): void
637637
$this->assertTrue($dynamicSamplingContext->isFrozen());
638638
});
639639
}
640+
641+
public function testContinueTraceWhenOrgMismatch(): void
642+
{
643+
$client = $this->createMock(ClientInterface::class);
644+
$client->expects($this->once())
645+
->method('getOptions')
646+
->willReturn(new Options([
647+
'strict_trace_continuation' => true,
648+
'org_id' => 1,
649+
]));
650+
651+
$hub = new Hub($client);
652+
SentrySdk::setCurrentHub($hub);
653+
654+
$transactionContext = continueTrace(
655+
'566e3688a61d4bc888951642d6f14a19-566e3688a61d4bc8-1',
656+
'sentry-org_id=2'
657+
);
658+
659+
$newTraceId = (string) $transactionContext->getTraceId();
660+
$newSampleRand = $transactionContext->getMetadata()->getSampleRand();
661+
662+
$this->assertNotSame('566e3688a61d4bc888951642d6f14a19', $newTraceId);
663+
$this->assertNotEmpty($newTraceId);
664+
$this->assertNull($transactionContext->getParentSpanId());
665+
$this->assertNull($transactionContext->getParentSampled());
666+
$this->assertNull($transactionContext->getMetadata()->getDynamicSamplingContext());
667+
$this->assertNotNull($newSampleRand);
668+
669+
configureScope(function (Scope $scope) use ($newTraceId, $newSampleRand): void {
670+
$propagationContext = $scope->getPropagationContext();
671+
672+
$this->assertSame($newTraceId, (string) $propagationContext->getTraceId());
673+
$this->assertNull($propagationContext->getParentSpanId());
674+
$this->assertNull($propagationContext->getDynamicSamplingContext());
675+
$this->assertSame($newSampleRand, $propagationContext->getSampleRand());
676+
});
677+
}
678+
679+
public function testContinueTraceWhenOrgMatch(): void
680+
{
681+
$client = $this->createMock(ClientInterface::class);
682+
$client->expects($this->once())
683+
->method('getOptions')
684+
->willReturn(new Options([
685+
'strict_trace_continuation' => true,
686+
'org_id' => 1,
687+
]));
688+
689+
$hub = new Hub($client);
690+
SentrySdk::setCurrentHub($hub);
691+
692+
$transactionContext = continueTrace(
693+
'566e3688a61d4bc888951642d6f14a19-566e3688a61d4bc8-1',
694+
'sentry-org_id=1'
695+
);
696+
697+
$this->assertSame('566e3688a61d4bc888951642d6f14a19', (string) $transactionContext->getTraceId());
698+
$this->assertSame('566e3688a61d4bc8', (string) $transactionContext->getParentSpanId());
699+
$this->assertTrue($transactionContext->getParentSampled());
700+
701+
configureScope(function (Scope $scope): void {
702+
$propagationContext = $scope->getPropagationContext();
703+
704+
$this->assertSame('566e3688a61d4bc888951642d6f14a19', (string) $propagationContext->getTraceId());
705+
$this->assertSame('566e3688a61d4bc8', (string) $propagationContext->getParentSpanId());
706+
707+
$dynamicSamplingContext = $propagationContext->getDynamicSamplingContext();
708+
709+
$this->assertNotNull($dynamicSamplingContext);
710+
$this->assertSame('1', $dynamicSamplingContext->get('org_id'));
711+
});
712+
}
640713
}

‎tests/OptionsTest.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,13 @@ static function (): void {},
289289
'setTracePropagationTargets',
290290
];
291291

292+
yield [
293+
'strict_trace_continuation',
294+
true,
295+
'isStrictTraceContinuationEnabled',
296+
'enableStrictTraceContinuation',
297+
];
298+
292299
yield [
293300
'strict_trace_propagation',
294301
true,

0 commit comments

Comments
 (0)