From dbecf4e1783ab90c07bf9d33fab1a4aa64db6125 Mon Sep 17 00:00:00 2001 From: Martin Linzmayer Date: Sat, 17 Oct 2026 17:34:49 +0200 Subject: [PATCH] feat(profiling): allow profiling before the SDK inits --- analysis-baseline.toml | 2 +- src/Profiling/ContinuousProfiler.php | 74 ++++++++--- src/Profiling/Profiles.php | 9 +- src/State/Hub.php | 6 + tests/Profiling/ContinuousProfilerTest.php | 146 ++++++++++++++++++++- tests/SentrySdkExtension.php | 13 ++ tests/State/HubTest.php | 31 +++++ 7 files changed, 256 insertions(+), 25 deletions(-) diff --git a/analysis-baseline.toml b/analysis-baseline.toml index 6a9bfdb0f..14c6ad9cf 100644 --- a/analysis-baseline.toml +++ b/analysis-baseline.toml @@ -1066,7 +1066,7 @@ count = 2 file = "src/Profiling/ContinuousProfiler.php" code = "invalid-method-access" message = "Attempting to access a method on a non-object type (`unknown-ref(ExcimerProfiler)`)." -count = 5 +count = 6 [[issues]] file = "src/Profiling/ContinuousProfiler.php" diff --git a/src/Profiling/ContinuousProfiler.php b/src/Profiling/ContinuousProfiler.php index 8d7c7643a..8c90ee141 100644 --- a/src/Profiling/ContinuousProfiler.php +++ b/src/Profiling/ContinuousProfiler.php @@ -4,7 +4,9 @@ namespace Sentry\Profiling; +use Sentry\ClientInterface; use Sentry\Event; +use Sentry\Options; use Sentry\SentrySdk; use Sentry\Util\SentryUid; use Sentry\Util\TelemetryStorage; @@ -72,7 +74,7 @@ final class ContinuousProfiler private $nextFlushTimestamp = 0.0; /** - * @var bool|null + * @var bool|null Whether the profiling session is sampled, undecided while a profiler started before a client was bound */ private $sampled; @@ -112,6 +114,11 @@ public function __construct(int $maxSamplesPerChunk = self::MAX_SAMPLES_PER_CHUN $this->pendingChunks = $pendingChunks; } + /** + * The profiler is started even if the SDK is not initialized yet. Once init is called, it will keep or discard + * the samples, based on the configured options. Only for manual profiling, `trace` will start with the + * transaction anyway. + */ public function start(): void { $this->restartInForkedProcess(); @@ -121,29 +128,16 @@ public function start(): void } $client = SentrySdk::getCurrentHub()->getClient(); - if ($client === null) { - return; - } - - $options = $client->getOptions(); - $logger = $options->getLoggerOrNullLogger(); - - if ($options->getProfilesSampleRate() !== null || $options->getProfilesSampler() !== null) { - $logger->warning('Continuous profiling is disabled because transaction based profiling is configured with the "profiles_sample_rate" or "profiles_sampler" option.'); - - return; - } + $options = $client !== null ? $client->getOptions() : null; - if ($this->sampled === null) { - $this->sampled = $this->isSessionSampled($options->getProfileSessionSampleRate()); - } - - if (!$this->sampled) { + if ($options !== null && !$this->isSessionEnabled($options)) { return; } if (!\extension_loaded('excimer')) { - $logger->warning('The continuous profiler was started but is not available because the "excimer" extension is not loaded.'); + if ($options !== null) { + $options->getLoggerOrNullLogger()->warning('The continuous profiler was started but is not available because the "excimer" extension is not loaded.'); + } return; } @@ -225,6 +219,27 @@ public function stopForRuntimeContext(string $runtimeContextId): void } } + /** + * Keeps a profiler started before a client was bound running if it is used manually and the + * profiling session is sampled, otherwise its samples are discarded. + */ + public function onClientBound(ClientInterface $client): void + { + if ($this->profiler === null || $this->sampled !== null) { + return; + } + + $options = $client->getOptions(); + + if ($options->getProfileLifecycle() === 'manual' && $this->isSessionEnabled($options)) { + return; + } + + $this->profiler->stop(); + + $this->clearProfiler(); + } + public function stop(): void { $this->restartInForkedProcess(); @@ -333,7 +348,8 @@ private function completeChunk(array $entries): void $sampleCount = \count($entries); - if ($sampleCount === 0) { + // Samples collected before a client was bound are only sent once it decided to keep the session + if ($sampleCount === 0 || $this->sampled === null) { return; } @@ -393,6 +409,24 @@ private function restartInForkedProcess(): void $this->start(); } + /** + * Decides whether the profiling session is sampled, unless continuous profiling is disabled by the options. + */ + private function isSessionEnabled(Options $options): bool + { + if ($options->getProfilesSampleRate() !== null || $options->getProfilesSampler() !== null) { + $options->getLoggerOrNullLogger()->warning('Continuous profiling is disabled because transaction based profiling is configured with the "profiles_sample_rate" or "profiles_sampler" option.'); + + return false; + } + + if ($this->sampled === null) { + $this->sampled = $this->isSessionSampled($options->getProfileSessionSampleRate()); + } + + return $this->sampled; + } + private function isSessionSampled(?float $sampleRate): bool { if ($sampleRate === null || $sampleRate <= 0.0) { diff --git a/src/Profiling/Profiles.php b/src/Profiling/Profiles.php index 40f263f91..065be0e04 100644 --- a/src/Profiling/Profiles.php +++ b/src/Profiling/Profiles.php @@ -34,7 +34,8 @@ public static function getInstance(): self /** * Starts the continuous profiler if the "profile_lifecycle" option is set to "manual" - * and the profiling session is sampled. + * and the profiling session is sampled. It can be started before the SDK is initialized, + * the collected samples are then discarded if the client that is bound decides so. */ public function start(): void { @@ -76,10 +77,14 @@ public function continuousProfiler(): ContinuousProfiler return $this->continuousProfiler; } + /** + * Without a client the lifecycle is not known yet, so the profiler is controlled manually until a client + * is bound, which decides whether the collected samples are kept. + */ private function isManualLifecycle(): bool { $client = SentrySdk::getCurrentHub()->getClient(); - return $client !== null && $client->getOptions()->getProfileLifecycle() === 'manual'; + return $client === null || $client->getOptions()->getProfileLifecycle() === 'manual'; } } diff --git a/src/State/Hub.php b/src/State/Hub.php index 2d80d3e66..f8ebe1314 100644 --- a/src/State/Hub.php +++ b/src/State/Hub.php @@ -47,6 +47,10 @@ class Hub implements HubInterface public function __construct(?ClientInterface $client = null, ?Scope $scope = null) { $this->stack[] = new Layer($client, $scope ?? new Scope()); + + if ($client !== null) { + Profiles::getInstance()->continuousProfiler()->onClientBound($client); + } } /** @@ -118,6 +122,8 @@ public function bindClient(ClientInterface $client): void { $layer = $this->getStackTop(); $layer->setClient($client); + + Profiles::getInstance()->continuousProfiler()->onClientBound($client); } /** diff --git a/tests/Profiling/ContinuousProfilerTest.php b/tests/Profiling/ContinuousProfilerTest.php index 368f59a57..08bebd766 100644 --- a/tests/Profiling/ContinuousProfilerTest.php +++ b/tests/Profiling/ContinuousProfilerTest.php @@ -456,9 +456,151 @@ public function testForkedProcessDoesNotCaptureThePendingChunksOfTheParentProces $profiler->stop(); } - private function bindClient(array $options): void + /** + * @requires extension excimer + */ + public function testStartWithoutClientKeepsTheSamplesIfTheBoundClientSamplesTheSession(): void { - SentrySdk::init()->bindClient(new Client(new Options($options), StubTransport::getInstance())); + $profiler = new ContinuousProfiler(); + $profiler->start(); + + $this->assertTrue($profiler->isRunning()); + + self::busyWait(0.05); + + $bindTimestamp = microtime(true); + + $profiler->onClientBound($this->bindClient(['profile_session_sample_rate' => 1.0])); + + $this->assertTrue($profiler->isRunning()); + + self::busyWait(0.05); + + $profiler->stop(); + + $this->assertCount(1, StubTransport::$events); + + $profileChunk = StubTransport::$events[0]->getProfileChunk(); + + $this->assertNotNull($profileChunk); + + $payload = $profileChunk->getFormattedData(StubTransport::$events[0]); + + $this->assertNotNull($payload); + $this->assertLessThan($bindTimestamp, $payload['profile']['samples'][0]['timestamp']); + } + + /** + * @requires extension excimer + */ + public function testStartWithoutClientDiscardsTheSamplesIfTheBoundClientDoesNotSampleTheSession(): void + { + $profiler = new ContinuousProfiler(); + $profiler->start(); + + self::busyWait(0.05); + + $profiler->onClientBound($this->bindClient(['profile_session_sample_rate' => 0.0])); + + $this->assertFalse($profiler->isRunning()); + + $profiler->stop(); + $profiler->start(); + + $this->assertFalse($profiler->isRunning()); + $this->assertSame([], StubTransport::$events); + } + + /** + * @requires extension excimer + */ + public function testStartWithoutClientDiscardsTheSamplesIfTheBoundClientUsesTheTraceLifecycle(): void + { + $profiler = new ContinuousProfiler(); + $profiler->start(); + + self::busyWait(0.05); + + $profiler->onClientBound($this->bindClient([ + 'profile_lifecycle' => 'trace', + 'profile_session_sample_rate' => 1.0, + ])); + + $this->assertFalse($profiler->isRunning()); + $this->assertSame([], StubTransport::$events); + + $profiler->startForRootSpan('566e3688a61d4bc8'); + + $this->assertTrue($profiler->isRunning()); + + $profiler->stopForRootSpan('566e3688a61d4bc8'); + } + + /** + * @requires extension excimer + */ + public function testChunksFilledBeforeAClientIsBoundAreDropped(): void + { + $profiler = new ContinuousProfiler(5); + $profiler->start(); + + self::busyWait(0.2); + + $bindTimestamp = microtime(true); + + $profiler->onClientBound($this->bindClient(['profile_session_sample_rate' => 1.0])); + + self::busyWait(0.05); + + $profiler->stop(); + + $samplesBeforeBind = 0; + + foreach (StubTransport::$events as $event) { + $profileChunk = $event->getProfileChunk(); + + $this->assertNotNull($profileChunk); + + $payload = $profileChunk->getFormattedData($event); + + $this->assertNotNull($payload); + + foreach ($payload['profile']['samples'] as $sample) { + if ($sample['timestamp'] < $bindTimestamp) { + ++$samplesBeforeBind; + } + } + } + + // Only the samples of the chunk that was not full yet when the client was bound are kept + $this->assertLessThan(5, $samplesBeforeBind); + } + + /** + * @requires extension excimer + */ + public function testSamplesAreNotSentBeforeTheSessionIsDecided(): void + { + $profiler = new ContinuousProfiler(); + $profiler->start(); + + // The client is bound without deciding the session of this profiler + $this->bindClient(['profile_session_sample_rate' => 1.0]); + + self::busyWait(0.05); + + $profiler->stop(); + + $this->assertSame([], StubTransport::$events); + } + + private function bindClient(array $options): Client + { + $client = new Client(new Options($options), StubTransport::getInstance()); + + SentrySdk::init()->bindClient($client); + + return $client; } /** diff --git a/tests/SentrySdkExtension.php b/tests/SentrySdkExtension.php index 145e35251..6d5c86aba 100644 --- a/tests/SentrySdkExtension.php +++ b/tests/SentrySdkExtension.php @@ -6,6 +6,7 @@ use PHPUnit\Runner\BeforeTestHook as BeforeTestHookInterface; use Sentry\Integration\IntegrationRegistry; +use Sentry\Profiling\Profiles; use Sentry\SentrySdk; use Sentry\State\Scope; @@ -60,6 +61,18 @@ public function executeBeforeTest(string $test): void $reflectionProperty->setAccessible(false); } + // A profiler left running by a previous test keeps sampling, so it is stopped before the profiling session is reset + Profiles::getInstance()->continuousProfiler()->stop(); + + $reflectionProperty = new \ReflectionProperty(Profiles::class, 'instance'); + if (\PHP_VERSION_ID < 80100) { + $reflectionProperty->setAccessible(true); + } + $reflectionProperty->setValue(null, null); + if (\PHP_VERSION_ID < 80100) { + $reflectionProperty->setAccessible(false); + } + $reflectionProperty = new \ReflectionProperty(IntegrationRegistry::class, 'integrations'); if (\PHP_VERSION_ID < 80100) { $reflectionProperty->setAccessible(true); diff --git a/tests/State/HubTest.php b/tests/State/HubTest.php index 8be476291..a647b5862 100644 --- a/tests/State/HubTest.php +++ b/tests/State/HubTest.php @@ -238,6 +238,37 @@ public function testBindClient(): void $this->assertSame($client2, $hub->getClient()); } + /** + * @requires extension excimer + */ + public function testBindClientStopsAProfilerStartedWithoutClientIfTheSessionIsNotSampled(): void + { + $profiler = Profiles::getInstance()->continuousProfiler(); + $profiler->start(); + + $this->assertTrue($profiler->isRunning()); + + $hub = new Hub(); + $hub->bindClient(new Client(new Options(['profile_session_sample_rate' => 0.0]), StubTransport::getInstance())); + + $this->assertFalse($profiler->isRunning()); + } + + /** + * @requires extension excimer + */ + public function testConstructorStopsAProfilerStartedWithoutClientIfTheSessionIsNotSampled(): void + { + $profiler = Profiles::getInstance()->continuousProfiler(); + $profiler->start(); + + $this->assertTrue($profiler->isRunning()); + + new Hub(new Client(new Options(['profile_session_sample_rate' => 0.0]), StubTransport::getInstance())); + + $this->assertFalse($profiler->isRunning()); + } + /** * @dataProvider captureMessageDataProvider */