Skip to content

Commit f24f251

Browse files
stayallivestayallive
authored
Internal improvements (#769)
1 parent c3d2317 commit f24f251

16 files changed

+160
-100
lines changed

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

Lines changed: 2 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,7 @@
99

1010
/**
1111
* @method void onBoot() Setup the feature in the environment.
12-
* @method void onRegister() Register the feature in the environment.
1312
* @method void onBootInactive() Setup the feature in the environment in an inactive state (when no DSN was set).
14-
* @method void onRegisterInactive() Register the feature in the environment in an inactive state (when no DSN was set).
1513
*
1614
* @internal
1715
*/
@@ -52,31 +50,11 @@ public function __construct(Container $container)
5250
abstract public function isApplicable(): bool;
5351

5452
/**
55-
* Register the feature.
53+
* Register the feature in the environment.
5654
*/
5755
public function register(): void
5856
{
59-
if (method_exists($this, 'onRegister') && $this->isApplicable()) {
60-
try {
61-
$this->container->call([$this, 'onRegister']);
62-
} catch (Throwable $exception) {
63-
// If the feature setup fails, we don't want to prevent the rest of the SDK from working.
64-
}
65-
}
66-
}
67-
68-
/**
69-
* Register the feature in an inactive state (when no DSN was set).
70-
*/
71-
public function registerInactive(): void
72-
{
73-
if (method_exists($this, 'onRegisterInactive') && $this->isApplicable()) {
74-
try {
75-
$this->container->call([$this, 'onRegisterInactive']);
76-
} catch (Throwable $exception) {
77-
// If the feature setup fails, we don't want to prevent the rest of the SDK from working.
78-
}
79-
}
57+
// ...
8058
}
8159

8260
/**

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?php
2+
3+
namespace Sentry\Laravel\Features;
4+
5+
use Illuminate\Support\Facades\Log;
6+
use Sentry\Laravel\LogChannel;
7+
8+
class LogIntegration extends Feature
9+
{
10+
public function isApplicable(): bool
11+
{
12+
return true;
13+
}
14+
15+
public function register(): void
16+
{
17+
Log::extend('sentry', function ($app, array $config) {
18+
return (new LogChannel($app))($config);
19+
});
20+
}
21+
}

‎src/Sentry/Laravel/Features/Storage/Integration.php‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,7 @@ public function isApplicable(): bool
2121
return true;
2222
}
2323

24-
public function onRegister(): void
25-
{
26-
$this->registerDiskDriver();
27-
}
28-
29-
public function onRegisterInactive(): void
24+
public function register(): void
3025
{
3126
$this->registerDiskDriver();
3227
}

‎src/Sentry/Laravel/ServiceProvider.php‎

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,8 @@
99
use Illuminate\Foundation\Console\AboutCommand;
1010
use Illuminate\Foundation\Http\Kernel as HttpKernel;
1111
use Illuminate\Http\Request;
12-
use Illuminate\Log\LogManager;
13-
use Illuminate\Support\Facades\Config;
1412
use Laravel\Lumen\Application as Lumen;
1513
use RuntimeException;
16-
use Sentry\Client;
1714
use Sentry\ClientBuilder;
1815
use Sentry\ClientBuilderInterface;
1916
use Sentry\Event;
@@ -26,8 +23,10 @@
2623
use Sentry\Laravel\Http\LaravelRequestFetcher;
2724
use Sentry\Laravel\Http\SetRequestIpMiddleware;
2825
use Sentry\Laravel\Http\SetRequestMiddleware;
26+
use Sentry\Laravel\Tracing\BacktraceHelper;
2927
use Sentry\Laravel\Tracing\ServiceProvider as TracingServiceProvider;
3028
use Sentry\SentrySdk;
29+
use Sentry\Serializer\RepresentationSerializer;
3130
use Sentry\State\Hub;
3231
use Sentry\State\HubInterface;
3332
use Sentry\Tracing\TransactionMetadata;
@@ -55,6 +54,7 @@ class ServiceProvider extends BaseServiceProvider
5554
* List of features that are provided by the SDK.
5655
*/
5756
protected const FEATURES = [
57+
Features\LogIntegration::class,
5858
Features\CacheIntegration::class,
5959
Features\QueueIntegration::class,
6060
Features\ConsoleIntegration::class,
@@ -113,12 +113,6 @@ public function register(): void
113113

114114
$this->configureAndRegisterClient();
115115

116-
if (($logManager = $this->app->make('log')) instanceof LogManager) {
117-
$logManager->extend('sentry', function ($app, array $config) {
118-
return (new LogChannel($app))($config);
119-
});
120-
}
121-
122116
$this->registerFeatures();
123117
}
124118

@@ -159,16 +153,12 @@ protected function registerFeatures(): void
159153
$this->app->singleton($feature);
160154
}
161155

162-
$bootActive = $this->hasDsnSet();
163-
164156
foreach (self::FEATURES as $feature) {
165157
try {
166158
/** @var Feature $featureInstance */
167159
$featureInstance = $this->app->make($feature);
168160

169-
$bootActive
170-
? $featureInstance->register()
171-
: $featureInstance->registerInactive();
161+
$featureInstance->register();
172162
} catch (Throwable $e) {
173163
// Ensure that features do not break the whole application
174164
}
@@ -340,6 +330,14 @@ protected function configureAndRegisterClient(): void
340330
});
341331

342332
$this->app->alias(HubInterface::class, static::$abstract);
333+
334+
$this->app->singleton(BacktraceHelper::class, function () {
335+
$sentry = $this->app->make(HubInterface::class);
336+
337+
$options = $sentry->getClient()->getOptions();
338+
339+
return new BacktraceHelper($options, new RepresentationSerializer($options));
340+
});
343341
}
344342

345343
/**

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@
88
use Illuminate\Support\Str;
99
use Sentry\Serializer\RepresentationSerializerInterface;
1010

11+
/**
12+
* @internal
13+
*/
1114
class BacktraceHelper
1215
{
1316
/**

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

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use Illuminate\Http\Client\Events as HttpClientEvents;
99
use Illuminate\Routing\Events as RoutingEvents;
1010
use RuntimeException;
11+
use Sentry\Laravel\Features\Concerns\ResolvesEventOrigin;
1112
use Sentry\Laravel\Integration;
1213
use Sentry\Laravel\Util\WorksWithUris;
1314
use Sentry\SentrySdk;
@@ -18,7 +19,7 @@
1819

1920
class EventHandler
2021
{
21-
use WorksWithUris;
22+
use WorksWithUris, ResolvesEventOrigin;
2223

2324
/**
2425
* Map event handlers to events.
@@ -87,17 +88,10 @@ class EventHandler
8788
*/
8889
private $currentSpanStack = [];
8990

90-
/**
91-
* The backtrace helper.
92-
*
93-
* @var BacktraceHelper
94-
*/
95-
private $backtraceHelper;
96-
9791
/**
9892
* EventHandler constructor.
9993
*/
100-
public function __construct(array $config, BacktraceHelper $backtraceHelper)
94+
public function __construct(array $config)
10195
{
10296
$this->traceSqlQueries = ($config['sql_queries'] ?? true) === true;
10397
$this->traceSqlQueryOrigins = ($config['sql_origin'] ?? true) === true;
@@ -106,8 +100,6 @@ public function __construct(array $config, BacktraceHelper $backtraceHelper)
106100

107101
$this->traceQueueJobs = ($config['queue_jobs'] ?? false) === true;
108102
$this->traceQueueJobsAsTransactions = ($config['queue_job_transactions'] ?? false) === true;
109-
110-
$this->backtraceHelper = $backtraceHelper;
111103
}
112104

113105
/**
@@ -211,13 +203,15 @@ protected function queryExecutedHandler(DatabaseEvents\QueryExecuted $query): vo
211203
*/
212204
private function resolveQueryOriginFromBacktrace(): ?string
213205
{
214-
$firstAppFrame = $this->backtraceHelper->findFirstInAppFrameForBacktrace(debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS));
206+
$backtraceHelper = $this->makeBacktraceHelper();
207+
208+
$firstAppFrame = $backtraceHelper->findFirstInAppFrameForBacktrace(debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS));
215209

216210
if ($firstAppFrame === null) {
217211
return null;
218212
}
219213

220-
$filePath = $this->backtraceHelper->getOriginalViewPathForFrameOfCompiledViewPath($firstAppFrame) ?? $firstAppFrame->getFile();
214+
$filePath = $backtraceHelper->getOriginalViewPathForFrameOfCompiledViewPath($firstAppFrame) ?? $firstAppFrame->getFile();
221215

222216
return "{$filePath}:{$firstAppFrame->getLine()}";
223217
}

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

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
use Sentry\Laravel\BaseServiceProvider;
1818
use Sentry\Laravel\Tracing\Routing\TracingCallableDispatcherTracing;
1919
use Sentry\Laravel\Tracing\Routing\TracingControllerDispatcherTracing;
20-
use Sentry\Serializer\RepresentationSerializer;
2120

2221
class ServiceProvider extends BaseServiceProvider
2322
{
@@ -64,23 +63,11 @@ public function register(): void
6463

6564
return new Middleware($this->app, $continueAfterResponse);
6665
});
67-
68-
$this->app->singleton(BacktraceHelper::class, function () {
69-
/** @var \Sentry\State\Hub $sentry */
70-
$sentry = $this->app->make(self::$abstract);
71-
72-
$options = $sentry->getClient()->getOptions();
73-
74-
return new BacktraceHelper($options, new RepresentationSerializer($options));
75-
});
7666
}
7767

7868
private function bindEvents(array $tracingConfig): void
7969
{
80-
$handler = new EventHandler(
81-
$tracingConfig,
82-
$this->app->make(BacktraceHelper::class)
83-
);
70+
$handler = new EventHandler($tracingConfig);
8471

8572
try {
8673
/** @var \Illuminate\Contracts\Events\Dispatcher $dispatcher */

‎test/Sentry/ClientBuilderDecoratorTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@
66

77
class ClientBuilderDecoratorTest extends TestCase
88
{
9-
protected function getEnvironmentSetUp($app): void
9+
protected function defineEnvironment($app): void
1010
{
11-
parent::getEnvironmentSetUp($app);
11+
parent::defineEnvironment($app);
1212

1313
$app->extend(ClientBuilderInterface::class, function (ClientBuilderInterface $clientBuilder) {
1414
$clientBuilder->getOptions()->setEnvironment('from_service_container');

‎test/Sentry/Features/ConsoleIntegrationTest.php‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,9 @@ public function testScheduleMacroWithoutSlugOrCommandName(): void
5252
$this->getScheduler()->call(function () {})->sentryMonitor();
5353
}
5454

55+
/** @define-env envWithoutDsnSet */
5556
public function testScheduleMacroWithoutDsnSet(): void
5657
{
57-
$this->resetApplicationWithConfig([
58-
'sentry.dsn' => null,
59-
]);
60-
6158
/** @var Event $scheduledEvent */
6259
$scheduledEvent = $this->getScheduler()->call(function () {})->sentryMonitor('test-monitor');
6360

@@ -79,6 +76,7 @@ public function testScheduleMacroIsRegistered(): void
7976
$this->assertTrue(Event::hasMacro('sentryMonitor'));
8077
}
8178

79+
/** @define-env envWithoutDsnSet */
8280
public function testScheduleMacroIsRegisteredWithoutDsnSet(): void
8381
{
8482
if (!method_exists(Event::class, 'flushMacros')) {
@@ -87,9 +85,7 @@ public function testScheduleMacroIsRegisteredWithoutDsnSet(): void
8785

8886
Event::flushMacros();
8987

90-
$this->resetApplicationWithConfig([
91-
'sentry.dsn' => null,
92-
]);
88+
$this->refreshApplication();
9389

9490
$this->assertTrue(Event::hasMacro('sentryMonitor'));
9591
}

‎test/Sentry/Features/LogIntegrationTest.php‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
<?php
2+
3+
namespace Sentry\Features;
4+
5+
use Illuminate\Config\Repository;
6+
use Illuminate\Support\Facades\Log;
7+
use Sentry\Laravel\Tests\TestCase;
8+
use Sentry\Severity;
9+
10+
class LogIntegrationTest extends TestCase
11+
{
12+
protected function defineEnvironment($app): void
13+
{
14+
parent::defineEnvironment($app);
15+
16+
tap($app['config'], static function (Repository $config) {
17+
$config->set('logging.channels.sentry', [
18+
'driver' => 'sentry',
19+
]);
20+
21+
$config->set('logging.channels.sentry_error_level', [
22+
'driver' => 'sentry',
23+
'level' => 'error',
24+
]);
25+
});
26+
}
27+
28+
public function testLogChannelIsRegistered(): void
29+
{
30+
$this->expectNotToPerformAssertions();
31+
32+
Log::channel('sentry');
33+
}
34+
35+
/** @define-env envWithoutDsnSet */
36+
public function testLogChannelIsRegisteredWithoutDsn(): void
37+
{
38+
$this->expectNotToPerformAssertions();
39+
40+
Log::channel('sentry');
41+
}
42+
43+
public function testLogChannelGeneratesEvents(): void
44+
{
45+
$logger = Log::channel('sentry');
46+
47+
$logger->info('Sentry Laravel info log message');
48+
49+
$this->assertEquals(1, $this->getEventsCount());
50+
51+
$event = $this->getLastEvent();
52+
53+
$this->assertEquals(Severity::info(), $event->getLevel());
54+
$this->assertEquals('Sentry Laravel info log message', $event->getMessage());
55+
}
56+
57+
public function testLogChannelGeneratesEventsOnlyForConfiguredLevel(): void
58+
{
59+
$logger = Log::channel('sentry_error_level');
60+
61+
$logger->info('Sentry Laravel info log message');
62+
$logger->warning('Sentry Laravel warning log message');
63+
$logger->error('Sentry Laravel error log message');
64+
65+
$this->assertEquals(1, $this->getEventsCount());
66+
67+
$event = $this->getLastEvent();
68+
69+
$this->assertEquals(Severity::error(), $event->getLevel());
70+
$this->assertEquals('Sentry Laravel error log message', $event->getMessage());
71+
}
72+
}

0 commit comments

Comments
 (0)