Always start performance tracing (#600)

Author: stayallive

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## Unreleased
 
+- Remove incorrect checks if performance tracing should be enabled and rely on the transaction sampling decision instead (#600)
+
 ## 3.0.0
 
 **New features**

composer.json

@@ -23,8 +23,8 @@
     "require": {
         "php": "^7.2 | ^8.0",
         "illuminate/support": "^6.0 | ^7.0 | ^8.0 | ^9.0",
-        "sentry/sentry": "^3.9",
-        "sentry/sdk": "^3.1",
+        "sentry/sentry": "^3.10",
+        "sentry/sdk": "^3.3",
         "symfony/psr-http-message-bridge": "^1.0 | ^2.0",
         "nyholm/psr7": "^1.0"
     },

src/Sentry/Laravel/EventHandler.php

@@ -14,7 +14,6 @@
 use Illuminate\Http\Request;
 use Illuminate\Log\Events as LogEvents;
 use Illuminate\Queue\Events as QueueEvents;
-use Illuminate\Queue\QueueManager;
 use Illuminate\Routing\Events as RoutingEvents;
 use Laravel\Octane\Events as Octane;
 use Laravel\Sanctum\Events as Sanctum;

src/Sentry/Laravel/ServiceProvider.php

@@ -238,7 +238,7 @@ private function resolveIntegrationsFromUserConfig(): array
 
         $enableDefaultTracingIntegrations = $userConfig['tracing']['default_integrations'] ?? true;
 
-        if ($enableDefaultTracingIntegrations && $this->couldHavePerformanceTracingEnabled()) {
+        if ($enableDefaultTracingIntegrations) {
             $integrationsToResolve = array_merge($integrationsToResolve, TracingServiceProvider::DEFAULT_INTEGRATIONS);
         }
 

src/Sentry/Laravel/Tracing/EventHandler.php

@@ -250,6 +250,7 @@ protected function transactionBeginningHandler(DatabaseEvents\TransactionBeginni
     {
         $parentSpan = SentrySdk::getCurrentHub()->getSpan();
 
+        // If there is no tracing span active there is no need to handle the event
         if ($parentSpan === null) {
             return;
         }
@@ -284,6 +285,7 @@ protected function httpClientRequestSendingHandler(HttpClientEvents\RequestSendi
     {
         $parentSpan = SentrySdk::getCurrentHub()->getSpan();
 
+        // If there is no tracing span active there is no need to handle the event
         if ($parentSpan === null) {
             return;
         }
@@ -387,12 +389,10 @@ private function afterQueuedJob(?SpanStatus $status = null): void
     {
         $span = $this->popSpan();
 
-        if ($span === null) {
-            return;
+        if ($span !== null) {
+            $span->finish();
+            $span->setStatus($status);
         }
-
-        $span->setStatus($status);
-        $span->finish();
     }
 
     private function pushSpan(Span $span): void

src/Sentry/Laravel/Tracing/Middleware.php

@@ -62,26 +62,29 @@ public function handle(Request $request, Closure $next)
      */
     public function terminate(Request $request, $response): void
     {
-        if ($this->transaction !== null && app()->bound(HubInterface::class)) {
-            // We stop here if a route has not been matched unless we are configured to trace missing routes
-            if (config('sentry.tracing.missing_routes', false) === false && $request->route() === null) {
-                return;
-            }
+        // If there is no transaction or the HubInterface is not bound in the container there is nothing for us to do
+        if ($this->transaction === null || !app()->bound(HubInterface::class)) {
+            return;
+        }
 
-            if ($this->appSpan !== null) {
-                $this->appSpan->finish();
-            }
+        // We stop here if a route has not been matched unless we are configured to trace missing routes
+        if (config('sentry.tracing.missing_routes', false) === false && $request->route() === null) {
+            return;
+        }
 
-            // Make sure we set the transaction and not have a child span in the Sentry SDK
-            // If the transaction is not on the scope during finish, the trace.context is wrong
-            SentrySdk::getCurrentHub()->setSpan($this->transaction);
+        if ($this->appSpan !== null) {
+            $this->appSpan->finish();
+        }
 
-            if ($response instanceof SymfonyResponse) {
-                $this->hydrateResponseData($response);
-            }
+        // Make sure we set the transaction and not have a child span in the Sentry SDK
+        // If the transaction is not on the scope during finish, the trace.context is wrong
+        SentrySdk::getCurrentHub()->setSpan($this->transaction);
 
-            $this->transaction->finish();
+        if ($response instanceof SymfonyResponse) {
+            $this->hydrateResponseData($response);
         }
+
+        $this->transaction->finish();
     }
 
     /**
@@ -119,7 +122,14 @@ private function startTransaction(Request $request, HubInterface $sentry): void
             'method' => strtoupper($request->method()),
         ]);
 
-        $this->transaction = $sentry->startTransaction($context);
+        $transaction = $sentry->startTransaction($context);
+
+        // If this transaction is not sampled, don't set it either and stop doing work from this point on
+        if (!$transaction->getSampled()) {
+            return;
+        }
+
+        $this->transaction = $transaction;
 
         // Setting the Transaction on the Hub
         SentrySdk::getCurrentHub()->setSpan($this->transaction);

src/Sentry/Laravel/Tracing/Routing/TracingRoutingDispatcher.php

@@ -12,7 +12,9 @@ protected function wrapRouteDispatch(callable $dispatch, Route $route)
     {
         $parentSpan = SentrySdk::getCurrentHub()->getSpan();
 
-        // When there is no active transaction we can skip doing anything and just immediately return the callable
+        // When there is no span we can skip creating
+        // the span and just immediately return with the
+        // callable result because there is no transaction.
         if ($parentSpan === null) {
             return $dispatch();
         }

src/Sentry/Laravel/Tracing/ServiceProvider.php

@@ -26,22 +26,29 @@ class ServiceProvider extends BaseServiceProvider
 
     public function boot(): void
     {
-        if ($this->hasDsnSet() && $this->couldHavePerformanceTracingEnabled()) {
-            $tracingConfig = $this->getUserConfig()['tracing'] ?? [];
+        // If there is no DSN set we register nothing since it's impossible for us to send traces without a DSN set
+        if (!$this->hasDsnSet()) {
+            return;
+        }
 
-            $this->bindEvents($tracingConfig);
+        $this->app->booted(function () {
+            $this->app->make(Middleware::class)->setBootedTimestamp();
+        });
 
-            $this->bindViewEngine($tracingConfig);
+        $tracingConfig = $this->getUserConfig()['tracing'] ?? [];
 
-            $this->decorateRoutingDispatchers();
+        $this->bindEvents($tracingConfig);
 
-            if ($this->app->bound(HttpKernelInterface::class)) {
-                /** @var \Illuminate\Foundation\Http\Kernel $httpKernel */
-                $httpKernel = $this->app->make(HttpKernelInterface::class);
+        $this->bindViewEngine($tracingConfig);
 
-                if ($httpKernel instanceof HttpKernel) {
-                    $httpKernel->prependMiddleware(Middleware::class);
-                }
+        $this->decorateRoutingDispatchers();
+
+        if ($this->app->bound(HttpKernelInterface::class)) {
+            /** @var \Illuminate\Foundation\Http\Kernel $httpKernel */
+            $httpKernel = $this->app->make(HttpKernelInterface::class);
+
+            if ($httpKernel instanceof HttpKernel) {
+                $httpKernel->prependMiddleware(Middleware::class);
             }
         }
     }
@@ -58,10 +65,6 @@ public function register(): void
 
             return new BacktraceHelper($options, new RepresentationSerializer($options));
         });
-
-        $this->app->booted(function () {
-            $this->app->make(Middleware::class)->setBootedTimestamp();
-        });
     }
 
     private function bindEvents(array $tracingConfig): void