From dc8e9925764881d5c3618826c35560a60ce2be23 Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Tue, 27 Sep 2022 14:00:51 +0200 Subject: [PATCH 1/4] feat: Add support for Dynamic Sampling --- src/Sentry/Laravel/Console/TestCommand.php | 1 + src/Sentry/Laravel/Integration.php | 20 +++++++++++++++++++- src/Sentry/Laravel/Tracing/EventHandler.php | 8 +++++++- src/Sentry/Laravel/Tracing/Middleware.php | 5 ++++- 4 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/Sentry/Laravel/Console/TestCommand.php b/src/Sentry/Laravel/Console/TestCommand.php index 0623d5fd..7cf52127 100644 --- a/src/Sentry/Laravel/Console/TestCommand.php +++ b/src/Sentry/Laravel/Console/TestCommand.php @@ -140,6 +140,7 @@ public function log($level, $message, array $context = []): void $transactionContext = new TransactionContext(); $transactionContext->setSampled(true); $transactionContext->setName('Sentry Test Transaction'); + $transactionContext->setSource(TransactionSource::custom()); $transactionContext->setOp('sentry.test'); $transaction = $hub->startTransaction($transactionContext); diff --git a/src/Sentry/Laravel/Integration.php b/src/Sentry/Laravel/Integration.php index f400aaf5..6a151258 100644 --- a/src/Sentry/Laravel/Integration.php +++ b/src/Sentry/Laravel/Integration.php @@ -247,7 +247,25 @@ public static function sentryTracingMeta(): string } $content = sprintf('', $span->toTraceparent()); - // $content .= sprintf('', $span->getDescription()); + + return $content; + } + + /** + * Retrieve the meta tags with baggage information to link this request to front-end requests. + * This propagates the Dynamic Sampling Context. + * + * @return string + */ + public static function sentryBaggageMeta(): string + { + $span = self::currentTracingSpan(); + + if ($span === null) { + return ''; + } + + $content = sprintf('', $span->toBaggage()); return $content; } diff --git a/src/Sentry/Laravel/Tracing/EventHandler.php b/src/Sentry/Laravel/Tracing/EventHandler.php index 054dc62e..ece177ee 100644 --- a/src/Sentry/Laravel/Tracing/EventHandler.php +++ b/src/Sentry/Laravel/Tracing/EventHandler.php @@ -16,11 +16,14 @@ use Sentry\Tracing\SpanContext; use Sentry\Tracing\SpanStatus; use Sentry\Tracing\TransactionContext; +use Sentry\Tracing\TransactionSource; class EventHandler { public const QUEUE_PAYLOAD_TRACE_PARENT_DATA = 'sentry_trace_parent_data'; + public const QUEUE_PAYLOAD_BAGGAGE_DATA = 'sentry_baggage_data'; + /** * Map event handlers to events. * @@ -153,6 +156,7 @@ public function subscribeQueueEvents(QueueManager $queue): void if ($currentSpan !== null && $payload !== null) { $payload[self::QUEUE_PAYLOAD_TRACE_PARENT_DATA] = $currentSpan->toTraceparent(); + $payload[self::QUEUE_PAYLOAD_BAGGAGE_DATA] = $currentSpan->toBaggage(); } return $payload; @@ -294,10 +298,11 @@ protected function queueJobProcessingHandler(QueueEvents\JobProcessing $event) if ($parentSpan === null) { $traceParent = $event->job->payload()[self::QUEUE_PAYLOAD_TRACE_PARENT_DATA] ?? null; + $baggage = $event->job->payload()[self::QUEUE_PAYLOAD_BAGGAGE_DATA] ?? null; $context = $traceParent === null ? new TransactionContext - : TransactionContext::fromSentryTrace($traceParent); + : TransactionContext::fromHeaders($traceParent, $baggage); // If the parent transaction was not sampled we also stop the queue job from being recorded if ($context->getParentSampled() === false) { @@ -325,6 +330,7 @@ protected function queueJobProcessingHandler(QueueEvents\JobProcessing $event) if ($context instanceof TransactionContext) { $context->setName($resolvedJobName ?? $event->job->getName()); + $context->setSource(TransactionSource::task()); } $context->setOp('queue.process'); diff --git a/src/Sentry/Laravel/Tracing/Middleware.php b/src/Sentry/Laravel/Tracing/Middleware.php index 9045446c..5fa03020 100644 --- a/src/Sentry/Laravel/Tracing/Middleware.php +++ b/src/Sentry/Laravel/Tracing/Middleware.php @@ -12,6 +12,7 @@ use Sentry\Tracing\Span; use Sentry\Tracing\SpanContext; use Sentry\Tracing\TransactionContext; +use Sentry\Tracing\TransactionSource; class Middleware { @@ -103,12 +104,14 @@ private function startTransaction(Request $request, HubInterface $sentry): void { $requestStartTime = $request->server('REQUEST_TIME_FLOAT', microtime(true)); $sentryTraceHeader = $request->header('sentry-trace'); + $baggageHeader = $request->header('baggage'); $context = $sentryTraceHeader - ? TransactionContext::fromSentryTrace($sentryTraceHeader) + ? TransactionContext::fromHeaders($sentryTraceHeader, $baggageHeader) : new TransactionContext; $context->setOp('http.server'); + $context->setSource(TransactionSource::url()); $context->setData([ 'url' => '/' . ltrim($request->path(), '/'), 'method' => strtoupper($request->method()), From 9e113c26bd8aae28158a9fb38ef0c50c12f47ef0 Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Wed, 5 Oct 2022 12:27:22 +0200 Subject: [PATCH 2/4] Bump minimum required PHP SDK --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index cd152ece..e95d2a4e 100644 --- a/composer.json +++ b/composer.json @@ -23,7 +23,7 @@ "require": { "php": "^7.2 | ^8.0", "illuminate/support": "5.0 - 5.8 | ^6.0 | ^7.0 | ^8.0 | ^9.0", - "sentry/sentry": "^3.3", + "sentry/sentry": "^3.9", "sentry/sdk": "^3.1", "symfony/psr-http-message-bridge": "^1.0 | ^2.0", "nyholm/psr7": "^1.0" From 82ccb28f7ddfda2ac2ac4d0e92dd92a26d2c780e Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Wed, 5 Oct 2022 13:50:11 +0200 Subject: [PATCH 3/4] wip --- src/Sentry/Laravel/EventHandler.php | 2 +- src/Sentry/Laravel/Integration.php | 50 ++++++++++++++++--- src/Sentry/Laravel/Tracing/EventHandler.php | 6 +-- .../Integrations/LighthouseIntegration.php | 4 +- src/Sentry/Laravel/Tracing/Middleware.php | 27 +++++----- 5 files changed, 63 insertions(+), 26 deletions(-) diff --git a/src/Sentry/Laravel/EventHandler.php b/src/Sentry/Laravel/EventHandler.php index f9f32bfe..8db2bb1d 100644 --- a/src/Sentry/Laravel/EventHandler.php +++ b/src/Sentry/Laravel/EventHandler.php @@ -283,7 +283,7 @@ public function __call($method, $arguments) */ protected function routerMatchedHandler(Route $route) { - $routeName = Integration::extractNameForRoute($route); + [$routeName] = Integration::extractNameAndSourceForRoute($route); Integration::addBreadcrumb(new Breadcrumb( Breadcrumb::LEVEL_INFO, diff --git a/src/Sentry/Laravel/Integration.php b/src/Sentry/Laravel/Integration.php index 6a151258..38e741b9 100644 --- a/src/Sentry/Laravel/Integration.php +++ b/src/Sentry/Laravel/Integration.php @@ -6,6 +6,7 @@ use Illuminate\Support\Str; use Sentry\SentrySdk; use Sentry\Tracing\Span; +use Sentry\Tracing\TransactionSource; use function Sentry\addBreadcrumb; use function Sentry\configureScope; use Sentry\Breadcrumb; @@ -122,27 +123,45 @@ public static function flushEvents(): void * @param \Illuminate\Routing\Route $route * * @return string + * + * @deprecated This will be removed in version 3.0, use `extractNameAndSourceForRoute` instead. */ public static function extractNameForRoute(Route $route): string { + return self::extractNameAndSourceForRoute($route)[0]; + } + + /** + * Extract the readable name for a route and the transaction source for where that route name came from. + * + * @param \Illuminate\Routing\Route $route + * + * @return array{0: string, 1: \Sentry\Tracing\TransactionSource} + */ + public static function extractNameAndSourceForRoute(Route $route): array + { + $source = null; $routeName = null; - // someaction (route name/alias) + // some.action (route name/alias) if ($route->getName()) { + $source = TransactionSource::component(); $routeName = self::extractNameForNamedRoute($route->getName()); } // Some\Controller@someAction (controller action) if (empty($routeName) && $route->getActionName()) { + $source = TransactionSource::component(); $routeName = self::extractNameForActionRoute($route->getActionName()); } - // /someaction // Fallback to the url + // /some/{action} // Fallback to the route uri (with parameter placeholders) if (empty($routeName) || $routeName === 'Closure') { + $source = TransactionSource::route(); $routeName = '/' . ltrim($route->uri(), '/'); } - return $routeName; + return [$routeName, $source]; } /** @@ -152,24 +171,42 @@ public static function extractNameForRoute(Route $route): string * @param string $path The path of the request * * @return string + * + * @deprecated This will be removed in version 3.0, use `extractNameAndSourceForLumenRoute` instead. */ public static function extractNameForLumenRoute(array $routeData, string $path): string { + return self::extractNameAndSourceForLumenRoute($routeData, $path)[0]; + } + + /** + * Extract the readable name for a Lumen route and the transaction source for where that route name came from. + * + * @param array $routeData The array of route data + * @param string $path The path of the request + * + * @return array{0: string, 1: \Sentry\Tracing\TransactionSource} + */ + public static function extractNameAndSourceForLumenRoute(array $routeData, string $path): array + { + $source = null; $routeName = null; $route = $routeData[1] ?? []; - // someaction (route name/alias) + // some.action (route name/alias) if (!empty($route['as'])) { + $source = TransactionSource::component(); $routeName = self::extractNameForNamedRoute($route['as']); } // Some\Controller@someAction (controller action) if (empty($routeName) && !empty($route['uses'])) { + $source = TransactionSource::component(); $routeName = self::extractNameForActionRoute($route['uses']); } - // /someaction // Fallback to the url + // /some/{action} // Fallback to the route uri (with parameter placeholders) if (empty($routeName) || $routeName === 'Closure') { $routeUri = array_reduce( array_keys($routeData[2]), @@ -179,10 +216,11 @@ static function ($carry, $key) use ($routeData) { $path ); + $source = TransactionSource::url(); $routeName = '/' . ltrim($routeUri, '/'); } - return $routeName; + return [$routeName, $source]; } /** diff --git a/src/Sentry/Laravel/Tracing/EventHandler.php b/src/Sentry/Laravel/Tracing/EventHandler.php index ece177ee..1fafb3bd 100644 --- a/src/Sentry/Laravel/Tracing/EventHandler.php +++ b/src/Sentry/Laravel/Tracing/EventHandler.php @@ -297,12 +297,10 @@ protected function queueJobProcessingHandler(QueueEvents\JobProcessing $event) } if ($parentSpan === null) { - $traceParent = $event->job->payload()[self::QUEUE_PAYLOAD_TRACE_PARENT_DATA] ?? null; $baggage = $event->job->payload()[self::QUEUE_PAYLOAD_BAGGAGE_DATA] ?? null; + $traceParent = $event->job->payload()[self::QUEUE_PAYLOAD_TRACE_PARENT_DATA] ?? null; - $context = $traceParent === null - ? new TransactionContext - : TransactionContext::fromHeaders($traceParent, $baggage); + $context = TransactionContext::fromHeaders($traceParent ?? '', $baggage ?? ''); // If the parent transaction was not sampled we also stop the queue job from being recorded if ($context->getParentSampled() === false) { diff --git a/src/Sentry/Laravel/Tracing/Integrations/LighthouseIntegration.php b/src/Sentry/Laravel/Tracing/Integrations/LighthouseIntegration.php index a05f113c..dc443a35 100644 --- a/src/Sentry/Laravel/Tracing/Integrations/LighthouseIntegration.php +++ b/src/Sentry/Laravel/Tracing/Integrations/LighthouseIntegration.php @@ -13,6 +13,7 @@ use Sentry\Laravel\Integration; use Sentry\SentrySdk; use Sentry\Tracing\SpanContext; +use Sentry\Tracing\TransactionSource; class LighthouseIntegration implements IntegrationInterface { @@ -157,7 +158,7 @@ private function updateTransactionName(): void return; } - array_walk($groupedOperations, static function (array &$operations, string $operationType) { + array_walk($groupedOperations, static function (&$operations, string $operationType) { sort($operations, SORT_STRING); $operations = "{$operationType}{" . implode(',', $operations) . '}'; @@ -168,6 +169,7 @@ private function updateTransactionName(): void $transactionName = 'lighthouse?' . implode('&', $groupedOperations); $transaction->setName($transactionName); + $transaction->getMetadata()->setSource(TransactionSource::custom()); Integration::setTransaction($transactionName); } diff --git a/src/Sentry/Laravel/Tracing/Middleware.php b/src/Sentry/Laravel/Tracing/Middleware.php index d199e48e..589d1353 100644 --- a/src/Sentry/Laravel/Tracing/Middleware.php +++ b/src/Sentry/Laravel/Tracing/Middleware.php @@ -103,15 +103,13 @@ public function setBootedTimestamp(?float $timestamp = null): void private function startTransaction(Request $request, HubInterface $sentry): void { $requestStartTime = $request->server('REQUEST_TIME_FLOAT', microtime(true)); - $sentryTraceHeader = $request->header('sentry-trace'); - $baggageHeader = $request->header('baggage'); - $context = $sentryTraceHeader - ? TransactionContext::fromHeaders($sentryTraceHeader, $baggageHeader) - : new TransactionContext; + $context = TransactionContext::fromHeaders( + $request->header('sentry-trace', ''), + $request->header('baggage', '') + ); $context->setOp('http.server'); - $context->setSource(TransactionSource::url()); $context->setData([ 'url' => '/' . ltrim($request->path(), '/'), 'method' => strtoupper($request->method()), @@ -183,9 +181,9 @@ private function hydrateRequestData(Request $request): void $route = $request->route(); if ($route instanceof Route) { - $this->updateTransactionNameIfDefault( - Integration::extractNameForRoute($route) - ); + [$transactionName, $transactionSource] = Integration::extractNameAndSourceForRoute($route); + + $this->updateTransactionNameIfDefault($transactionName, $transactionSource); $this->transaction->setData([ 'name' => $route->getName(), @@ -193,9 +191,9 @@ private function hydrateRequestData(Request $request): void 'method' => $request->getMethod(), ]); } elseif (is_array($route) && count($route) === 3) { - $this->updateTransactionNameIfDefault( - Integration::extractNameForLumenRoute($route, $request->path()) - ); + [$transactionName, $transactionSource] = Integration::extractNameAndSourceForLumenRoute($route, $request->path()); + + $this->updateTransactionNameIfDefault($transactionName, $transactionSource); $action = $route[1] ?? []; @@ -206,7 +204,7 @@ private function hydrateRequestData(Request $request): void ]); } - $this->updateTransactionNameIfDefault('/' . ltrim($request->path(), '/')); + $this->updateTransactionNameIfDefault('/' . ltrim($request->path(), '/'), TransactionSource::url()); } private function hydrateResponseData(Response $response): void @@ -214,7 +212,7 @@ private function hydrateResponseData(Response $response): void $this->transaction->setHttpStatus($response->getStatusCode()); } - private function updateTransactionNameIfDefault(?string $name): void + private function updateTransactionNameIfDefault(?string $name, ?TransactionSource $source): void { // Ignore empty names (and `null`) for caller convenience if (empty($name)) { @@ -229,5 +227,6 @@ private function updateTransactionNameIfDefault(?string $name): void } $this->transaction->setName($name); + $this->transaction->getMetadata()->setSource($source ?? TransactionSource::custom()); } } From 00b9079f8ea864783f9211aaf27df331b7f9acce Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Wed, 5 Oct 2022 15:19:33 +0200 Subject: [PATCH 4/4] Add @internal to helpers on the Integration class --- src/Sentry/Laravel/Integration.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Sentry/Laravel/Integration.php b/src/Sentry/Laravel/Integration.php index 38e741b9..905d3622 100644 --- a/src/Sentry/Laravel/Integration.php +++ b/src/Sentry/Laravel/Integration.php @@ -124,6 +124,7 @@ public static function flushEvents(): void * * @return string * + * @internal This helper is used in various places to extra meaninful info from a Laravel Route object. * @deprecated This will be removed in version 3.0, use `extractNameAndSourceForRoute` instead. */ public static function extractNameForRoute(Route $route): string @@ -137,6 +138,8 @@ public static function extractNameForRoute(Route $route): string * @param \Illuminate\Routing\Route $route * * @return array{0: string, 1: \Sentry\Tracing\TransactionSource} + * + * @internal This helper is used in various places to extra meaninful info from a Laravel Route object. */ public static function extractNameAndSourceForRoute(Route $route): array { @@ -172,6 +175,7 @@ public static function extractNameAndSourceForRoute(Route $route): array * * @return string * + * @internal This helper is used in various places to extra meaninful info from a Lumen route data. * @deprecated This will be removed in version 3.0, use `extractNameAndSourceForLumenRoute` instead. */ public static function extractNameForLumenRoute(array $routeData, string $path): string @@ -186,6 +190,8 @@ public static function extractNameForLumenRoute(array $routeData, string $path): * @param string $path The path of the request * * @return array{0: string, 1: \Sentry\Tracing\TransactionSource} + * + * @internal This helper is used in various places to extra meaninful info from a Lumen route data. */ public static function extractNameAndSourceForLumenRoute(array $routeData, string $path): array {