diff --git a/src/Options.php b/src/Options.php index c6d4b002f..f8610a74b 100644 --- a/src/Options.php +++ b/src/Options.php @@ -304,6 +304,33 @@ public function setProfilesSampler(?callable $sampler): self return $this->updateOptions(['profiles_sampler' => $sampler]); } + /** + * Gets the factory used to create a profiler for each sampled transaction. + * + * @phpstan-return null|callable(Options): Profiling\ProfilerInterface + */ + public function getProfilerFactory(): ?callable + { + /** @var callable(Options): Profiling\ProfilerInterface|null $value */ + $value = $this->options['profiler_factory']; + + return $value; + } + + /** + * Sets the factory used to create a profiler for each sampled transaction. + * Return a new profiler for each transaction so concurrent profiles remain independent. + * When null, the SDK uses its default Excimer profiler. + * + * @param ?callable $factory The factory + * + * @phpstan-param null|callable(Options): Profiling\ProfilerInterface $factory + */ + public function setProfilerFactory(?callable $factory): self + { + return $this->updateOptions(['profiler_factory' => $factory]); + } + /** * Gets whether tracing is enabled or not. The feature is enabled when at * least one of the `traces_sample_rate` and `traces_sampler` options is @@ -1262,6 +1289,7 @@ private function configureOptions(OptionsResolver $resolver): void $resolver->setAllowedTypes('traces_sampler', ['null', 'callable']); $resolver->setAllowedTypes('profiles_sample_rate', ['null', 'int', 'float']); $resolver->setAllowedTypes('profiles_sampler', ['null', 'callable']); + $resolver->setAllowedTypes('profiler_factory', ['null', 'callable']); $resolver->setAllowedTypes('attach_stacktrace', 'bool'); $resolver->setAllowedTypes('attach_metric_code_locations', 'bool'); $resolver->setAllowedTypes('context_lines', ['null', 'int']); @@ -1347,6 +1375,7 @@ private function configureOptions(OptionsResolver $resolver): void 'traces_sampler' => null, 'profiles_sample_rate' => null, 'profiles_sampler' => null, + 'profiler_factory' => null, 'attach_stacktrace' => false, /** * @deprecated Metrics are no longer supported. Metrics API is a no-op and will be removed in 5.x. diff --git a/src/Profiling/Profile.php b/src/Profiling/Profile.php index 70bf5f972..9d6508f4a 100644 --- a/src/Profiling/Profile.php +++ b/src/Profiling/Profile.php @@ -73,8 +73,6 @@ * trace: array, * timestamp: float * } - * - * @internal */ final class Profile { @@ -137,6 +135,8 @@ public function setStartTimeStamp(float $startTimeStamp): void } /** + * Entry timestamps are offsets in seconds from the profile's start timestamp. + * * @param \ExcimerLog|array $excimerLog */ public function setExcimerLog($excimerLog): void @@ -144,6 +144,9 @@ public function setExcimerLog($excimerLog): void $this->excimerLog = $excimerLog; } + /** + * @internal + */ public function setEventId(EventId $eventId): void { $this->eventId = $eventId; @@ -151,6 +154,8 @@ public function setEventId(EventId $eventId): void /** * @return SentryProfile|null + * + * @internal */ public function getFormattedData(Event $event): ?array { diff --git a/src/Profiling/Profiler.php b/src/Profiling/Profiler.php index f379b53b2..f0724212f 100644 --- a/src/Profiling/Profiler.php +++ b/src/Profiling/Profiler.php @@ -11,7 +11,7 @@ /** * @internal */ -final class Profiler +final class Profiler implements ProfilerInterface { /** * @var \ExcimerProfiler|null diff --git a/src/Profiling/ProfilerInterface.php b/src/Profiling/ProfilerInterface.php new file mode 100644 index 000000000..7c4c5bdc4 --- /dev/null +++ b/src/Profiling/ProfilerInterface.php @@ -0,0 +1,14 @@ +sample($profilesSampleRate)) { $logger->info(\sprintf('Transaction [%s] started profiling because it was sampled.', (string) $transaction->getTraceId())); - $transaction->initProfiler()->start(); + try { + $transaction->initProfiler()->start(); + } catch (\Throwable $exception) { + $transaction->detachProfiler(); + $logger->error(\sprintf('Failed to start the profiler. Reason: "%s".', $exception->getMessage()), ['exception' => $exception]); + } } else { $logger->info(\sprintf('Transaction [%s] is not profiling because it was not sampled.', (string) $transaction->getTraceId())); } diff --git a/src/Tracing/Transaction.php b/src/Tracing/Transaction.php index 5e4d727d2..57676f332 100644 --- a/src/Tracing/Transaction.php +++ b/src/Tracing/Transaction.php @@ -6,7 +6,9 @@ use Sentry\Event; use Sentry\EventId; +use Sentry\Profiling\Profile; use Sentry\Profiling\Profiler; +use Sentry\Profiling\ProfilerInterface; use Sentry\SentrySdk; use Sentry\State\HubInterface; @@ -36,7 +38,7 @@ final class Transaction extends Span protected $metadata; /** - * @var Profiler|null Reference instance to the {@see Profiler} + * @var ProfilerInterface|null */ protected $profiler; @@ -119,19 +121,23 @@ public function initSpanRecorder(int $maxSpans = 1000): self return $this; } - public function initProfiler(): Profiler + public function initProfiler(): ProfilerInterface { if ($this->profiler === null) { $client = $this->hub->getClient(); $options = $client !== null ? $client->getOptions() : null; - $this->profiler = new Profiler($options); + if ($options !== null && ($factory = $options->getProfilerFactory()) !== null) { + $this->profiler = $factory($options); + } else { + $this->profiler = new Profiler($options); + } } return $this->profiler; } - public function getProfiler(): ?Profiler + public function getProfiler(): ?ProfilerInterface { return $this->profiler; } @@ -148,15 +154,13 @@ public function detachProfiler(): self */ public function finish(?float $endTimestamp = null): ?EventId { - if ($this->profiler !== null) { - $this->profiler->stop(); - } - if ($this->endTimestamp !== null) { // Transaction was already finished once and we don't want to re-flush it return null; } + $profile = $this->profiler !== null ? $this->finishProfiler($this->profiler) : null; + parent::finish($endTimestamp); if ($this->sampled !== true) { @@ -183,13 +187,26 @@ public function finish(?float $endTimestamp = null): ?EventId $event->setSdkMetadata('dynamic_sampling_context', $this->getDynamicSamplingContext()); $event->setSdkMetadata('transaction_metadata', $this->getMetadata()); - if ($this->profiler !== null) { - $profile = $this->profiler->getProfile(); - if ($profile !== null) { - $event->setSdkMetadata('profile', $profile); - } + if ($profile !== null) { + $event->setSdkMetadata('profile', $profile); } return $this->hub->captureEvent($event); } + + private function finishProfiler(ProfilerInterface $profiler): ?Profile + { + try { + $profiler->stop(); + + return $this->sampled === true ? $profiler->getProfile() : null; + } catch (\Throwable $exception) { + $client = $this->hub->getClient(); + if ($client !== null) { + $client->getOptions()->getLoggerOrNullLogger()->error(\sprintf('Failed to stop the profiler. Reason: "%s".', $exception->getMessage()), ['exception' => $exception]); + } + + return null; + } + } } diff --git a/src/functions.php b/src/functions.php index 432f00f57..7eb64d276 100644 --- a/src/functions.php +++ b/src/functions.php @@ -75,6 +75,7 @@ * max_value_length?: int, * org_id?: int|null, * prefixes?: array, + * profiler_factory?: callable|null, * profiles_sample_rate?: int|float|null, * profiles_sampler?: callable|null, * release?: string|null, diff --git a/tests/OptionsTest.php b/tests/OptionsTest.php index 050285612..2d659ed59 100644 --- a/tests/OptionsTest.php +++ b/tests/OptionsTest.php @@ -183,6 +183,13 @@ static function (): void {}, 'setProfilesSampler', ]; + yield [ + 'profiler_factory', + static function (): void {}, + 'getProfilerFactory', + 'setProfilerFactory', + ]; + yield [ 'attach_stacktrace', false, @@ -606,6 +613,7 @@ public function testDefaultOptionValues(): void 'traces_sampler' => null, 'profiles_sample_rate' => null, 'profiles_sampler' => null, + 'profiler_factory' => null, 'attach_stacktrace' => false, 'attach_metric_code_locations' => false, 'context_lines' => 5, diff --git a/tests/State/HubTest.php b/tests/State/HubTest.php index e2763aa82..76360bc61 100644 --- a/tests/State/HubTest.php +++ b/tests/State/HubTest.php @@ -17,6 +17,7 @@ use Sentry\MonitorConfig; use Sentry\MonitorSchedule; use Sentry\Options; +use Sentry\Profiling\ProfilerInterface; use Sentry\Severity; use Sentry\State\Hub; use Sentry\State\Scope; @@ -917,6 +918,61 @@ public function testStartTransactionStartsProfilerWithProfilesSampler(): void $this->assertNotNull($transaction->getProfiler()); } + public function testStartTransactionCreatesAProfilerForEachSampledTransaction(): void + { + $profilers = [ + $this->createMock(ProfilerInterface::class), + $this->createMock(ProfilerInterface::class), + ]; + foreach ($profilers as $profiler) { + $profiler->expects($this->once())->method('start'); + } + $calls = 0; + $options = new Options([ + 'traces_sample_rate' => 1.0, + 'profiles_sample_rate' => 1.0, + ]); + $options->setProfilerFactory(function (Options $factoryOptions) use ($options, $profilers, &$calls): ProfilerInterface { + $this->assertSame($options, $factoryOptions); + + return $profilers[$calls++]; + }); + $client = $this->createMock(ClientInterface::class); + $client->method('getOptions')->willReturn($options); + $hub = new Hub($client); + + foreach ($profilers as $profiler) { + $transaction = $hub->startTransaction(new TransactionContext()); + + $this->assertSame($profiler, $transaction->getProfiler()); + } + } + + public function testProfilerFactoryExceptionDropsProfileAndIsLogged(): void + { + StubLogger::$logs = []; + $exception = new \RuntimeException('test'); + $client = $this->createMock(ClientInterface::class); + $client->method('getOptions')->willReturn(new Options([ + 'logger' => StubLogger::getInstance(), + 'traces_sample_rate' => 1.0, + 'profiles_sample_rate' => 1.0, + 'profiler_factory' => static function () use ($exception): void { + throw $exception; + }, + ])); + + $transaction = (new Hub($client))->startTransaction(new TransactionContext()); + + $this->assertTrue($transaction->getSampled()); + $this->assertNull($transaction->getProfiler()); + $this->assertContains([ + 'level' => 'error', + 'message' => 'Failed to start the profiler. Reason: "test".', + 'context' => ['exception' => $exception], + ], StubLogger::$logs); + } + public function testStartTransactionDoesNotStartProfilerWhenProfilesSamplerReturnsZero(): void { $client = $this->createMock(ClientInterface::class); diff --git a/tests/Tracing/TransactionTest.php b/tests/Tracing/TransactionTest.php index 76a2c3aab..5edc29500 100644 --- a/tests/Tracing/TransactionTest.php +++ b/tests/Tracing/TransactionTest.php @@ -10,8 +10,11 @@ use Sentry\EventId; use Sentry\EventType; use Sentry\Options; +use Sentry\Profiling\Profile; +use Sentry\Profiling\ProfilerInterface; use Sentry\State\Hub; use Sentry\State\HubInterface; +use Sentry\Tests\StubLogger; use Sentry\Tests\TestUtil\ClockMock; use Sentry\Tracing\SpanContext; use Sentry\Tracing\Transaction; @@ -85,6 +88,66 @@ public function testFinishDoesNothingIfSampledFlagIsNotTrue(): void $transaction->finish(); } + public function testFinishIncludesTheCustomProfile(): void + { + $profile = new Profile(); + $profiler = $this->createMock(ProfilerInterface::class); + $profiler->expects($this->once())->method('stop'); + $profiler->expects($this->once())->method('getProfile')->willReturn($profile); + $options = new Options([ + 'profiler_factory' => static function () use ($profiler): ProfilerInterface { + return $profiler; + }, + ]); + $client = $this->createMock(ClientInterface::class); + $client->method('getOptions')->willReturn($options); + $hub = $this->createMock(HubInterface::class); + $hub->method('getClient')->willReturn($client); + $hub->expects($this->once()) + ->method('captureEvent') + ->with($this->callback(static function (Event $event) use ($profile): bool { + return $event->getSdkMetadata('profile') === $profile; + })); + + $transaction = new Transaction(TransactionContext::make()->setSampled(true), $hub); + $transaction->initProfiler(); + $transaction->finish(); + $transaction->finish(); + } + + public function testProfilerExceptionDoesNotPreventFinishingTheTransaction(): void + { + StubLogger::$logs = []; + $exception = new \RuntimeException('test'); + $profiler = $this->createMock(ProfilerInterface::class); + $profiler->expects($this->once())->method('stop')->willThrowException($exception); + $profiler->expects($this->never())->method('getProfile'); + $client = $this->createMock(ClientInterface::class); + $client->method('getOptions')->willReturn(new Options([ + 'logger' => StubLogger::getInstance(), + 'profiler_factory' => static function () use ($profiler): ProfilerInterface { + return $profiler; + }, + ])); + $hub = $this->createMock(HubInterface::class); + $hub->method('getClient')->willReturn($client); + $hub->expects($this->once()) + ->method('captureEvent') + ->with($this->callback(static function (Event $event): bool { + return $event->getSdkMetadata('profile') === null; + })); + + $transaction = new Transaction(TransactionContext::make()->setSampled(true), $hub); + $transaction->initProfiler(); + $transaction->finish(); + + $this->assertContains([ + 'level' => 'error', + 'message' => 'Failed to stop the profiler. Reason: "test".', + 'context' => ['exception' => $exception], + ], StubLogger::$logs); + } + public function testFluentApi(): void { $transaction = new Transaction(TransactionContext::make());