diff --git a/src/Tracing/Traits/TraceHeaderParserTrait.php b/src/Tracing/Traits/TraceHeaderParserTrait.php index 1a5c0f7c0..1b1c228e4 100644 --- a/src/Tracing/Traits/TraceHeaderParserTrait.php +++ b/src/Tracing/Traits/TraceHeaderParserTrait.php @@ -92,10 +92,11 @@ protected static function parseTraceAndBaggageHeaders(string $sentryTrace, strin } // Store the propagated trace sample rand or generate a new one - if ($samplingContext->has('sample_rand')) { - $result['sampleRand'] = (float) $samplingContext->get('sample_rand'); - } else { - if ($samplingContext->has('sample_rate') && $result['parentSampled'] !== null) { + if ($hasSentryTrace) { + $incomingSampleRand = self::parseSampleRand($samplingContext); + if ($incomingSampleRand !== null) { + $result['sampleRand'] = $incomingSampleRand; + } elseif ($samplingContext->has('sample_rate') && $result['parentSampled'] !== null) { if ($result['parentSampled'] === true) { // [0, rate) $result['sampleRand'] = round(mt_rand(0, mt_getrandmax() - 1) / mt_getrandmax() * (float) $samplingContext->get('sample_rate'), 6); @@ -112,6 +113,32 @@ protected static function parseTraceAndBaggageHeaders(string $sentryTrace, strin return $result; } + private static function parseSampleRand(DynamicSamplingContext $samplingContext): ?float + { + $sampleRand = $samplingContext->get('sample_rand'); + if ($sampleRand === null) { + return null; + } + + if (is_numeric($sampleRand)) { + $sampleRandAsFloat = (float) $sampleRand; + if ($sampleRandAsFloat >= 0.0 && $sampleRandAsFloat < 1.0) { + return $sampleRandAsFloat; + } + } + + $hub = SentrySdk::getCurrentHub(); + $client = $hub->getClient(); + if ($client !== null) { + $client->getOptions()->getLoggerOrNullLogger()->debug( + 'Ignoring invalid sentry-sample_rand baggage value because it must be a numeric value in the range [0, 1).', + ['sample_rand' => $sampleRand] + ); + } + + return null; + } + private static function shouldContinueTrace(DynamicSamplingContext $samplingContext): bool { $hub = SentrySdk::getCurrentHub(); diff --git a/tests/State/HubTest.php b/tests/State/HubTest.php index 19ec34c48..edd8a5987 100644 --- a/tests/State/HubTest.php +++ b/tests/State/HubTest.php @@ -760,6 +760,17 @@ public static function startTransactionDataProvider(): iterable false, ]; + yield 'Invalid incoming sample_rand is ignored' => [ + new Options([ + 'traces_sample_rate' => 1.0, + ]), + TransactionContext::fromHeaders( + '566e3688a61d4bc888951642d6f14a19-566e3688a61d4bc8', + 'sentry-sample_rand=2.0' + ), + true, + ]; + yield 'Out of range sample rate returned from traces_sampler (lower than minimum)' => [ new Options([ 'traces_sampler' => static function (): float { diff --git a/tests/Tracing/PropagationContextTest.php b/tests/Tracing/PropagationContextTest.php index a63db80ae..beefa0cb1 100644 --- a/tests/Tracing/PropagationContextTest.php +++ b/tests/Tracing/PropagationContextTest.php @@ -136,6 +136,41 @@ public function testGetTraceContext(): void ], $propagationContext->getTraceContext()); } + /** + * @dataProvider invalidSampleRandDataProvider + */ + public function testInvalidSampleRandIsIgnored(string $sampleRand): void + { + $propagationContext = PropagationContext::fromHeaders( + '566e3688a61d4bc888951642d6f14a19-566e3688a61d4bc8-1', + 'sentry-sample_rate=0.4,sentry-sample_rand=' . rawurlencode($sampleRand) + ); + + $generatedSampleRand = $propagationContext->getSampleRand(); + + $this->assertNotNull($generatedSampleRand); + $this->assertGreaterThanOrEqual(0.0, $generatedSampleRand); + $this->assertLessThan(0.4, $generatedSampleRand); + } + + public function testSampleRandIsIgnoredWithoutSentryTraceHeader(): void + { + $propagationContext = PropagationContext::fromHeaders('', 'sentry-sample_rand=-1.0'); + $sampleRand = $propagationContext->getSampleRand(); + + $this->assertNotNull($sampleRand); + $this->assertGreaterThanOrEqual(0.0, $sampleRand); + $this->assertLessThanOrEqual(1.0, $sampleRand); + } + + public static function invalidSampleRandDataProvider(): iterable + { + yield ['-1.0']; + yield ['1']; + yield ['2.0']; + yield ['foo']; + } + public function testSampleRandRangeWhenParentNotSampledAndSampleRateProvided(): void { $propagationContext = PropagationContext::fromHeaders( diff --git a/tests/Tracing/TransactionContextTest.php b/tests/Tracing/TransactionContextTest.php index 709bc60ab..542dae623 100644 --- a/tests/Tracing/TransactionContextTest.php +++ b/tests/Tracing/TransactionContextTest.php @@ -5,6 +5,11 @@ namespace Sentry\Tests\Tracing; use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; +use Sentry\ClientInterface; +use Sentry\Options; +use Sentry\SentrySdk; +use Sentry\State\Hub; use Sentry\Tracing\DynamicSamplingContext; use Sentry\Tracing\SpanId; use Sentry\Tracing\TraceId; @@ -135,6 +140,67 @@ public static function tracingDataProvider(): iterable ]; } + /** + * @dataProvider invalidSampleRandDataProvider + */ + public function testInvalidSampleRandIsIgnored(string $sampleRand): void + { + $context = TransactionContext::fromHeaders( + '566e3688a61d4bc888951642d6f14a19-566e3688a61d4bc8-1', + 'sentry-sample_rate=0.4,sentry-sample_rand=' . rawurlencode($sampleRand) + ); + + $generatedSampleRand = $context->getMetadata()->getSampleRand(); + + $this->assertNotNull($generatedSampleRand); + $this->assertGreaterThanOrEqual(0.0, $generatedSampleRand); + $this->assertLessThan(0.4, $generatedSampleRand); + } + + public function testSampleRandIsIgnoredWithoutSentryTraceHeader(): void + { + $context = TransactionContext::fromHeaders('', 'sentry-sample_rand=-1.0'); + $sampleRand = $context->getMetadata()->getSampleRand(); + + $this->assertNotNull($sampleRand); + $this->assertGreaterThanOrEqual(0.0, $sampleRand); + $this->assertLessThanOrEqual(1.0, $sampleRand); + } + + public function testInvalidSampleRandIsLogged(): void + { + $logger = $this->createMock(LoggerInterface::class); + $logger->expects($this->once()) + ->method('debug') + ->with( + $this->stringContains('Ignoring invalid sentry-sample_rand baggage value'), + ['sample_rand' => '-1.0'] + ); + + $client = $this->createMock(ClientInterface::class); + $client->method('getOptions') + ->willReturn(new Options(['logger' => $logger])); + + SentrySdk::setCurrentHub(new Hub($client)); + + try { + TransactionContext::fromHeaders( + '566e3688a61d4bc888951642d6f14a19-566e3688a61d4bc8', + 'sentry-sample_rand=-1.0' + ); + } finally { + SentrySdk::setCurrentHub(new Hub()); + } + } + + public static function invalidSampleRandDataProvider(): iterable + { + yield ['-1.0']; + yield ['1']; + yield ['2.0']; + yield ['foo']; + } + public function testSampleRandRangeWhenParentNotSampledAndSampleRateProvided(): void { $context = TransactionContext::fromHeaders(