diff --git a/STRICT_TRACE_CONTINUATION.md b/STRICT_TRACE_CONTINUATION.md new file mode 100644 index 000000000..8d8eb2074 --- /dev/null +++ b/STRICT_TRACE_CONTINUATION.md @@ -0,0 +1,122 @@ +# Strict Trace Continuation Feature + +## Overview + +The `strictTraceContinuation` feature allows the Sentry PHP SDK to control trace continuation from unknown 3rd party services. When enabled, the SDK will only continue traces from the same organization, preventing 3rd party services from affecting your trace sample rates. + +## Problem Statement + +When a 3rd party accesses your API and sets headers like `traceparent` and `tracestate`, it can trigger trace propagation on your backend even when your trace sample rate is set to 0%. This can lead to: +- Unexpected trace data from external sources +- Affected sampling rates +- Unwanted trace continuation from different organizations + +## Solution + +The `strictTraceContinuation` option validates the organization ID from incoming trace headers against your configured DSN's organization ID. If they don't match, a new trace is created instead of continuing the existing one. + +## Configuration + +### Enable Strict Trace Continuation + +```php +use Sentry\ClientBuilder; + +$client = ClientBuilder::create([ + 'dsn' => 'https://your-key@o123.ingest.sentry.io/project-id', + 'strictTraceContinuation' => true, // Enable strict validation +])->getClient(); +``` + +### Using with Options Object + +```php +use Sentry\Options; + +$options = new Options([ + 'dsn' => 'https://your-key@o123.ingest.sentry.io/project-id', +]); +$options->enableStrictTraceContinuation(true); +``` + +## How It Works + +1. **Organization ID Extraction**: The SDK extracts the organization ID from your DSN (e.g., `o123` from `o123.ingest.sentry.io`) + +2. **Baggage Header Validation**: When receiving trace headers, the SDK checks the `sentry-org_id` entry in the baggage header + +3. **Trace Decision**: + - If `strictTraceContinuation` is **disabled** (default): Continues the trace regardless of org ID + - If `strictTraceContinuation` is **enabled**: + - **Matching org IDs**: Continues the existing trace + - **Mismatched org IDs**: Creates a new trace + - **Missing org ID**: Continues the trace (backwards compatibility) + +## Example Usage + +```php +use Sentry\Tracing\TransactionContext; +use function Sentry\continueTrace; + +// Incoming headers from a request +$sentryTrace = $_SERVER['HTTP_SENTRY_TRACE'] ?? ''; +$baggage = $_SERVER['HTTP_BAGGAGE'] ?? ''; + +// Continue or create a new trace based on org ID validation +$transactionContext = continueTrace($sentryTrace, $baggage); + +// Start a transaction +$transaction = \Sentry\startTransaction($transactionContext); + +// Your application logic here... + +// Finish the transaction +$transaction->finish(); +``` + +## Behavior Examples + +### Scenario 1: Disabled (Default) +``` +strictTraceContinuation: false +Incoming org_id: 456 +Local org_id: 123 +Result: Trace continues (backwards compatible) +``` + +### Scenario 2: Enabled with Matching Org +``` +strictTraceContinuation: true +Incoming org_id: 123 +Local org_id: 123 +Result: Trace continues +``` + +### Scenario 3: Enabled with Mismatched Org +``` +strictTraceContinuation: true +Incoming org_id: 456 +Local org_id: 123 +Result: New trace created +``` + +## Implementation Details + +The feature is implemented in: +- `Options::isStrictTraceContinuationEnabled()` - Check if enabled +- `Options::enableStrictTraceContinuation()` - Enable/disable the feature +- `TransactionContext::fromHeaders()` - Validates org ID for transactions +- `PropagationContext::fromHeaders()` - Validates org ID for propagation +- `continueTrace()` - Main entry point for continuing traces + +## Compatibility + +- **Default**: Disabled (backwards compatible) +- **Minimum PHP Version**: Same as the SDK requirements +- **Sentry SaaS**: Works with org IDs in DSN format `o{orgId}.ingest.sentry.io` +- **Self-hosted**: Works if org ID is configured in the DSN + +## Related Documentation + +- [Sentry SDK Specification - strictTraceContinuation](https://develop.sentry.dev/sdk/telemetry/traces/#stricttracecontinuation) +- [GitHub Issue #1830](https://github.com/getsentry/sentry-php/issues/1830) \ No newline at end of file diff --git a/src/Options.php b/src/Options.php index ea5e04787..da532dd71 100644 --- a/src/Options.php +++ b/src/Options.php @@ -720,19 +720,24 @@ public function setTracePropagationTargets(array $tracePropagationTargets): self } /** - * Returns whether strict trace propagation is enabled or not. + * Returns whether strict trace continuation is enabled or not. + * + * When enabled, the SDK will only continue traces from the same organization + * based on the org ID in the baggage header matching the org ID from the DSN. */ - public function isStrictTracePropagationEnabled(): bool + public function isStrictTraceContinuationEnabled(): bool { - return $this->options['strict_trace_propagation']; + return $this->options['strict_trace_continuation']; } /** - * Sets if strict trace propagation should be enabled or not. + * Sets if strict trace continuation should be enabled or not. + * + * @param bool $strictTraceContinuation Whether to enable strict trace continuation */ - public function enableStrictTracePropagation(bool $strictTracePropagation): self + public function enableStrictTraceContinuation(bool $strictTraceContinuation): self { - $options = array_merge($this->options, ['strict_trace_propagation' => $strictTracePropagation]); + $options = array_merge($this->options, ['strict_trace_continuation' => $strictTraceContinuation]); $this->options = $this->resolver->resolve($options); @@ -1261,7 +1266,7 @@ private function configureOptions(OptionsResolver $resolver): void return null; }, 'trace_propagation_targets' => null, - 'strict_trace_propagation' => false, + 'strict_trace_continuation' => false, 'tags' => [], 'error_types' => null, 'max_breadcrumbs' => self::DEFAULT_MAX_BREADCRUMBS, @@ -1312,7 +1317,7 @@ private function configureOptions(OptionsResolver $resolver): void $resolver->setAllowedTypes('ignore_exceptions', 'string[]'); $resolver->setAllowedTypes('ignore_transactions', 'string[]'); $resolver->setAllowedTypes('trace_propagation_targets', ['null', 'string[]']); - $resolver->setAllowedTypes('strict_trace_propagation', 'bool'); + $resolver->setAllowedTypes('strict_trace_continuation', 'bool'); $resolver->setAllowedTypes('tags', 'string[]'); $resolver->setAllowedTypes('error_types', ['null', 'int']); $resolver->setAllowedTypes('max_breadcrumbs', 'int'); diff --git a/src/Tracing/PropagationContext.php b/src/Tracing/PropagationContext.php index 363dc71d1..52e0a8259 100644 --- a/src/Tracing/PropagationContext.php +++ b/src/Tracing/PropagationContext.php @@ -4,6 +4,7 @@ namespace Sentry\Tracing; +use Sentry\ClientInterface; use Sentry\SentrySdk; use Sentry\State\Scope; @@ -59,14 +60,14 @@ public static function fromDefaults(): self return $context; } - public static function fromHeaders(string $sentryTraceHeader, string $baggageHeader): self + public static function fromHeaders(string $sentryTraceHeader, string $baggageHeader, ?ClientInterface $client = null): self { - return self::parseTraceparentAndBaggage($sentryTraceHeader, $baggageHeader); + return self::parseTraceparentAndBaggage($sentryTraceHeader, $baggageHeader, $client); } - public static function fromEnvironment(string $sentryTrace, string $baggage): self + public static function fromEnvironment(string $sentryTrace, string $baggage, ?ClientInterface $client = null): self { - return self::parseTraceparentAndBaggage($sentryTrace, $baggage); + return self::parseTraceparentAndBaggage($sentryTrace, $baggage, $client); } /** @@ -183,8 +184,20 @@ public function setSampleRand(?float $sampleRand): self return $this; } + public function getParentSampled(): ?bool + { + return $this->parentSampled; + } + + public function setParentSampled(?bool $parentSampled): self + { + $this->parentSampled = $parentSampled; + + return $this; + } + // TODO add same logic as in TransactionContext - private static function parseTraceparentAndBaggage(string $traceparent, string $baggage): self + private static function parseTraceparentAndBaggage(string $traceparent, string $baggage, ?ClientInterface $client = null): self { $context = self::fromDefaults(); $hasSentryTrace = false; @@ -208,6 +221,25 @@ private static function parseTraceparentAndBaggage(string $traceparent, string $ $samplingContext = DynamicSamplingContext::fromHeader($baggage); + // Check for org ID mismatch - always validate when both local and remote org IDs are present + if ($client !== null && $hasSentryTrace) { + $options = $client->getOptions(); + // Get org ID from either the org_id option or the DSN + $localOrgId = $options->getOrgId(); + if ($localOrgId === null && $options->getDsn() !== null) { + $localOrgId = $options->getDsn()->getOrgId(); + } + $remoteOrgId = $samplingContext->has('org_id') ? (int) $samplingContext->get('org_id') : null; + + // If we have both a local org ID and a remote org ID, and they don't match, create a new trace + if ($localOrgId !== null && $remoteOrgId !== null && $localOrgId !== $remoteOrgId) { + // Create a new propagation context instead of continuing the existing one + $context = self::fromDefaults(); + + return $context; + } + } + 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 95ba78979..f111a50dd 100644 --- a/src/Tracing/TransactionContext.php +++ b/src/Tracing/TransactionContext.php @@ -4,6 +4,8 @@ namespace Sentry\Tracing; +use Sentry\ClientInterface; + final class TransactionContext extends SpanContext { private const SENTRY_TRACEPARENT_HEADER_REGEX = '/^[ \\t]*(?[0-9a-f]{32})?-?(?[0-9a-f]{16})?-?(?[01])?[ \\t]*$/i'; @@ -125,26 +127,28 @@ public function setSource(TransactionSource $transactionSource): self /** * Returns a context populated with the data of the given environment variables. * - * @param string $sentryTrace The sentry-trace value from the environment - * @param string $baggage The baggage header value from the environment + * @param string $sentryTrace The sentry-trace value from the environment + * @param string $baggage The baggage header value from the environment + * @param ClientInterface|null $client The client to use for validation (optional) */ - public static function fromEnvironment(string $sentryTrace, string $baggage): self + public static function fromEnvironment(string $sentryTrace, string $baggage, ?ClientInterface $client = null): self { - return self::parseTraceAndBaggage($sentryTrace, $baggage); + return self::parseTraceAndBaggage($sentryTrace, $baggage, $client); } /** * Returns a context populated with the data of the given headers. * - * @param string $sentryTraceHeader The sentry-trace header from an incoming request - * @param string $baggageHeader The baggage header from an incoming request + * @param string $sentryTraceHeader The sentry-trace header from an incoming request + * @param string $baggageHeader The baggage header from an incoming request + * @param ClientInterface|null $client The client to use for validation (optional) */ - public static function fromHeaders(string $sentryTraceHeader, string $baggageHeader): self + public static function fromHeaders(string $sentryTraceHeader, string $baggageHeader, ?ClientInterface $client = null): self { - return self::parseTraceAndBaggage($sentryTraceHeader, $baggageHeader); + return self::parseTraceAndBaggage($sentryTraceHeader, $baggageHeader, $client); } - private static function parseTraceAndBaggage(string $sentryTrace, string $baggage): self + private static function parseTraceAndBaggage(string $sentryTrace, string $baggage, ?ClientInterface $client = null): self { $context = new self(); $hasSentryTrace = false; @@ -168,6 +172,31 @@ private static function parseTraceAndBaggage(string $sentryTrace, string $baggag $samplingContext = DynamicSamplingContext::fromHeader($baggage); + // Check for org ID mismatch - always validate when both local and remote org IDs are present + if ($client !== null && $hasSentryTrace) { + $options = $client->getOptions(); + // Get org ID from either the org_id option or the DSN + $localOrgId = $options->getOrgId(); + if ($localOrgId === null && $options->getDsn() !== null) { + $localOrgId = $options->getDsn()->getOrgId(); + } + $remoteOrgId = $samplingContext->has('org_id') ? (int) $samplingContext->get('org_id') : null; + + // If we have both a local org ID and a remote org ID, and they don't match, create a new trace + if ($localOrgId !== null && $remoteOrgId !== null && $localOrgId !== $remoteOrgId) { + // Create a new trace context instead of continuing the existing one + $context = new self(); + $context->traceId = TraceId::generate(); + $context->parentSpanId = null; + $context->parentSampled = null; + + // Generate a new sample rand since we're starting a new trace + $context->getMetadata()->setSampleRand(round(mt_rand(0, mt_getrandmax() - 1) / mt_getrandmax(), 6)); + + return $context; + } + } + 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/functions.php b/src/functions.php index 23f6eb1e1..9fdf01962 100644 --- a/src/functions.php +++ b/src/functions.php @@ -356,12 +356,14 @@ function getBaggage(): string function continueTrace(string $sentryTrace, string $baggage): TransactionContext { $hub = SentrySdk::getCurrentHub(); - $hub->configureScope(function (Scope $scope) use ($sentryTrace, $baggage) { - $propagationContext = PropagationContext::fromHeaders($sentryTrace, $baggage); + $client = $hub->getClient(); + + $hub->configureScope(function (Scope $scope) use ($sentryTrace, $baggage, $client) { + $propagationContext = PropagationContext::fromHeaders($sentryTrace, $baggage, $client); $scope->setPropagationContext($propagationContext); }); - return TransactionContext::fromHeaders($sentryTrace, $baggage); + return TransactionContext::fromHeaders($sentryTrace, $baggage, $client); } /** diff --git a/tests/OptionsTest.php b/tests/OptionsTest.php index 4192781a0..8572f9fad 100644 --- a/tests/OptionsTest.php +++ b/tests/OptionsTest.php @@ -293,10 +293,10 @@ static function (): void {}, ]; yield [ - 'strict_trace_propagation', + 'strict_trace_continuation', true, - 'isStrictTracePropagationEnabled', - 'enableStrictTracePropagation', + 'isStrictTraceContinuationEnabled', + 'enableStrictTraceContinuation', ]; yield [ diff --git a/tests/Tracing/PropagationContextTest.php b/tests/Tracing/PropagationContextTest.php index 2b6c4600b..37df7dea4 100644 --- a/tests/Tracing/PropagationContextTest.php +++ b/tests/Tracing/PropagationContextTest.php @@ -5,6 +5,7 @@ namespace Sentry\Tests\Tracing; use PHPUnit\Framework\TestCase; +use Sentry\ClientInterface; use Sentry\Options; use Sentry\State\Scope; use Sentry\Tracing\DynamicSamplingContext; @@ -180,4 +181,79 @@ public static function gettersAndSettersDataProvider(): array ['getDynamicSamplingContext', 'setDynamicSamplingContext', $dynamicSamplingContext], ]; } + + /** + * Tests that org ID validation allows trace continuation when org IDs match. + */ + public function testStrictTraceContinuationWithMatchingOrgId(): void + { + $client = $this->createMock(ClientInterface::class); + $client->expects($this->once()) + ->method('getOptions') + ->willReturn(new Options([ + 'dsn' => 'https://public_key@sentry.io/project_id', + 'org_id' => 123, + ])); + + $sentryTrace = '566e3688a61d4bc888951642d6f14a19-566e3688a61d4bc8-1'; + $baggage = 'sentry-org_id=123,sentry-trace_id=566e3688a61d4bc888951642d6f14a19'; + + $context = PropagationContext::fromHeaders($sentryTrace, $baggage, $client); + + // Should continue the trace since org IDs match + $this->assertEquals(new TraceId('566e3688a61d4bc888951642d6f14a19'), $context->getTraceId()); + $this->assertEquals(new SpanId('566e3688a61d4bc8'), $context->getParentSpanId()); + $this->assertTrue($context->getParentSampled()); + } + + /** + * Tests that org ID validation creates a new trace when org IDs don't match. + */ + public function testStrictTraceContinuationWithMismatchedOrgId(): void + { + $client = $this->createMock(ClientInterface::class); + $client->expects($this->once()) + ->method('getOptions') + ->willReturn(new Options([ + 'dsn' => 'https://public_key@sentry.io/project_id', + 'org_id' => 123, + ])); + + $sentryTrace = '566e3688a61d4bc888951642d6f14a19-566e3688a61d4bc8-1'; + $baggage = 'sentry-org_id=456,sentry-trace_id=566e3688a61d4bc888951642d6f14a19'; + + $context = PropagationContext::fromHeaders($sentryTrace, $baggage, $client); + + // Should create a new trace since org IDs don't match + $this->assertNotEquals(new TraceId('566e3688a61d4bc888951642d6f14a19'), $context->getTraceId()); + $this->assertNull($context->getParentSpanId()); + $this->assertNull($context->getParentSampled()); + $this->assertNotNull($context->getSampleRand()); + } + + /** + * Tests that org ID validation happens regardless of strictTraceContinuation setting. + */ + public function testOrgIdValidationAlwaysHappens(): void + { + $client = $this->createMock(ClientInterface::class); + $client->expects($this->once()) + ->method('getOptions') + ->willReturn(new Options([ + 'dsn' => 'https://public_key@sentry.io/project_id', + 'org_id' => 123, + // strictTraceContinuation is not set (defaults to false) + ])); + + $sentryTrace = '566e3688a61d4bc888951642d6f14a19-566e3688a61d4bc8-1'; + $baggage = 'sentry-org_id=456,sentry-trace_id=566e3688a61d4bc888951642d6f14a19'; + + $context = PropagationContext::fromHeaders($sentryTrace, $baggage, $client); + + // Should create a new trace when org IDs don't match, even without strictTraceContinuation enabled + $this->assertNotEquals(new TraceId('566e3688a61d4bc888951642d6f14a19'), $context->getTraceId()); + $this->assertNull($context->getParentSpanId()); + $this->assertNull($context->getParentSampled()); + $this->assertNotNull($context->getSampleRand()); + } } diff --git a/tests/Tracing/TransactionContextTest.php b/tests/Tracing/TransactionContextTest.php index eb5e12cbc..642234fc3 100644 --- a/tests/Tracing/TransactionContextTest.php +++ b/tests/Tracing/TransactionContextTest.php @@ -5,6 +5,8 @@ namespace Sentry\Tests\Tracing; use PHPUnit\Framework\TestCase; +use Sentry\ClientInterface; +use Sentry\Options; use Sentry\Tracing\DynamicSamplingContext; use Sentry\Tracing\SpanId; use Sentry\Tracing\TraceId; @@ -119,7 +121,7 @@ public static function tracingDataProvider(): iterable yield [ '566e3688a61d4bc888951642d6f14a19-566e3688a61d4bc8-1', - 'sentry-public_key=public,sentry-trace_id=566e3688a61d4bc888951642d6f14a19,sentry-sample_rate=1', + 'sentry-environment=production,sentry-public_key=49d0f7c7e546418e8b684ff47a6c4fae,sentry-trace_id=566e3688a61d4bc888951642d6f14a19', new SpanId('566e3688a61d4bc8'), new TraceId('566e3688a61d4bc888951642d6f14a19'), true, @@ -138,6 +140,83 @@ public static function tracingDataProvider(): iterable ]; } + /** + * Tests that org ID validation allows trace continuation when org IDs match. + */ + public function testStrictTraceContinuationWithMatchingOrgId(): void + { + $client = $this->createMock(ClientInterface::class); + $client->expects($this->once()) + ->method('getOptions') + ->willReturn(new Options([ + 'strict_trace_continuation' => true, + 'dsn' => 'https://public_key@sentry.io/project_id', + 'org_id' => 123, + ])); + + $sentryTrace = '566e3688a61d4bc888951642d6f14a19-566e3688a61d4bc8-1'; + $baggage = 'sentry-org_id=123,sentry-trace_id=566e3688a61d4bc888951642d6f14a19'; + + $context = TransactionContext::fromHeaders($sentryTrace, $baggage, $client); + + // Should continue the trace since org IDs match + $this->assertEquals(new TraceId('566e3688a61d4bc888951642d6f14a19'), $context->getTraceId()); + $this->assertEquals(new SpanId('566e3688a61d4bc8'), $context->getParentSpanId()); + $this->assertTrue($context->getParentSampled()); + } + + /** + * Tests that org ID validation creates a new trace when org IDs don't match. + */ + public function testStrictTraceContinuationWithMismatchedOrgId(): void + { + $client = $this->createMock(ClientInterface::class); + $client->expects($this->once()) + ->method('getOptions') + ->willReturn(new Options([ + 'strict_trace_continuation' => true, + 'dsn' => 'https://public_key@sentry.io/project_id', + 'org_id' => 123, + ])); + + $sentryTrace = '566e3688a61d4bc888951642d6f14a19-566e3688a61d4bc8-1'; + $baggage = 'sentry-org_id=456,sentry-trace_id=566e3688a61d4bc888951642d6f14a19'; + + $context = TransactionContext::fromHeaders($sentryTrace, $baggage, $client); + + // Should create a new trace since org IDs don't match + $this->assertNotEquals(new TraceId('566e3688a61d4bc888951642d6f14a19'), $context->getTraceId()); + $this->assertNull($context->getParentSpanId()); + $this->assertNull($context->getParentSampled()); + $this->assertNotNull($context->getMetadata()->getSampleRand()); + } + + /** + * Tests that org ID validation happens regardless of strictTraceContinuation setting. + */ + public function testOrgIdValidationAlwaysHappens(): void + { + $client = $this->createMock(ClientInterface::class); + $client->expects($this->once()) + ->method('getOptions') + ->willReturn(new Options([ + 'dsn' => 'https://public_key@sentry.io/project_id', + 'org_id' => 123, + // strictTraceContinuation is not set (defaults to false) + ])); + + $sentryTrace = '566e3688a61d4bc888951642d6f14a19-566e3688a61d4bc8-1'; + $baggage = 'sentry-org_id=456,sentry-trace_id=566e3688a61d4bc888951642d6f14a19'; + + $context = TransactionContext::fromHeaders($sentryTrace, $baggage, $client); + + // Should create a new trace when org IDs don't match, even without strictTraceContinuation enabled + $this->assertNotEquals(new TraceId('566e3688a61d4bc888951642d6f14a19'), $context->getTraceId()); + $this->assertNull($context->getParentSpanId()); + $this->assertNull($context->getParentSampled()); + $this->assertNotNull($context->getMetadata()->getSampleRand()); + } + public function testSampleRandRangeWhenParentNotSampledAndSampleRateProvided(): void { $context = TransactionContext::fromHeaders(