Skip to content

Commit d30f9d7

Browse files
committed
Fix cache integration span data merging and improve session handling in tests
1 parent 95ddd2d commit d30f9d7

File tree

2 files changed

+30
-25
lines changed

2 files changed

+30
-25
lines changed

src/Sentry/Laravel/Features/CacheIntegration.php

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -225,9 +225,10 @@ private function maybeHandleCacheEventAsEndOfSpan(Events\CacheEvent $event): boo
225225
$finishedSpan = $this->maybeFinishSpan(SpanStatus::ok());
226226

227227
if ($finishedSpan !== null && count($finishedSpan->getData()['cache.key'] ?? []) === 1) {
228-
$finishedSpan->setData([
229-
'cache.hit' => $event instanceof Events\CacheHit,
230-
]);
228+
$finishedSpan->setData(array_merge(
229+
$finishedSpan->getData(),
230+
['cache.hit' => $event instanceof Events\CacheHit]
231+
));
231232
}
232233

233234
return true;
@@ -240,9 +241,10 @@ private function maybeHandleCacheEventAsEndOfSpan(Events\CacheEvent $event): boo
240241
);
241242

242243
if ($finishedSpan !== null) {
243-
$finishedSpan->setData([
244-
'cache.success' => $event instanceof Events\KeyWritten,
245-
]);
244+
$finishedSpan->setData(array_merge(
245+
$finishedSpan->getData(),
246+
['cache.success' => $event instanceof Events\KeyWritten]
247+
));
246248
}
247249

248250
return true;
@@ -314,9 +316,9 @@ private function replaceSessionKey(string $key): string
314316
/**
315317
* Replace session keys in an array of keys with placeholders.
316318
*
317-
* @param array $keys
319+
* @param string[] $keys
318320
*
319-
* @return array
321+
* @return string[]
320322
*/
321323
private function replaceSessionKeys(array $keys): array
322324
{

test/Sentry/Features/CacheIntegrationTest.php

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@
99

1010
class CacheIntegrationTest extends TestCase
1111
{
12+
protected $defaultSetupConfig = [
13+
'session.driver' => 'array',
14+
];
15+
1216
public function testCacheBreadcrumbForWriteAndHitIsRecorded(): void
1317
{
1418
Cache::put($key = 'foo', 'bar');
@@ -53,10 +57,10 @@ public function testCacheBreadcrumbIsNotRecordedWhenDisabled(): void
5357

5458
public function testCacheBreadcrumbReplacesSessionKeyWithPlaceholder(): void
5559
{
56-
// Start a session
57-
$this->app['request']->setLaravelSession($session = $this->app['session.store']);
58-
$session->start();
59-
$sessionId = $session->getId();
60+
// Start a session properly in the test environment
61+
$this->withSession(['test' => 'value']);
62+
63+
$sessionId = $this->app['session']->getId();
6064

6165
// Use the session ID as a cache key
6266
Cache::put($sessionId, 'session-data');
@@ -178,10 +182,10 @@ public function testCacheSpanReplacesSessionKeyWithPlaceholder(): void
178182
{
179183
$this->markSkippedIfTracingEventsNotAvailable();
180184

181-
// Start a session
182-
$this->app['request']->setLaravelSession($session = $this->app['session.store']);
183-
$session->start();
184-
$sessionId = $session->getId();
185+
// Start a session properly in the test environment
186+
$this->withSession(['test' => 'value']);
187+
188+
$sessionId = $this->app['session']->getId();
185189

186190
$span = $this->executeAndReturnMostRecentSpan(function () use ($sessionId) {
187191
Cache::get($sessionId);
@@ -196,10 +200,10 @@ public function testCacheSpanReplacesMultipleSessionKeysWithPlaceholder(): void
196200
{
197201
$this->markSkippedIfTracingEventsNotAvailable();
198202

199-
// Start a session
200-
$this->app['request']->setLaravelSession($session = $this->app['session.store']);
201-
$session->start();
202-
$sessionId = $session->getId();
203+
// Start a session properly in the test environment
204+
$this->withSession(['test' => 'value']);
205+
206+
$sessionId = $this->app['session']->getId();
203207

204208
$span = $this->executeAndReturnMostRecentSpan(function () use ($sessionId) {
205209
Cache::get([$sessionId, 'regular-key', $sessionId . '_another']);
@@ -214,15 +218,14 @@ public function testCacheOperationDoesNotStartSessionPrematurely(): void
214218
{
215219
$this->markSkippedIfTracingEventsNotAvailable();
216220

217-
// Make sure session is not started
218-
$this->assertFalse($this->app['session.store']->isStarted());
221+
// Don't start a session to ensure it's not started
219222

220223
$span = $this->executeAndReturnMostRecentSpan(function () {
221224
Cache::get('some-key');
222225
});
223-
224-
// Session should still not be started
225-
$this->assertFalse($this->app['session.store']->isStarted());
226+
227+
// Check that session was not started
228+
$this->assertFalse($this->app['session']->isStarted());
226229

227230
// And the key should not be replaced
228231
$this->assertEquals('some-key', $span->getDescription());

0 commit comments

Comments
 (0)