Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cache.hit to Redis span data #819

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 47 additions & 10 deletions src/Sentry/Laravel/Features/CacheIntegration.php
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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']);
Expand All @@ -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
Expand Down Expand Up @@ -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;
}
}
55 changes: 55 additions & 0 deletions test/Sentry/Features/CacheIntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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';
}
};
}
}
Loading