From fd85d9b4246bf94e2bce9ddbdb355d28db468b3f Mon Sep 17 00:00:00 2001 From: Michi Hoffmann Date: Tue, 11 Oct 2022 11:59:34 +0200 Subject: [PATCH] Revert "feat: Add support for Dynamic Sampling (#572)" (#584) This reverts commit b9bb0e361b1bcfc48d1d2ac85a21ac8e57907344. --- composer.json | 2 +- src/Sentry/Laravel/Console/TestCommand.php | 1 - src/Sentry/Laravel/EventHandler.php | 2 +- src/Sentry/Laravel/Integration.php | 76 ++----------------- src/Sentry/Laravel/Tracing/EventHandler.php | 10 +-- .../Integrations/LighthouseIntegration.php | 4 +- src/Sentry/Laravel/Tracing/Middleware.php | 26 +++---- 7 files changed, 25 insertions(+), 96 deletions(-) diff --git a/composer.json b/composer.json index e95d2a4e..cd152ece 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.9", + "sentry/sentry": "^3.3", "sentry/sdk": "^3.1", "symfony/psr-http-message-bridge": "^1.0 | ^2.0", "nyholm/psr7": "^1.0" diff --git a/src/Sentry/Laravel/Console/TestCommand.php b/src/Sentry/Laravel/Console/TestCommand.php index 24a85776..d921317d 100644 --- a/src/Sentry/Laravel/Console/TestCommand.php +++ b/src/Sentry/Laravel/Console/TestCommand.php @@ -145,7 +145,6 @@ 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/EventHandler.php b/src/Sentry/Laravel/EventHandler.php index 8db2bb1d..f9f32bfe 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::extractNameAndSourceForRoute($route); + $routeName = Integration::extractNameForRoute($route); Integration::addBreadcrumb(new Breadcrumb( Breadcrumb::LEVEL_INFO, diff --git a/src/Sentry/Laravel/Integration.php b/src/Sentry/Laravel/Integration.php index 905d3622..f400aaf5 100644 --- a/src/Sentry/Laravel/Integration.php +++ b/src/Sentry/Laravel/Integration.php @@ -6,7 +6,6 @@ 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; @@ -123,48 +122,27 @@ public static function flushEvents(): void * @param \Illuminate\Routing\Route $route * * @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 { - 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} - * - * @internal This helper is used in various places to extra meaninful info from a Laravel Route object. - */ - public static function extractNameAndSourceForRoute(Route $route): array - { - $source = null; $routeName = null; - // some.action (route name/alias) + // someaction (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()); } - // /some/{action} // Fallback to the route uri (with parameter placeholders) + // /someaction // Fallback to the url if (empty($routeName) || $routeName === 'Closure') { - $source = TransactionSource::route(); $routeName = '/' . ltrim($route->uri(), '/'); } - return [$routeName, $source]; + return $routeName; } /** @@ -174,45 +152,24 @@ public static function extractNameAndSourceForRoute(Route $route): array * @param string $path The path of the request * * @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 { - 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} - * - * @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 - { - $source = null; $routeName = null; $route = $routeData[1] ?? []; - // some.action (route name/alias) + // someaction (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']); } - // /some/{action} // Fallback to the route uri (with parameter placeholders) + // /someaction // Fallback to the url if (empty($routeName) || $routeName === 'Closure') { $routeUri = array_reduce( array_keys($routeData[2]), @@ -222,11 +179,10 @@ static function ($carry, $key) use ($routeData) { $path ); - $source = TransactionSource::url(); $routeName = '/' . ltrim($routeUri, '/'); } - return [$routeName, $source]; + return $routeName; } /** @@ -291,25 +247,7 @@ public static function sentryTracingMeta(): string } $content = sprintf('', $span->toTraceparent()); - - 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()); + // $content .= sprintf('', $span->getDescription()); return $content; } diff --git a/src/Sentry/Laravel/Tracing/EventHandler.php b/src/Sentry/Laravel/Tracing/EventHandler.php index 1fafb3bd..054dc62e 100644 --- a/src/Sentry/Laravel/Tracing/EventHandler.php +++ b/src/Sentry/Laravel/Tracing/EventHandler.php @@ -16,14 +16,11 @@ 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. * @@ -156,7 +153,6 @@ 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; @@ -297,10 +293,11 @@ protected function queueJobProcessingHandler(QueueEvents\JobProcessing $event) } if ($parentSpan === null) { - $baggage = $event->job->payload()[self::QUEUE_PAYLOAD_BAGGAGE_DATA] ?? null; $traceParent = $event->job->payload()[self::QUEUE_PAYLOAD_TRACE_PARENT_DATA] ?? null; - $context = TransactionContext::fromHeaders($traceParent ?? '', $baggage ?? ''); + $context = $traceParent === null + ? new TransactionContext + : TransactionContext::fromSentryTrace($traceParent); // If the parent transaction was not sampled we also stop the queue job from being recorded if ($context->getParentSampled() === false) { @@ -328,7 +325,6 @@ 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/Integrations/LighthouseIntegration.php b/src/Sentry/Laravel/Tracing/Integrations/LighthouseIntegration.php index dc443a35..a05f113c 100644 --- a/src/Sentry/Laravel/Tracing/Integrations/LighthouseIntegration.php +++ b/src/Sentry/Laravel/Tracing/Integrations/LighthouseIntegration.php @@ -13,7 +13,6 @@ use Sentry\Laravel\Integration; use Sentry\SentrySdk; use Sentry\Tracing\SpanContext; -use Sentry\Tracing\TransactionSource; class LighthouseIntegration implements IntegrationInterface { @@ -158,7 +157,7 @@ private function updateTransactionName(): void return; } - array_walk($groupedOperations, static function (&$operations, string $operationType) { + array_walk($groupedOperations, static function (array &$operations, string $operationType) { sort($operations, SORT_STRING); $operations = "{$operationType}{" . implode(',', $operations) . '}'; @@ -169,7 +168,6 @@ 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 589d1353..24eb8341 100644 --- a/src/Sentry/Laravel/Tracing/Middleware.php +++ b/src/Sentry/Laravel/Tracing/Middleware.php @@ -11,7 +11,6 @@ use Sentry\Tracing\Span; use Sentry\Tracing\SpanContext; use Sentry\Tracing\TransactionContext; -use Sentry\Tracing\TransactionSource; use Symfony\Component\HttpFoundation\Response; class Middleware @@ -103,11 +102,11 @@ 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'); - $context = TransactionContext::fromHeaders( - $request->header('sentry-trace', ''), - $request->header('baggage', '') - ); + $context = $sentryTraceHeader + ? TransactionContext::fromSentryTrace($sentryTraceHeader) + : new TransactionContext; $context->setOp('http.server'); $context->setData([ @@ -181,9 +180,9 @@ private function hydrateRequestData(Request $request): void $route = $request->route(); if ($route instanceof Route) { - [$transactionName, $transactionSource] = Integration::extractNameAndSourceForRoute($route); - - $this->updateTransactionNameIfDefault($transactionName, $transactionSource); + $this->updateTransactionNameIfDefault( + Integration::extractNameForRoute($route) + ); $this->transaction->setData([ 'name' => $route->getName(), @@ -191,9 +190,9 @@ private function hydrateRequestData(Request $request): void 'method' => $request->getMethod(), ]); } elseif (is_array($route) && count($route) === 3) { - [$transactionName, $transactionSource] = Integration::extractNameAndSourceForLumenRoute($route, $request->path()); - - $this->updateTransactionNameIfDefault($transactionName, $transactionSource); + $this->updateTransactionNameIfDefault( + Integration::extractNameForLumenRoute($route, $request->path()) + ); $action = $route[1] ?? []; @@ -204,7 +203,7 @@ private function hydrateRequestData(Request $request): void ]); } - $this->updateTransactionNameIfDefault('/' . ltrim($request->path(), '/'), TransactionSource::url()); + $this->updateTransactionNameIfDefault('/' . ltrim($request->path(), '/')); } private function hydrateResponseData(Response $response): void @@ -212,7 +211,7 @@ private function hydrateResponseData(Response $response): void $this->transaction->setHttpStatus($response->getStatusCode()); } - private function updateTransactionNameIfDefault(?string $name, ?TransactionSource $source): void + private function updateTransactionNameIfDefault(?string $name): void { // Ignore empty names (and `null`) for caller convenience if (empty($name)) { @@ -227,6 +226,5 @@ private function updateTransactionNameIfDefault(?string $name, ?TransactionSourc } $this->transaction->setName($name); - $this->transaction->getMetadata()->setSource($source ?? TransactionSource::custom()); } }