Skip to content

Commit 1e0d5da

Browse files
stayallivestayallive
committed
CR
1 parent a3ec9e6 commit 1e0d5da

File tree

4 files changed

+88
-36
lines changed

4 files changed

+88
-36
lines changed

‎src/Sentry/Laravel/ServiceProvider.php

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use Illuminate\Foundation\Http\Kernel as HttpKernel;
88
use Illuminate\Log\LogManager;
99
use Laravel\Lumen\Application as Lumen;
10+
use RuntimeException;
1011
use Sentry\ClientBuilder;
1112
use Sentry\ClientBuilderInterface;
1213
use Sentry\Integration as SdkIntegration;
@@ -23,9 +24,10 @@ class ServiceProvider extends BaseServiceProvider
2324
* List of configuration options that are Laravel specific and should not be sent to the base PHP SDK.
2425
*/
2526
private const LARAVEL_SPECIFIC_OPTIONS = [
26-
// We do not want these settings to hit the PHP SDK because it's Laravel specific and the SDK will throw errors
27+
// We do not want these settings to hit the PHP SDK because they are Laravel specific and the PHP SDK will throw errors
2728
'tracing',
2829
'breadcrumbs',
30+
'tracing_integrations',
2931
// We resolve the integrations through the container later, so we initially do not pass it to the SDK yet
3032
'integrations',
3133
// This is kept for backwards compatibility and can be dropped in a future breaking release
@@ -132,7 +134,7 @@ protected function configureAndRegisterClient(): void
132134
}
133135

134136
$this->app->bind(ClientBuilderInterface::class, function () {
135-
$basePath = base_path();
137+
$basePath = base_path();
136138
$userConfig = $this->getUserConfig();
137139

138140
foreach (self::LARAVEL_SPECIFIC_OPTIONS as $laravelSpecificOptionName) {
@@ -141,7 +143,7 @@ protected function configureAndRegisterClient(): void
141143

142144
$options = \array_merge(
143145
[
144-
'prefixes' => [$basePath],
146+
'prefixes' => [$basePath],
145147
'in_app_exclude' => ["{$basePath}/vendor"],
146148
],
147149
$userConfig
@@ -233,16 +235,27 @@ private function resolveIntegrationsFromUserConfig(): array
233235
foreach ($userIntegrations as $userIntegration) {
234236
if ($userIntegration instanceof SdkIntegration\IntegrationInterface) {
235237
$integrations[] = $userIntegration;
236-
} elseif (\is_string($userIntegration)) {
238+
} elseif (\is_string($userIntegration) && $this->app->bound($userIntegration)) {
237239
$resolvedIntegration = $this->app->make($userIntegration);
238240

239-
if (!($resolvedIntegration instanceof SdkIntegration\IntegrationInterface)) {
240-
throw new \RuntimeException('Sentry integrations should a instance of `\Sentry\Integration\IntegrationInterface`.');
241+
if (!$resolvedIntegration instanceof SdkIntegration\IntegrationInterface) {
242+
throw new RuntimeException(
243+
sprintf(
244+
'Sentry integrations must be an instance of `%s` got `%s`.',
245+
SdkIntegration\IntegrationInterface::class,
246+
get_class($resolvedIntegration)
247+
)
248+
);
241249
}
242250

243251
$integrations[] = $resolvedIntegration;
244252
} else {
245-
throw new \RuntimeException('Sentry integrations should either be a container reference or a instance of `\Sentry\Integration\IntegrationInterface`.');
253+
throw new RuntimeException(
254+
sprintf(
255+
'Sentry integrations must either be a valid container reference or an instance of `%s`.',
256+
SdkIntegration\IntegrationInterface::class
257+
)
258+
);
246259
}
247260
}
248261

‎src/Sentry/Laravel/Tracing/Integrations/IntegrationInterface.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?php
2+
3+
namespace Sentry\Laravel\Tracing\Integrations;
4+
5+
interface IntegrationInterface
6+
{
7+
public static function supported(): bool;
8+
}

‎src/Sentry/Laravel/Tracing/Integrations/LighthouseIntegration.php

Lines changed: 36 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@
22

33
namespace Sentry\Laravel\Tracing\Integrations;
44

5+
use GraphQL\Language\AST\DocumentNode;
56
use GraphQL\Language\AST\OperationDefinitionNode;
6-
use Illuminate\Contracts\Events\Dispatcher;
7+
use Illuminate\Contracts\Events\Dispatcher as EventDispatcher;
78
use Nuwave\Lighthouse\Events\EndExecution;
89
use Nuwave\Lighthouse\Events\EndRequest;
910
use Nuwave\Lighthouse\Events\StartExecution;
@@ -12,9 +13,9 @@
1213
use Sentry\SentrySdk;
1314
use Sentry\Tracing\SpanContext;
1415

15-
class LighthouseIntegration
16+
class LighthouseIntegration implements IntegrationInterface
1617
{
17-
/** @var array<int, <string|null, \GraphQL\Language\AST\OperationDefinitionNode>> $operations */
18+
/** @var array<int, array{?string, \GraphQL\Language\AST\OperationDefinitionNode}> $operations */
1819
private $operations;
1920

2021
/** @var \Sentry\Tracing\Span|null $previousSpan */
@@ -26,16 +27,12 @@ class LighthouseIntegration
2627
/** @var \Sentry\Tracing\Span|null $operationSpan */
2728
private $operationSpan;
2829

29-
public function __construct(Dispatcher $evenDispatcher)
30+
public function __construct(EventDispatcher $eventDispatcher)
3031
{
31-
if (!class_exists(StartRequest::class)) {
32-
return;
33-
}
34-
35-
$evenDispatcher->listen(StartRequest::class, [$this, 'handleStartRequest']);
36-
$evenDispatcher->listen(StartExecution::class, [$this, 'handleStartExecution']);
37-
$evenDispatcher->listen(EndExecution::class, [$this, 'handleEndExecution']);
38-
$evenDispatcher->listen(EndRequest::class, [$this, 'handleEndRequest']);
32+
$eventDispatcher->listen(StartRequest::class, [$this, 'handleStartRequest']);
33+
$eventDispatcher->listen(StartExecution::class, [$this, 'handleStartExecution']);
34+
$eventDispatcher->listen(EndExecution::class, [$this, 'handleEndExecution']);
35+
$eventDispatcher->listen(EndRequest::class, [$this, 'handleEndRequest']);
3936
}
4037

4138
public function handleStartRequest(StartRequest $startRequest): void
@@ -46,12 +43,12 @@ public function handleStartRequest(StartRequest $startRequest): void
4643
return;
4744
}
4845

49-
$this->operations = [];
50-
5146
$context = new SpanContext;
5247
$context->setOp('graphql.request');
5348

54-
$this->requestSpan = $this->previousSpan->startChild($context);
49+
$this->operations = [];
50+
$this->requestSpan = $this->previousSpan->startChild($context);
51+
$this->operationSpan = null;
5552

5653
SentrySdk::getCurrentHub()->setSpan($this->requestSpan);
5754
}
@@ -62,14 +59,18 @@ public function handleStartExecution(StartExecution $startExecution): void
6259
return;
6360
}
6461

62+
if (!$startExecution->query instanceof DocumentNode) {
63+
return;
64+
}
65+
6566
/** @var \GraphQL\Language\AST\OperationDefinitionNode|null $operationDefinition */
6667
$operationDefinition = $startExecution->query->definitions[0] ?? null;
6768

68-
if ($operationDefinition === null) {
69+
if (!$operationDefinition instanceof OperationDefinitionNode) {
6970
return;
7071
}
7172

72-
$this->operations[] = [$startExecution->operationName, $operationDefinition];
73+
$this->operations[] = [$startExecution->operationName ?? null, $operationDefinition];
7374

7475
$context = new SpanContext;
7576
$context->setOp(
@@ -116,6 +117,12 @@ public function handleEndRequest(EndRequest $endRequest): void
116117

117118
private function updateTransaction(): void
118119
{
120+
$transaction = SentrySdk::getCurrentHub()->getTransaction();
121+
122+
if ($transaction === null) {
123+
return;
124+
}
125+
119126
$groupedOperations = [];
120127

121128
foreach ($this->operations as [$operationName, $operation]) {
@@ -143,10 +150,12 @@ private function updateTransaction(): void
143150

144151
$transactionName = 'lighthouse?' . implode('&', $groupedOperations);
145152

146-
Integration::setTransaction($transactionName);
147-
SentrySdk::getCurrentHub()->getTransaction()->setName($transactionName);
153+
$transaction->setName($transactionName);
148154
}
149155

156+
/**
157+
* @return array<int, string>
158+
*/
150159
private function extractOperationNames(OperationDefinitionNode $operation): array
151160
{
152161
if ($operation->name !== null) {
@@ -157,9 +166,9 @@ private function extractOperationNames(OperationDefinitionNode $operation): arra
157166

158167
/** @var \GraphQL\Language\AST\FieldNode $selection */
159168
foreach ($operation->selectionSet->selections as $selection) {
160-
$selectionSet[] = $selection->alias === null
161-
? $selection->name->value
162-
: $selection->alias->value;
169+
// Not respecting aliases because they are only relevant for clients
170+
// and the tracing we extract here is targeted at server developers.
171+
$selectionSet[] = $selection->name->value;
163172
}
164173

165174
sort($selectionSet, SORT_STRING);
@@ -169,6 +178,10 @@ private function extractOperationNames(OperationDefinitionNode $operation): arra
169178

170179
public static function supported(): bool
171180
{
172-
return class_exists(StartRequest::class);
181+
if (!class_exists(StartRequest::class) || !class_exists(StartExecution::class)) {
182+
return false;
183+
}
184+
185+
return property_exists(StartExecution::class, 'query');
173186
}
174187
}

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

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,17 @@
1111
use Illuminate\View\Factory as ViewFactory;
1212
use InvalidArgumentException;
1313
use Laravel\Lumen\Application as Lumen;
14+
use RuntimeException;
1415
use Sentry\Laravel\BaseServiceProvider;
1516
use Sentry\Serializer\RepresentationSerializer;
1617
use Throwable;
1718

1819
class ServiceProvider extends BaseServiceProvider
1920
{
21+
protected const DEFAULT_INTEGRATIONS = [
22+
Integrations\LighthouseIntegration::class,
23+
];
24+
2025
public function boot(): void
2126
{
2227
if ($this->hasDsnSet()) {
@@ -121,18 +126,31 @@ private function wrapViewEngine(Engine $realEngine): Engine
121126

122127
private function bootIntegrations(): void
123128
{
124-
$enableDefaultIntegrations = $this->getUserConfig()['tracing']['default_integrations'] ?? true;
129+
$userConfig = $this->getUserConfig();
125130

126-
$defaultIntegrations = $enableDefaultIntegrations ? [
127-
Integrations\LighthouseIntegration::class,
128-
] : [];
131+
$enableDefaultIntegrations = $userConfig['tracing']['default_integrations'] ?? true;
129132

130133
$integrations = array_merge(
131-
$defaultIntegrations,
132-
$this->getUserConfig()['tracing_integrations'] ?? []
134+
$enableDefaultIntegrations ? static::DEFAULT_INTEGRATIONS : [],
135+
$userConfig['tracing_integrations'] ?? []
133136
);
134137

138+
/** @var \Sentry\Laravel\Tracing\Integrations\IntegrationInterface $tracingIntegration */
135139
foreach ($integrations as $tracingIntegration) {
140+
if (!is_subclass_of($tracingIntegration, Integrations\IntegrationInterface::class)) {
141+
throw new RuntimeException(
142+
sprintf(
143+
'Sentry tracing integrations must be an instance of `%s` got `%s`.',
144+
SdkIntegration\IntegrationInterface::class,
145+
get_class($tracingIntegration)
146+
)
147+
);
148+
}
149+
150+
if (!$tracingIntegration::supported()) {
151+
continue;
152+
}
153+
136154
try {
137155
$this->app->make($tracingIntegration);
138156
} catch (Throwable $e) {

0 commit comments

Comments
 (0)