Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always start performance tracing #600

Merged
merged 8 commits into from Oct 27, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -21,6 +21,7 @@
- Drop support for Laravel 5.x (#581)
- Remove `Sentry\Integration::extractNameForRoute()`, it's alternative `Sentry\Integration::extractNameAndSourceForRoute()` is marked as `@internal` (#580)
- Remove internal `Sentry\Integration::currentTracingSpan()`, use `SentrySdk::getCurrentHub()->getSpan()` if you were using this internal method (#592)
- Remove incorrect checks if performance tracing should be enabled and rely on the transaction sampling decision instead (#600)
stayallive marked this conversation as resolved.
Show resolved Hide resolved

**Other changes**

Expand Down
2 changes: 1 addition & 1 deletion src/Sentry/Laravel/ServiceProvider.php
Expand Up @@ -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);
}

Expand Down
1 change: 0 additions & 1 deletion src/Sentry/Laravel/Tracing/EventHandler.php
Expand Up @@ -206,7 +206,6 @@ protected function queryExecutedHandler(DatabaseEvents\QueryExecuted $query): vo

$parentSpan = SentrySdk::getCurrentHub()->getSpan();

// If there is no tracing span active there is no need to handle the event
stayallive marked this conversation as resolved.
Show resolved Hide resolved
if ($parentSpan === null) {
return;
}
Expand Down
42 changes: 26 additions & 16 deletions src/Sentry/Laravel/Tracing/Middleware.php
Expand Up @@ -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();
}

/**
Expand Down Expand Up @@ -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);
Expand Down
Expand Up @@ -12,7 +12,6 @@ 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
stayallive marked this conversation as resolved.
Show resolved Hide resolved
if ($parentSpan === null) {
return $dispatch();
}
Expand Down
33 changes: 18 additions & 15 deletions src/Sentry/Laravel/Tracing/ServiceProvider.php
Expand Up @@ -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);
}
}
}
Expand All @@ -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
Expand Down