feat(profiling): allow profiling before the SDK inits

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"

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) {

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';
     }
 }

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);
     }
 
     /**

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;
     }
 
     /**

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);