Skip to content

Commit 9f3a0d7

Browse files
stayallivecleptric
stayallive
and
cleptric
authored
Improve how we signal that a route was matched (#772)
Co-authored-by: Michi Hoffmann <cleptric@users.noreply.github.com>
1 parent a36f34b commit 9f3a0d7

21 files changed

+256
-80
lines changed

‎src/Sentry/Laravel/EventHandler.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
use Laravel\Sanctum\Events as Sanctum;
2020
use RuntimeException;
2121
use Sentry\Breadcrumb;
22+
use Sentry\Laravel\Tracing\Middleware;
2223
use Sentry\Laravel\Util\WorksWithUris;
2324
use Sentry\SentrySdk;
2425
use Sentry\State\Scope;
@@ -216,6 +217,8 @@ protected function routeMatchedHandler(RoutingEvents\RouteMatched $match): void
216217
return;
217218
}
218219

220+
Middleware::signalRouteWasMatched();
221+
219222
[$routeName] = Integration::extractNameAndSourceForRoute($match->route);
220223

221224
Integration::addBreadcrumb(new Breadcrumb(

‎src/Sentry/Laravel/Features/FolioPackageIntegration.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use Laravel\Folio\Pipeline\MatchedView;
1111
use Sentry\Breadcrumb;
1212
use Sentry\Laravel\Integration;
13+
use Sentry\Laravel\Tracing\Middleware;
1314
use Sentry\SentrySdk;
1415
use Sentry\Tracing\TransactionSource;
1516

@@ -29,6 +30,8 @@ public function onBoot(Dispatcher $events): void
2930

3031
public function handleViewMatched(ViewMatched $matched): void
3132
{
33+
Middleware::signalRouteWasMatched();
34+
3235
$routeName = $this->extractRouteForMatchedView($matched->matchedView, $matched->mountPath);
3336

3437
Integration::addBreadcrumb(new Breadcrumb(

‎src/Sentry/Laravel/Tracing/Middleware.php

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515

1616
use function Sentry\continueTrace;
1717

18+
/**
19+
* @internal
20+
*/
1821
class Middleware
1922
{
2023
/**
@@ -59,6 +62,13 @@ class Middleware
5962
*/
6063
private $registeredTerminatingCallback = false;
6164

65+
/**
66+
* Whether a defined route was matched in the application.
67+
*
68+
* @var bool
69+
*/
70+
private $didRouteMatch = false;
71+
6272
/**
6373
* Construct the Sentry tracing middleware.
6474
*
@@ -80,8 +90,8 @@ public function __construct($app, bool $continueAfterResponse = true)
8090
*/
8191
public function handle(Request $request, Closure $next)
8292
{
83-
if (app()->bound(HubInterface::class)) {
84-
$this->startTransaction($request, app(HubInterface::class));
93+
if ($this->app->bound(HubInterface::class)) {
94+
$this->startTransaction($request, $this->app->make(HubInterface::class));
8595
}
8696

8797
return $next($request);
@@ -98,12 +108,12 @@ public function handle(Request $request, Closure $next)
98108
public function terminate(Request $request, $response): void
99109
{
100110
// If there is no transaction or the HubInterface is not bound in the container there is nothing for us to do
101-
if ($this->transaction === null || !app()->bound(HubInterface::class)) {
111+
if ($this->transaction === null || !$this->app->bound(HubInterface::class)) {
102112
return;
103113
}
104114

105115
// We stop here if a route has not been matched unless we are configured to trace missing routes
106-
if (config('sentry.tracing.missing_routes', false) === false && $request->route() === null) {
116+
if (!$this->didRouteMatch && config('sentry.tracing.missing_routes', false) === false) {
107117
return;
108118
}
109119

@@ -154,6 +164,9 @@ public function setBootedTimestamp(?float $timestamp = null): void
154164

155165
private function startTransaction(Request $request, HubInterface $sentry): void
156166
{
167+
// Reset our internal state in case we are handling multiple requests (e.g. in Octane)
168+
$this->didRouteMatch = false;
169+
157170
// Try $_SERVER['REQUEST_TIME_FLOAT'] then LARAVEL_START and fallback to microtime(true) if neither are defined
158171
$requestStartTime = $request->server(
159172
'REQUEST_TIME_FLOAT',
@@ -260,4 +273,18 @@ private function finishTransaction(): void
260273
$this->transaction->finish();
261274
$this->transaction = null;
262275
}
276+
277+
private function internalSignalRouteWasMatched(): void
278+
{
279+
$this->didRouteMatch = true;
280+
}
281+
282+
public static function signalRouteWasMatched(): void
283+
{
284+
if (!app()->bound(self::class)) {
285+
return;
286+
}
287+
288+
app(self::class)->internalSignalRouteWasMatched();
289+
}
263290
}

‎test/Sentry/ClientBuilderDecoratorTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ public function testClientHasEnvironmentSetFromDecorator(): void
2121
{
2222
$this->assertEquals(
2323
'from_service_container',
24-
$this->getClientFromContainer()->getOptions()->getEnvironment()
24+
$this->getSentryClientFromContainer()->getOptions()->getEnvironment()
2525
);
2626
}
2727
}

‎test/Sentry/EventHandler/ConsoleEventsTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ public function testCommandBreadcrumbIsRecordedWhenEnabled(): void
1919

2020
$this->dispatchCommandStartEvent();
2121

22-
$lastBreadcrumb = $this->getLastBreadcrumb();
22+
$lastBreadcrumb = $this->getLastSentryBreadcrumb();
2323

2424
$this->assertEquals('Starting Artisan command: test:command', $lastBreadcrumb->getMessage());
2525
$this->assertEquals('--foo=bar', $lastBreadcrumb->getMetadata()['input']);
@@ -35,7 +35,7 @@ public function testCommandBreadcrumIsNotRecordedWhenDisabled(): void
3535

3636
$this->dispatchCommandStartEvent();
3737

38-
$this->assertEmpty($this->getCurrentBreadcrumbs());
38+
$this->assertEmpty($this->getCurrentSentryBreadcrumbs());
3939
}
4040

4141
private function dispatchCommandStartEvent(): void

‎test/Sentry/EventHandler/DatabaseEventsTest.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public function testSqlQueriesAreRecordedWhenEnabled(): void
2424
$this->getMockedConnection()
2525
));
2626

27-
$lastBreadcrumb = $this->getLastBreadcrumb();
27+
$lastBreadcrumb = $this->getLastSentryBreadcrumb();
2828

2929
$this->assertEquals($query, $lastBreadcrumb->getMessage());
3030
}
@@ -44,7 +44,7 @@ public function testSqlBindingsAreRecordedWhenEnabled(): void
4444
$this->getMockedConnection()
4545
));
4646

47-
$lastBreadcrumb = $this->getLastBreadcrumb();
47+
$lastBreadcrumb = $this->getLastSentryBreadcrumb();
4848

4949
$this->assertEquals($query, $lastBreadcrumb->getMessage());
5050
$this->assertEquals($bindings, $lastBreadcrumb->getMetadata()['bindings']);
@@ -65,7 +65,7 @@ public function testSqlQueriesAreRecordedWhenDisabled(): void
6565
$this->getMockedConnection()
6666
));
6767

68-
$this->assertEmpty($this->getCurrentBreadcrumbs());
68+
$this->assertEmpty($this->getCurrentSentryBreadcrumbs());
6969
}
7070

7171
public function testSqlBindingsAreRecordedWhenDisabled(): void
@@ -83,7 +83,7 @@ public function testSqlBindingsAreRecordedWhenDisabled(): void
8383
$this->getMockedConnection()
8484
));
8585

86-
$lastBreadcrumb = $this->getLastBreadcrumb();
86+
$lastBreadcrumb = $this->getLastSentryBreadcrumb();
8787

8888
$this->assertEquals($query, $lastBreadcrumb->getMessage());
8989
$this->assertFalse(isset($lastBreadcrumb->getMetadata()['bindings']));

‎test/Sentry/EventHandler/LogEventsTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ public function testLaravelLogsAreRecordedWhenEnabled(): void
2121
$context = ['1']
2222
));
2323

24-
$lastBreadcrumb = $this->getLastBreadcrumb();
24+
$lastBreadcrumb = $this->getLastSentryBreadcrumb();
2525

2626
$this->assertEquals($level, $lastBreadcrumb->getLevel());
2727
$this->assertEquals($message, $lastBreadcrumb->getMessage());
@@ -38,6 +38,6 @@ public function testLaravelLogsAreRecordedWhenDisabled(): void
3838

3939
$this->dispatchLaravelEvent(new MessageLogged('debug', 'test message'));
4040

41-
$this->assertEmpty($this->getCurrentBreadcrumbs());
41+
$this->assertEmpty($this->getCurrentSentryBreadcrumbs());
4242
}
4343
}

‎test/Sentry/EventHandler/QueueEventsTest.php

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,18 @@ public function testQueueJobPushesAndPopsScopeWithBreadcrumbs(): void
1515
{
1616
dispatch(new QueueEventsTestJobWithBreadcrumb);
1717

18-
$this->assertCount(0, $this->getCurrentBreadcrumbs());
18+
$this->assertCount(0, $this->getCurrentSentryBreadcrumbs());
1919
}
2020

2121
public function testQueueJobThatReportsPushesAndPopsScopeWithBreadcrumbs(): void
2222
{
2323
dispatch(new QueueEventsTestJobThatReportsAnExceptionWithBreadcrumb);
2424

25-
$this->assertCount(0, $this->getCurrentBreadcrumbs());
25+
$this->assertCount(0, $this->getCurrentSentryBreadcrumbs());
2626

27-
$this->assertNotNull($this->getLastEvent());
27+
$this->assertNotNull($this->getLastSentryEvent());
2828

29-
$event = $this->getLastEvent();
29+
$event = $this->getLastSentryEvent();
3030

3131
$this->assertCount(2, $event->getBreadcrumbs());
3232
}
@@ -41,12 +41,12 @@ public function testQueueJobThatThrowsLeavesPushedScopeWithBreadcrumbs(): void
4141

4242
// We still expect to find the breadcrumbs from the job here so they are attached to reported exceptions
4343

44-
$this->assertCount(2, $this->getCurrentBreadcrumbs());
44+
$this->assertCount(2, $this->getCurrentSentryBreadcrumbs());
4545

46-
$firstBreadcrumb = $this->getCurrentBreadcrumbs()[0];
46+
$firstBreadcrumb = $this->getCurrentSentryBreadcrumbs()[0];
4747
$this->assertEquals('queue.job', $firstBreadcrumb->getCategory());
4848

49-
$secondBreadcrumb = $this->getCurrentBreadcrumbs()[1];
49+
$secondBreadcrumb = $this->getCurrentSentryBreadcrumbs()[1];
5050
$this->assertEquals('test', $secondBreadcrumb->getCategory());
5151
}
5252

@@ -66,12 +66,12 @@ public function testQueueJobsThatThrowPopsAndPushesScopeWithBreadcrumbsBeforeNew
6666

6767
// We only expect to find the breadcrumbs from the second job here
6868

69-
$this->assertCount(2, $this->getCurrentBreadcrumbs());
69+
$this->assertCount(2, $this->getCurrentSentryBreadcrumbs());
7070

71-
$firstBreadcrumb = $this->getCurrentBreadcrumbs()[0];
71+
$firstBreadcrumb = $this->getCurrentSentryBreadcrumbs()[0];
7272
$this->assertEquals('queue.job', $firstBreadcrumb->getCategory());
7373

74-
$secondBreadcrumb = $this->getCurrentBreadcrumbs()[1];
74+
$secondBreadcrumb = $this->getCurrentSentryBreadcrumbs()[1];
7575
$this->assertEquals('test #2', $secondBreadcrumb->getMessage());
7676
}
7777

@@ -83,7 +83,7 @@ public function testQueueJobsWithBreadcrumbSetInBetweenKeepsNonJobBreadcrumbsOnC
8383

8484
dispatch(new QueueEventsTestJobWithBreadcrumb);
8585

86-
$this->assertCount(1, $this->getCurrentBreadcrumbs());
86+
$this->assertCount(1, $this->getCurrentSentryBreadcrumbs());
8787
}
8888
}
8989

‎test/Sentry/Features/CacheIntegrationTest.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,29 +11,29 @@ public function testCacheBreadcrumbForWriteAndHitIsRecorded(): void
1111
{
1212
Cache::put($key = 'foo', 'bar');
1313

14-
$this->assertEquals("Written: {$key}", $this->getLastBreadcrumb()->getMessage());
14+
$this->assertEquals("Written: {$key}", $this->getLastSentryBreadcrumb()->getMessage());
1515

1616
Cache::get('foo');
1717

18-
$this->assertEquals("Read: {$key}", $this->getLastBreadcrumb()->getMessage());
18+
$this->assertEquals("Read: {$key}", $this->getLastSentryBreadcrumb()->getMessage());
1919
}
2020

2121
public function testCacheBreadcrumbForWriteAndForgetIsRecorded(): void
2222
{
2323
Cache::put($key = 'foo', 'bar');
2424

25-
$this->assertEquals("Written: {$key}", $this->getLastBreadcrumb()->getMessage());
25+
$this->assertEquals("Written: {$key}", $this->getLastSentryBreadcrumb()->getMessage());
2626

2727
Cache::forget($key);
2828

29-
$this->assertEquals("Forgotten: {$key}", $this->getLastBreadcrumb()->getMessage());
29+
$this->assertEquals("Forgotten: {$key}", $this->getLastSentryBreadcrumb()->getMessage());
3030
}
3131

3232
public function testCacheBreadcrumbForMissIsRecorded(): void
3333
{
3434
Cache::get($key = 'foo');
3535

36-
$this->assertEquals("Missed: {$key}", $this->getLastBreadcrumb()->getMessage());
36+
$this->assertEquals("Missed: {$key}", $this->getLastSentryBreadcrumb()->getMessage());
3737
}
3838

3939
public function testCacheBreadcrumbIsNotRecordedWhenDisabled(): void
@@ -46,6 +46,6 @@ public function testCacheBreadcrumbIsNotRecordedWhenDisabled(): void
4646

4747
Cache::get('foo');
4848

49-
$this->assertEmpty($this->getCurrentBreadcrumbs());
49+
$this->assertEmpty($this->getCurrentSentryBreadcrumbs());
5050
}
5151
}

‎test/Sentry/Features/ConsoleIntegrationTest.php

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,18 @@ class ConsoleIntegrationTest extends TestCase
1313
public function testScheduleMacro(): void
1414
{
1515
/** @var Event $scheduledEvent */
16-
$scheduledEvent = $this->getScheduler()->call(function () {})->sentryMonitor('test-monitor');
16+
$scheduledEvent = $this->getScheduler()
17+
->call(function () {})
18+
->sentryMonitor('test-monitor');
1719

1820
$scheduledEvent->run($this->app);
1921

2022
// We expect a total of 2 events to be sent to Sentry:
2123
// 1. The start check-in event
2224
// 2. The finish check-in event
23-
$this->assertEquals(2, $this->getEventsCount());
25+
$this->assertSentryCheckInCount(2);
2426

25-
$finishCheckInEvent = $this->getLastEvent();
27+
$finishCheckInEvent = $this->getLastSentryEvent();
2628

2729
$this->assertNotNull($finishCheckInEvent->getCheckIn());
2830
$this->assertEquals('test-monitor', $finishCheckInEvent->getCheckIn()->getMonitorSlug());
@@ -48,9 +50,9 @@ public function testScheduleMacroWithTimeZone(): void
4850
// We expect a total of 2 events to be sent to Sentry:
4951
// 1. The start check-in event
5052
// 2. The finish check-in event
51-
$this->assertEquals(2, $this->getEventsCount());
53+
$this->assertSentryCheckInCount(2);
5254

53-
$finishCheckInEvent = $this->getLastEvent();
55+
$finishCheckInEvent = $this->getLastSentryEvent();
5456

5557
$this->assertNotNull($finishCheckInEvent->getCheckIn());
5658
$this->assertEquals($expectedTimezone, $finishCheckInEvent->getCheckIn()->getMonitorConfig()->getTimezone());
@@ -66,9 +68,9 @@ public function testScheduleMacroAutomaticSlug(): void
6668
// We expect a total of 2 events to be sent to Sentry:
6769
// 1. The start check-in event
6870
// 2. The finish check-in event
69-
$this->assertEquals(2, $this->getEventsCount());
71+
$this->assertSentryCheckInCount(2);
7072

71-
$finishCheckInEvent = $this->getLastEvent();
73+
$finishCheckInEvent = $this->getLastSentryEvent();
7274

7375
$this->assertNotNull($finishCheckInEvent->getCheckIn());
7476
$this->assertEquals('scheduled_artisan-inspire', $finishCheckInEvent->getCheckIn()->getMonitorSlug());
@@ -89,7 +91,7 @@ public function testScheduleMacroWithoutDsnSet(): void
8991

9092
$scheduledEvent->run($this->app);
9193

92-
$this->assertEquals(0, $this->getEventsCount());
94+
$this->assertSentryCheckInCount(0);
9395
}
9496

9597
public function testScheduleMacroIsRegistered(): void

0 commit comments

Comments
 (0)