From 2d45a88b907bed374479d0ba09de7ae477fcd285 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Thu, 27 Jul 2023 11:38:21 +0200 Subject: [PATCH] Add tracing for storage access (#726) Co-authored-by: Alex Bouma --- composer.json | 3 +- config/sentry.php | 6 + src/Sentry/Laravel/EventHandler.php | 2 +- .../Laravel/Features/Storage/Integration.php | 82 +++++++ .../Storage/SentryCloudFilesystem.php | 21 ++ .../Features/Storage/SentryFilesystem.php | 214 ++++++++++++++++++ src/Sentry/Laravel/ServiceProvider.php | 1 + src/Sentry/Laravel/Tracing/EventHandler.php | 2 +- .../Laravel/Tracing/ViewEngineDecorator.php | 2 +- src/Sentry/Laravel/Util/Filesize.php | 31 +++ test/Sentry/Features/CacheIntegrationTest.php | 4 +- .../Features/StorageIntegrationTest.php | 130 +++++++++++ test/Sentry/LogChannelTest.php | 4 +- test/Sentry/TestCaseExceptionHandler.php | 9 +- 14 files changed, 499 insertions(+), 12 deletions(-) create mode 100644 src/Sentry/Laravel/Features/Storage/Integration.php create mode 100644 src/Sentry/Laravel/Features/Storage/SentryCloudFilesystem.php create mode 100644 src/Sentry/Laravel/Features/Storage/SentryFilesystem.php create mode 100644 src/Sentry/Laravel/Util/Filesize.php create mode 100644 test/Sentry/Features/StorageIntegrationTest.php diff --git a/composer.json b/composer.json index 925f3882..0ef18c52 100644 --- a/composer.json +++ b/composer.json @@ -74,7 +74,8 @@ "minimum-stability": "dev", "config": { "allow-plugins": { - "kylekatarnls/update-helper": false + "kylekatarnls/update-helper": false, + "php-http/discovery": false } } } diff --git a/config/sentry.php b/config/sentry.php index b6ffc8bb..12514426 100644 --- a/config/sentry.php +++ b/config/sentry.php @@ -19,6 +19,9 @@ // Capture Laravel cache events in breadcrumbs 'cache' => true, + // Capture storage access as breadcrumbs + 'storage' => true, + // Capture Livewire components in breadcrumbs 'livewire' => true, @@ -54,6 +57,9 @@ // Capture views as spans 'views' => true, + // Capture storage access as spans + 'storage' => true, + // Capture Livewire components as spans 'livewire' => true, diff --git a/src/Sentry/Laravel/EventHandler.php b/src/Sentry/Laravel/EventHandler.php index bb01c578..09b5bcac 100644 --- a/src/Sentry/Laravel/EventHandler.php +++ b/src/Sentry/Laravel/EventHandler.php @@ -200,7 +200,7 @@ public function __call(string $method, array $arguments) } try { - call_user_func_array([$this, $handlerMethod], $arguments); + $this->{$handlerMethod}(...$arguments); } catch (Exception $exception) { // Ignore } diff --git a/src/Sentry/Laravel/Features/Storage/Integration.php b/src/Sentry/Laravel/Features/Storage/Integration.php new file mode 100644 index 00000000..a700c39c --- /dev/null +++ b/src/Sentry/Laravel/Features/Storage/Integration.php @@ -0,0 +1,82 @@ +isTracingFeatureEnabled(self::FEATURE_KEY) + || $this->isBreadcrumbFeatureEnabled(self::FEATURE_KEY); + } + + public function setup(): void + { + foreach (config('filesystems.disks') as $disk => $config) { + $currentDriver = $config['driver']; + + if ($currentDriver === self::STORAGE_DRIVER_NAME) { + continue; + } + + config([ + "filesystems.disks.{$disk}.driver" => self::STORAGE_DRIVER_NAME, + "filesystems.disks.{$disk}.sentry_disk_name" => $disk, + "filesystems.disks.{$disk}.sentry_original_driver" => $config['driver'], + ]); + } + + $this->container()->afterResolving(FilesystemManager::class, function (FilesystemManager $filesystemManager): void { + $filesystemManager->extend( + self::STORAGE_DRIVER_NAME, + function (Application $application, array $config) use ($filesystemManager): Filesystem { + if (empty($config['sentry_disk_name'])) { + throw new RuntimeException(sprintf('Missing `sentry_disk_name` config key for `%s` filesystem driver.', self::STORAGE_DRIVER_NAME)); + } + + if (empty($config['sentry_original_driver'])) { + throw new RuntimeException(sprintf('Missing `sentry_original_driver` config key for `%s` filesystem driver.', self::STORAGE_DRIVER_NAME)); + } + + if ($config['sentry_original_driver'] === self::STORAGE_DRIVER_NAME) { + throw new RuntimeException(sprintf('`sentry_original_driver` for Sentry storage integration cannot be the `%s` driver.', self::STORAGE_DRIVER_NAME)); + } + + $disk = $config['sentry_disk_name']; + + $config['driver'] = $config['sentry_original_driver']; + unset($config['sentry_original_driver']); + + $diskResolver = (function (string $disk, array $config) { + // This is a "hack" to make sure that the original driver is resolved by the FilesystemManager + config(["filesystems.disks.{$disk}" => $config]); + + return $this->resolve($disk); + })->bindTo($filesystemManager, FilesystemManager::class); + + $originalFilesystem = $diskResolver($disk, $config); + + $defaultData = ['disk' => $disk, 'driver' => $config['driver']]; + + $recordSpans = $this->isTracingFeatureEnabled(self::FEATURE_KEY); + $recordBreadcrumbs = $this->isBreadcrumbFeatureEnabled(self::FEATURE_KEY); + + return $originalFilesystem instanceof CloudFilesystem + ? new SentryCloudFilesystem($originalFilesystem, $defaultData, $recordSpans, $recordBreadcrumbs) + : new SentryFilesystem($originalFilesystem, $defaultData, $recordSpans, $recordBreadcrumbs); + } + ); + }); + } +} diff --git a/src/Sentry/Laravel/Features/Storage/SentryCloudFilesystem.php b/src/Sentry/Laravel/Features/Storage/SentryCloudFilesystem.php new file mode 100644 index 00000000..418be57a --- /dev/null +++ b/src/Sentry/Laravel/Features/Storage/SentryCloudFilesystem.php @@ -0,0 +1,21 @@ +withSentry(__FUNCTION__, func_get_args(), $path, compact('path')); + } +} diff --git a/src/Sentry/Laravel/Features/Storage/SentryFilesystem.php b/src/Sentry/Laravel/Features/Storage/SentryFilesystem.php new file mode 100644 index 00000000..94521927 --- /dev/null +++ b/src/Sentry/Laravel/Features/Storage/SentryFilesystem.php @@ -0,0 +1,214 @@ +filesystem = $filesystem; + $this->defaultData = $defaultData; + $this->recordSpans = $recordSpans; + $this->recordBreadcrumbs = $recordBreadcrumbs; + } + + /** + * Execute the method on the underlying filesystem and wrap it with tracing and log a breadcrumb. + * + * @param list $args + * @param array $data + * + * @return mixed + */ + protected function withSentry(string $method, array $args, string $description, array $data) + { + $op = "file.{$method}"; // See https://develop.sentry.dev/sdk/performance/span-operations/#web-server + $data = array_merge($data, $this->defaultData); + + if ($this->recordBreadcrumbs) { + Integration::addBreadcrumb(new Breadcrumb( + Breadcrumb::LEVEL_INFO, + Breadcrumb::TYPE_DEFAULT, + $op, + $description, + $data + )); + } + + if ($this->recordSpans) { + $spanContext = new SpanContext; + $spanContext->setOp($op); + $spanContext->setData($data); + $spanContext->setDescription($description); + + return trace(function () use ($method, $args) { + return $this->filesystem->{$method}(...$args); + }, $spanContext); + } + + return $this->filesystem->{$method}(...$args); + } + + /** @see \Illuminate\Filesystem\FilesystemAdapter::assertExists() */ + public function assertExists($path, $content = null) + { + return $this->withSentry(__FUNCTION__, func_get_args(), $path, compact('path')); + } + + /** @see \Illuminate\Filesystem\FilesystemAdapter::assertMissing() */ + public function assertMissing($path) + { + return $this->withSentry(__FUNCTION__, func_get_args(), $path, compact('path')); + } + + /** @see \Illuminate\Filesystem\FilesystemAdapter::assertDirectoryEmpty() */ + public function assertDirectoryEmpty($path) + { + return $this->withSentry(__FUNCTION__, func_get_args(), $path, compact('path')); + } + + public function exists($path) + { + return $this->withSentry(__FUNCTION__, func_get_args(), $path, compact('path')); + } + + public function get($path) + { + return $this->withSentry(__FUNCTION__, func_get_args(), $path, compact('path')); + } + + public function readStream($path) + { + return $this->withSentry(__FUNCTION__, func_get_args(), $path, compact('path')); + } + + public function put($path, $contents, $options = []) + { + $description = is_string($contents) ? sprintf('%s (%s)', $path, Filesize::toHuman(strlen($contents))) : $path; + + return $this->withSentry(__FUNCTION__, func_get_args(), $description, compact('path', 'options')); + } + + public function writeStream($path, $resource, array $options = []) + { + return $this->withSentry(__FUNCTION__, func_get_args(), $path, compact('path', 'options')); + } + + public function getVisibility($path) + { + return $this->withSentry(__FUNCTION__, func_get_args(), $path, compact('path')); + } + + public function setVisibility($path, $visibility) + { + return $this->withSentry(__FUNCTION__, func_get_args(), $path, compact('path', 'visibility')); + } + + public function prepend($path, $data) + { + $description = is_string($data) ? sprintf('%s (%s)', $path, Filesize::toHuman(strlen($data))) : $path; + + return $this->withSentry(__FUNCTION__, func_get_args(), $description, compact('path')); + } + + public function append($path, $data) + { + $description = is_string($data) ? sprintf('%s (%s)', $path, Filesize::toHuman(strlen($data))) : $path; + + return $this->withSentry(__FUNCTION__, func_get_args(), $description, compact('path')); + } + + public function delete($paths) + { + if (is_array($paths)) { + $data = compact('paths'); + $description = sprintf('%s paths', count($paths)); + } else { + $data = ['path' => $paths]; + $description = $paths; + } + + return $this->withSentry(__FUNCTION__, func_get_args(), $description, $data); + } + + public function copy($from, $to) + { + return $this->withSentry(__FUNCTION__, func_get_args(), sprintf('from "%s" to "%s"', $from, $to), compact('from', 'to')); + } + + public function move($from, $to) + { + return $this->withSentry(__FUNCTION__, func_get_args(), sprintf('from "%s" to "%s"', $from, $to), compact('from', 'to')); + } + + public function size($path) + { + return $this->withSentry(__FUNCTION__, func_get_args(), $path, compact('path')); + } + + public function lastModified($path) + { + return $this->withSentry(__FUNCTION__, func_get_args(), $path, compact('path')); + } + + public function files($directory = null, $recursive = false) + { + return $this->withSentry(__FUNCTION__, func_get_args(), $directory, compact('directory', 'recursive')); + } + + public function allFiles($directory = null) + { + return $this->withSentry(__FUNCTION__, func_get_args(), $directory, compact('directory')); + } + + public function directories($directory = null, $recursive = false) + { + return $this->withSentry(__FUNCTION__, func_get_args(), $directory, compact('directory', 'recursive')); + } + + public function allDirectories($directory = null) + { + return $this->withSentry(__FUNCTION__, func_get_args(), $directory, compact('directory')); + } + + public function makeDirectory($path) + { + return $this->withSentry(__FUNCTION__, func_get_args(), $path, compact('path')); + } + + public function deleteDirectory($directory) + { + return $this->withSentry(__FUNCTION__, func_get_args(), $directory, compact('directory')); + } + + public function __call($name, $arguments) + { + return $this->filesystem->{$name}(...$arguments); + } +} diff --git a/src/Sentry/Laravel/ServiceProvider.php b/src/Sentry/Laravel/ServiceProvider.php index c48a3c01..88fafd14 100644 --- a/src/Sentry/Laravel/ServiceProvider.php +++ b/src/Sentry/Laravel/ServiceProvider.php @@ -54,6 +54,7 @@ class ServiceProvider extends BaseServiceProvider Features\CacheIntegration::class, Features\QueueIntegration::class, Features\ConsoleIntegration::class, + Features\Storage\Integration::class, Features\LivewirePackageIntegration::class, ]; diff --git a/src/Sentry/Laravel/Tracing/EventHandler.php b/src/Sentry/Laravel/Tracing/EventHandler.php index 768794d9..18855e41 100644 --- a/src/Sentry/Laravel/Tracing/EventHandler.php +++ b/src/Sentry/Laravel/Tracing/EventHandler.php @@ -146,7 +146,7 @@ public function __call(string $method, array $arguments) } try { - call_user_func_array([$this, $handlerMethod], $arguments); + $this->{$handlerMethod}(...$arguments); } catch (Exception $e) { // Ignore to prevent bubbling up errors in the SDK } diff --git a/src/Sentry/Laravel/Tracing/ViewEngineDecorator.php b/src/Sentry/Laravel/Tracing/ViewEngineDecorator.php index a153243f..593964ca 100644 --- a/src/Sentry/Laravel/Tracing/ViewEngineDecorator.php +++ b/src/Sentry/Laravel/Tracing/ViewEngineDecorator.php @@ -53,6 +53,6 @@ public function get($path, array $data = []): string public function __call($name, $arguments) { - return call_user_func_array([$this->engine, $name], $arguments); + return $this->engine->{$name}(...$arguments); } } diff --git a/src/Sentry/Laravel/Util/Filesize.php b/src/Sentry/Laravel/Util/Filesize.php new file mode 100644 index 00000000..d347ee6c --- /dev/null +++ b/src/Sentry/Laravel/Util/Filesize.php @@ -0,0 +1,31 @@ +assertEquals("Missed: {$key}", $this->getLastBreadcrumb()->getMessage()); } - public function testCacheBreadcrumIsNotRecordedWhenDisabled(): void + public function testCacheBreadcrumbIsNotRecordedWhenDisabled(): void { $this->resetApplicationWithConfig([ 'sentry.breadcrumbs.cache' => false, diff --git a/test/Sentry/Features/StorageIntegrationTest.php b/test/Sentry/Features/StorageIntegrationTest.php new file mode 100644 index 00000000..b7e0e010 --- /dev/null +++ b/test/Sentry/Features/StorageIntegrationTest.php @@ -0,0 +1,130 @@ +getHubFromContainer(); + + $transaction = $hub->startTransaction(new TransactionContext); + $transaction->initSpanRecorder(); + + $this->getCurrentScope()->setSpan($transaction); + + Storage::put('foo', 'bar'); + $fooContent = Storage::get('foo'); + Storage::assertExists('foo', 'bar'); + Storage::delete('foo'); + Storage::delete(['foo', 'bar']); + + $spans = $transaction->getSpanRecorder()->getSpans(); + + $this->assertArrayHasKey(1, $spans); + $span = $spans[1]; + $this->assertSame('file.put', $span->getOp()); + $this->assertSame('foo (3 B)', $span->getDescription()); + $this->assertSame(['path' => 'foo', 'options' => [], 'disk' => 'local', 'driver' => 'local'], $span->getData()); + + $this->assertArrayHasKey(2, $spans); + $span = $spans[2]; + $this->assertSame('file.get', $span->getOp()); + $this->assertSame('foo', $span->getDescription()); + $this->assertSame(['path' => 'foo', 'disk' => 'local', 'driver' => 'local'], $span->getData()); + $this->assertSame('bar', $fooContent); + + $this->assertArrayHasKey(3, $spans); + $span = $spans[3]; + $this->assertSame('file.assertExists', $span->getOp()); + $this->assertSame('foo', $span->getDescription()); + $this->assertSame(['path' => 'foo', 'disk' => 'local', 'driver' => 'local'], $span->getData()); + + $this->assertArrayHasKey(4, $spans); + $span = $spans[4]; + $this->assertSame('file.delete', $span->getOp()); + $this->assertSame('foo', $span->getDescription()); + $this->assertSame(['path' => 'foo', 'disk' => 'local', 'driver' => 'local'], $span->getData()); + + $this->assertArrayHasKey(5, $spans); + $span = $spans[5]; + $this->assertSame('file.delete', $span->getOp()); + $this->assertSame('2 paths', $span->getDescription()); + $this->assertSame(['paths' => ['foo', 'bar'], 'disk' => 'local', 'driver' => 'local'], $span->getData()); + } + + public function testDoesntCreateSpansWhenDisabled(): void + { + $this->resetApplicationWithConfig([ + 'sentry.tracing.storage' => false, + ]); + + $hub = $this->getHubFromContainer(); + + $transaction = $hub->startTransaction(new TransactionContext); + $transaction->initSpanRecorder(); + + $this->getCurrentScope()->setSpan($transaction); + + Storage::exists('foo'); + + $this->assertCount(1, $transaction->getSpanRecorder()->getSpans()); + } + + public function testCreatesBreadcrumbsFor(): void + { + Storage::put('foo', 'bar'); + $fooContent = Storage::get('foo'); + Storage::assertExists('foo', 'bar'); + Storage::delete('foo'); + Storage::delete(['foo', 'bar']); + + $breadcrumbs = $this->getCurrentBreadcrumbs(); + + $this->assertArrayHasKey(0, $breadcrumbs); + $span = $breadcrumbs[0]; + $this->assertSame('file.put', $span->getCategory()); + $this->assertSame('foo (3 B)', $span->getMessage()); + $this->assertSame(['path' => 'foo', 'options' => [], 'disk' => 'local', 'driver' => 'local'], $span->getMetadata()); + + $this->assertArrayHasKey(1, $breadcrumbs); + $span = $breadcrumbs[1]; + $this->assertSame('file.get', $span->getCategory()); + $this->assertSame('foo', $span->getMessage()); + $this->assertSame(['path' => 'foo', 'disk' => 'local', 'driver' => 'local'], $span->getMetadata()); + $this->assertSame('bar', $fooContent); + + $this->assertArrayHasKey(2, $breadcrumbs); + $span = $breadcrumbs[2]; + $this->assertSame('file.assertExists', $span->getCategory()); + $this->assertSame('foo', $span->getMessage()); + $this->assertSame(['path' => 'foo', 'disk' => 'local', 'driver' => 'local'], $span->getMetadata()); + + $this->assertArrayHasKey(3, $breadcrumbs); + $span = $breadcrumbs[3]; + $this->assertSame('file.delete', $span->getCategory()); + $this->assertSame('foo', $span->getMessage()); + $this->assertSame(['path' => 'foo', 'disk' => 'local', 'driver' => 'local'], $span->getMetadata()); + + $this->assertArrayHasKey(4, $breadcrumbs); + $span = $breadcrumbs[4]; + $this->assertSame('file.delete', $span->getCategory()); + $this->assertSame('2 paths', $span->getMessage()); + $this->assertSame(['paths' => ['foo', 'bar'], 'disk' => 'local', 'driver' => 'local'], $span->getMetadata()); + } + + public function testDoesntCreateBreadcrumbsWhenDisabled(): void + { + $this->resetApplicationWithConfig([ + 'sentry.breadcrumbs.storage' => false, + ]); + + Storage::exists('foo'); + + $this->assertCount(0, $this->getCurrentBreadcrumbs()); + } +} diff --git a/test/Sentry/LogChannelTest.php b/test/Sentry/LogChannelTest.php index 2844b02a..728d711a 100644 --- a/test/Sentry/LogChannelTest.php +++ b/test/Sentry/LogChannelTest.php @@ -28,7 +28,9 @@ public function testCreatingHandlerWithActionLevelConfig(): void $currentHandler = current($logger->getHandlers()); - $this->assertInstanceOf(SentryHandler::class, $currentHandler->getHandler()); + if (method_exists($currentHandler, 'getHandler')) { + $this->assertInstanceOf(SentryHandler::class, $currentHandler->getHandler()); + } $loggerWithoutActionLevel = $logChannel(['action_level' => null]); diff --git a/test/Sentry/TestCaseExceptionHandler.php b/test/Sentry/TestCaseExceptionHandler.php index e256e8da..ff285f4d 100644 --- a/test/Sentry/TestCaseExceptionHandler.php +++ b/test/Sentry/TestCaseExceptionHandler.php @@ -6,12 +6,11 @@ use Sentry\Laravel\Integration; /** - * This is a proxy class so we can inject the Sentry bits while running tests and handle exceptions like "normal". - * - * All type hints are remove from this class to prevent issues when running lower PHP versions where Throwable is not yet a thing. + * This is a proxy class, so we can inject the Sentry bits while running tests and handle exceptions like "normal". */ class TestCaseExceptionHandler implements ExceptionHandler { + /** @var ExceptionHandler */ private $handler; public function __construct(ExceptionHandler $handler) @@ -38,11 +37,11 @@ public function render($request, $e) public function renderForConsole($output, $e) { - $this->handler->render($output, $e); + $this->handler->renderForConsole($output, $e); } public function __call($name, $arguments) { - return call_user_func_array([$this->handler, $name], $arguments); + return $this->handler->{$name}(...$arguments); } }