From 0ec586053ddec16f58301e87bc8efa7fcb42a026 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Thu, 22 May 2025 11:30:27 +0200 Subject: [PATCH 1/3] Implement `strict_trace_propagation` --- src/Tracing/PropagationContext.php | 29 ++++++++++++++++++-- src/Tracing/TransactionContext.php | 31 +++++++++++++++++++-- tests/FunctionsTest.php | 43 ++++++++++++++++++++++++++++-- 3 files changed, 97 insertions(+), 6 deletions(-) diff --git a/src/Tracing/PropagationContext.php b/src/Tracing/PropagationContext.php index cb150fe64..8112d9710 100644 --- a/src/Tracing/PropagationContext.php +++ b/src/Tracing/PropagationContext.php @@ -188,6 +188,33 @@ private static function parseTraceparentAndBaggage(string $traceparent, string $ { $context = self::fromDefaults(); $hasSentryTrace = false; + $samplingContext = DynamicSamplingContext::fromHeader($baggage); + + $client = SentrySdk::getCurrentHub()->getClient(); + if ($client !== null) { + $options = $client->getOptions(); + $orgId = $options->getOrgId() ?? $options->getDsn()->getOrgId(); + + // Always check if the received org ID in the baggages matches + // the one configured in the SDK. + if ( + $orgId !== null && + $samplingContext->has('org_id') && + $samplingContext->get('org_id') !== (string) $orgId + ) { + // We do not continue the trace. + return $context; + } + + if ($client->getOptions()->isStrictTracePropagationEnabled()) { + if (!$samplingContext->has('org_id')) { + // We did not receive any org id in the baggage, + // do not continue the trace. The none mathcing org ID + // case was already handled above. + return $context; + } + } + } if (preg_match(self::SENTRY_TRACEPARENT_HEADER_REGEX, $traceparent, $matches)) { if (!empty($matches['trace_id'])) { @@ -206,8 +233,6 @@ private static function parseTraceparentAndBaggage(string $traceparent, string $ } } - $samplingContext = DynamicSamplingContext::fromHeader($baggage); - if ($hasSentryTrace && !$samplingContext->hasEntries()) { // The request comes from an old SDK which does not support Dynamic Sampling. // Propagate the Dynamic Sampling Context as is, but frozen, even without sentry-* entries. diff --git a/src/Tracing/TransactionContext.php b/src/Tracing/TransactionContext.php index e7774a3f0..fb099a262 100644 --- a/src/Tracing/TransactionContext.php +++ b/src/Tracing/TransactionContext.php @@ -4,6 +4,8 @@ namespace Sentry\Tracing; +use Sentry\SentrySdk; + final class TransactionContext extends SpanContext { private const SENTRY_TRACEPARENT_HEADER_REGEX = '/^[ \\t]*(?[0-9a-f]{32})?-?(?[0-9a-f]{16})?-?(?[01])?[ \\t]*$/i'; @@ -148,6 +150,33 @@ private static function parseTraceAndBaggage(string $sentryTrace, string $baggag { $context = new self(); $hasSentryTrace = false; + $samplingContext = DynamicSamplingContext::fromHeader($baggage); + + $client = SentrySdk::getCurrentHub()->getClient(); + if ($client !== null) { + $options = $client->getOptions(); + $orgId = $options->getOrgId() ?? $options->getDsn()->getOrgId(); + + // Always check if the received org ID in the baggages matches + // the one configured in the SDK. + if ( + $orgId !== null && + $samplingContext->has('org_id') && + $samplingContext->get('org_id') !== (string) $orgId + ) { + // We do not continue the trace. + return $context; + } + + if ($client->getOptions()->isStrictTracePropagationEnabled()) { + if (!$samplingContext->has('org_id')) { + // We did not receive any org id in the baggage, + // do not continue the trace. The none mathcing org ID + // case was already handled above. + return $context; + } + } + } if (preg_match(self::SENTRY_TRACEPARENT_HEADER_REGEX, $sentryTrace, $matches)) { if (!empty($matches['trace_id'])) { @@ -166,8 +195,6 @@ private static function parseTraceAndBaggage(string $sentryTrace, string $baggag } } - $samplingContext = DynamicSamplingContext::fromHeader($baggage); - if ($hasSentryTrace && !$samplingContext->hasEntries()) { // The request comes from an old SDK which does not support Dynamic Sampling. // Propagate the Dynamic Sampling Context as is, but frozen, even without sentry-* entries. diff --git a/tests/FunctionsTest.php b/tests/FunctionsTest.php index 949f9fadd..64d4be834 100644 --- a/tests/FunctionsTest.php +++ b/tests/FunctionsTest.php @@ -514,13 +514,20 @@ public function testBaggageWithTracingEnabled(): void public function testContinueTrace(): void { - $hub = new Hub(); + $client = $this->createMock(ClientInterface::class); + $client->expects($this->atLeastOnce()) + ->method('getOptions') + ->willReturn(new Options([ + 'org_id' => 1, + ])); + + $hub = new Hub($client); SentrySdk::setCurrentHub($hub); $transactionContext = continueTrace( '566e3688a61d4bc888951642d6f14a19-566e3688a61d4bc8-1', - 'sentry-trace_id=566e3688a61d4bc888951642d6f14a19' + 'sentry-trace_id=566e3688a61d4bc888951642d6f14a19,sentry-org_id=1' ); $this->assertSame('566e3688a61d4bc888951642d6f14a19', (string) $transactionContext->getTraceId()); @@ -539,4 +546,36 @@ public function testContinueTrace(): void $this->assertTrue($dynamicSamplingContext->isFrozen()); }); } + + public function testContinueTraceWithNoneMatchingOrgId(): void + { + $client = $this->createMock(ClientInterface::class); + $client->expects($this->atLeastOnce()) + ->method('getOptions') + ->willReturn(new Options([ + 'org_id' => 1, + ])); + + $hub = new Hub($client); + + SentrySdk::setCurrentHub($hub); + + $transactionContext = continueTrace( + '566e3688a61d4bc888951642d6f14a19-566e3688a61d4bc8-1', + 'sentry-trace_id=566e3688a61d4bc888951642d6f14a19,sentry-org_id=2' + ); + + $this->assertNull($transactionContext->getTraceId()); + $this->assertNull($transactionContext->getParentSpanId()); + $this->assertNull($transactionContext->getParentSampled()); + + configureScope(function (Scope $scope): void { + $propagationContext = $scope->getPropagationContext(); + + $this->assertNotSame('566e3688a61d4bc888951642d6f14a19', (string) $propagationContext->getTraceId()); + $this->assertNotSame('566e3688a61d4bc8', (string) $propagationContext->getParentSpanId()); + + $this->assertNull($propagationContext->getDynamicSamplingContext()); + }); + } } From 268e72ddd1c362a36c6dfb89b4556464c4f53b06 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Thu, 22 May 2025 11:59:25 +0200 Subject: [PATCH 2/3] wip --- src/Tracing/PropagationContext.php | 6 +++--- src/Tracing/TransactionContext.php | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Tracing/PropagationContext.php b/src/Tracing/PropagationContext.php index 8112d9710..22db5f32b 100644 --- a/src/Tracing/PropagationContext.php +++ b/src/Tracing/PropagationContext.php @@ -198,9 +198,9 @@ private static function parseTraceparentAndBaggage(string $traceparent, string $ // Always check if the received org ID in the baggages matches // the one configured in the SDK. if ( - $orgId !== null && - $samplingContext->has('org_id') && - $samplingContext->get('org_id') !== (string) $orgId + $orgId !== null + && $samplingContext->has('org_id') + && $samplingContext->get('org_id') !== (string) $orgId ) { // We do not continue the trace. return $context; diff --git a/src/Tracing/TransactionContext.php b/src/Tracing/TransactionContext.php index fb099a262..cdd820578 100644 --- a/src/Tracing/TransactionContext.php +++ b/src/Tracing/TransactionContext.php @@ -160,9 +160,9 @@ private static function parseTraceAndBaggage(string $sentryTrace, string $baggag // Always check if the received org ID in the baggages matches // the one configured in the SDK. if ( - $orgId !== null && - $samplingContext->has('org_id') && - $samplingContext->get('org_id') !== (string) $orgId + $orgId !== null + && $samplingContext->has('org_id') + && $samplingContext->get('org_id') !== (string) $orgId ) { // We do not continue the trace. return $context; From 61bd2a9b5ac907b56843888da6c545a65950b2c2 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Thu, 22 May 2025 12:06:35 +0200 Subject: [PATCH 3/3] wip --- src/Tracing/PropagationContext.php | 2 +- src/Tracing/TransactionContext.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Tracing/PropagationContext.php b/src/Tracing/PropagationContext.php index 22db5f32b..e9d36c4c4 100644 --- a/src/Tracing/PropagationContext.php +++ b/src/Tracing/PropagationContext.php @@ -209,7 +209,7 @@ private static function parseTraceparentAndBaggage(string $traceparent, string $ if ($client->getOptions()->isStrictTracePropagationEnabled()) { if (!$samplingContext->has('org_id')) { // We did not receive any org id in the baggage, - // do not continue the trace. The none mathcing org ID + // do not continue the trace. The none matching org ID // case was already handled above. return $context; } diff --git a/src/Tracing/TransactionContext.php b/src/Tracing/TransactionContext.php index cdd820578..c6fd251c5 100644 --- a/src/Tracing/TransactionContext.php +++ b/src/Tracing/TransactionContext.php @@ -171,7 +171,7 @@ private static function parseTraceAndBaggage(string $sentryTrace, string $baggag if ($client->getOptions()->isStrictTracePropagationEnabled()) { if (!$samplingContext->has('org_id')) { // We did not receive any org id in the baggage, - // do not continue the trace. The none mathcing org ID + // do not continue the trace. The none matching org ID // case was already handled above. return $context; }