diff --git a/src/Sentry/Laravel/Features/CacheIntegration.php b/src/Sentry/Laravel/Features/CacheIntegration.php index 06fea139..27c8aa26 100644 --- a/src/Sentry/Laravel/Features/CacheIntegration.php +++ b/src/Sentry/Laravel/Features/CacheIntegration.php @@ -17,6 +17,13 @@ class CacheIntegration extends Feature { use ResolvesEventOrigin; + /** + * The most recent Redis span that was created. + * + * @var \Sentry\Tracing\Span|null + */ + private $lastRedisSpan; + public function isApplicable(): bool { return $this->isTracingFeatureEnabled('redis_commands') @@ -25,10 +32,13 @@ public function isApplicable(): bool public function onBoot(Dispatcher $events): void { + $events->listen([ + Events\CacheHit::class, + Events\CacheMissed::class, + ], [$this, 'handleCacheEvent']); + if ($this->isBreadcrumbFeatureEnabled('cache')) { $events->listen([ - Events\CacheHit::class, - Events\CacheMissed::class, Events\KeyWritten::class, Events\KeyForgotten::class, ], [$this, 'handleCacheEvent']); @@ -54,22 +64,26 @@ public function handleCacheEvent(Events\CacheEvent $event): void break; case $event instanceof Events\CacheMissed: $message = 'Missed'; + $this->maybeUpdateLastRedisSpanCacheHitStatus(false); break; case $event instanceof Events\CacheHit: $message = 'Read'; + $this->maybeUpdateLastRedisSpanCacheHitStatus(true); break; default: // In case events are added in the future we do nothing when an unknown event is encountered return; } - Integration::addBreadcrumb(new Breadcrumb( - Breadcrumb::LEVEL_INFO, - Breadcrumb::TYPE_DEFAULT, - 'cache', - "{$message}: {$event->key}", - $event->tags ? ['tags' => $event->tags] : [] - )); + if ($this->isBreadcrumbFeatureEnabled('cache')) { + Integration::addBreadcrumb(new Breadcrumb( + Breadcrumb::LEVEL_INFO, + Breadcrumb::TYPE_DEFAULT, + 'cache', + "{$message}: {$event->key}", + $event->tags ? ['tags' => $event->tags] : [] + )); + } } public function handleRedisCommand(RedisEvents\CommandExecuted $event): void @@ -114,6 +128,29 @@ public function handleRedisCommand(RedisEvents\CommandExecuted $event): void $context->setData($data); - $parentSpan->startChild($context); + $this->lastRedisSpan = $parentSpan->startChild($context); + } + + /** + * Updates the cache hit status of the last Redis span. + * + * We assume that the last Redis span is the one that was created for the cache event. + * + * @param bool $hit Whether the cache was hit or missed + */ + private function maybeUpdateLastRedisSpanCacheHitStatus(bool $hit): void + { + if ($this->lastRedisSpan === null) { + return; + } + + $this->lastRedisSpan->setData(array_merge( + $this->lastRedisSpan->getData(), + [ + 'cache.hit' => $hit, + ] + )); + + $this->lastRedisSpan = null; } } diff --git a/test/Sentry/Features/CacheIntegrationTest.php b/test/Sentry/Features/CacheIntegrationTest.php index 2d5dcac3..f54a64e9 100644 --- a/test/Sentry/Features/CacheIntegrationTest.php +++ b/test/Sentry/Features/CacheIntegrationTest.php @@ -2,6 +2,9 @@ namespace Sentry\Laravel\Tests\Features; +use Closure; +use Illuminate\Redis\Connections\Connection; +use Illuminate\Redis\Events\CommandExecuted; use Illuminate\Support\Facades\Cache; use Sentry\Laravel\Tests\TestCase; @@ -48,4 +51,56 @@ public function testCacheBreadcrumbIsNotRecordedWhenDisabled(): void $this->assertEmpty($this->getCurrentSentryBreadcrumbs()); } + + public function testCacheMissIsRecordedForRedisCommand(): void + { + $this->resetApplicationWithConfig([ + 'sentry.tracing.redis_commands' => true, + ]); + + $connection = $this->mockRedisConnection(); + + $transaction = $this->startTransaction(); + + $key = 'foo'; + + // The `Cache::get()` method would trigger a Redis command before the `CacheHit` or `CacheMissed` event + // (these events are responsible for setting the tested `cache.hit` data on the Redis span. We will fake + // the `CommandExecuted` event before executing a `Cache::*()` method to not need a Redis server running. + + $this->dispatchLaravelEvent(new CommandExecuted('get', [$key], 0.1, $connection)); + + Cache::get($key); + + Cache::set($key, 'bar'); + + $this->dispatchLaravelEvent(new CommandExecuted('get', [$key], 0.1, $connection)); + + Cache::get($key); + + $this->dispatchLaravelEvent(new CommandExecuted('del', [$key], 0.1, $connection)); + + Cache::forget($key); + + [, $cacheMissSpan, $cacheHitSpan, $otherCommandSpan] = $transaction->getSpanRecorder()->getSpans(); + + $this->assertFalse($cacheMissSpan->getData()['cache.hit']); + $this->assertTrue($cacheHitSpan->getData()['cache.hit']); + $this->assertArrayNotHasKey('cache.hit', $otherCommandSpan->getData()); + } + + private function mockRedisConnection(): Connection + { + return new class extends Connection { + public function createSubscription($channels, Closure $callback, $method = 'subscribe') + { + // We have no need for this method in this test. + } + + public function getName() + { + return 'mock-redis-connection'; + } + }; + } }