From 55cdea8507ac7c3d3d96f810dee086143612bd68 Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Wed, 6 Jul 2022 11:46:00 +0100 Subject: [PATCH 1/4] Do not invoke and enable extension if monitoring is disabled --- src/Agent.php | 37 ++++++++++++++++++- .../DoNotInvokeAnyExtensionCapabilities.php | 26 +++++++++++++ ...oNotInvokeAnyExtensionCapabilitiesTest.php | 33 +++++++++++++++++ 3 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 src/Extension/DoNotInvokeAnyExtensionCapabilities.php create mode 100644 tests/Unit/Extension/DoNotInvokeAnyExtensionCapabilitiesTest.php diff --git a/src/Agent.php b/src/Agent.php index 687608fc..657cdd2d 100644 --- a/src/Agent.php +++ b/src/Agent.php @@ -31,6 +31,7 @@ use Scoutapm\Events\Span\Span; use Scoutapm\Events\Span\SpanReference; use Scoutapm\Events\Tag\Tag; +use Scoutapm\Extension\DoNotInvokeAnyExtensionCapabilities; use Scoutapm\Extension\ExtensionCapabilities; use Scoutapm\Extension\PotentiallyAvailableExtensionCapabilities; use Scoutapm\Extension\Version; @@ -149,6 +150,40 @@ private static function createConnectorFromConfig(Config $config): SocketConnect ); } + /** + * The {@see PotentiallyAvailableExtensionCapabilities} constructor implementation automatically invokes the + * {@see \scoutapm_enable_instrumentation()} function, which enables the extension, if it is available. However, if + * the extension is present, but monitoring is disabled, the extension is still enabled. Therefore, when setting up + * the Agent, if monitoring is NOT enabled, then use the new alternate implementation in + * {@see DoNotInvokeAnyExtensionCapabilities} which is essentially a no-op. This means the extension will remain + * dormant if monitoring is disabled. + */ + private static function createExtensionCapabilitiesFromConfig(Config $config): ExtensionCapabilities + { + return $config->get(ConfigKey::MONITORING_ENABLED) + ? new PotentiallyAvailableExtensionCapabilities() + : new DoNotInvokeAnyExtensionCapabilities(); + } + + /** + * The only required arguments are the {@see Config}, and a {@see LoggerInterface} implementation. + * + * @example + * + * $agent = Agent::fromConfig( + * Config::fromArray([]), // Uses default configuration, which can be overridden with environment variables + * $logger // PSR-3 compatible logger, e.g. Monolog, or most framework loggers + * ); + * + * If any of the other parameters are not provided, default configuration is used, which should be fine for most + * usual cases. Default implementations are as follows: + * + * - $cache - {@see DevNullCache} - i.e. no caching + * - $connector - {@see SocketConnector} - supports both UNIX sockets and TCP sockets + * - $extensionCapabilities - {@see PotentiallyAvailableExtensionCapabilities} or {@see DoNotInvokeAnyExtensionCapabilities} depending on configuration + * - $locateFileOrFolder - {@see LocateFileOrFolderUsingFilesystem} + * - $errorHandling - {@see ScoutErrorHandling} or {@see NoErrorHandling} depending on configuration + */ public static function fromConfig( Config $config, LoggerInterface $logger, @@ -171,7 +206,7 @@ public static function fromConfig( $config, $connector ?? self::createConnectorFromConfig($config), $logger, - $extensionCapabilities ?? new PotentiallyAvailableExtensionCapabilities(), + $extensionCapabilities ?? self::createExtensionCapabilitiesFromConfig($config), $cache ?? new DevNullCache(), $locateFileOrFolder ?? new LocateFileOrFolderUsingFilesystem(), $errorHandling ?? ErrorHandlingDiscoveryFactory::createAndListen($config, $logger, $superglobals), diff --git a/src/Extension/DoNotInvokeAnyExtensionCapabilities.php b/src/Extension/DoNotInvokeAnyExtensionCapabilities.php new file mode 100644 index 00000000..fce42c09 --- /dev/null +++ b/src/Extension/DoNotInvokeAnyExtensionCapabilities.php @@ -0,0 +1,26 @@ + */ + public function getCalls(): array + { + // Intentionally, there are no calls + return []; + } + + public function clearRecordedCalls(): void + { + // Intentially no-op, don't invoke the extension + } + + public function version(): ?Version + { + // Extension is ignored/doesn't exist, so no version to return + return null; + } +} diff --git a/tests/Unit/Extension/DoNotInvokeAnyExtensionCapabilitiesTest.php b/tests/Unit/Extension/DoNotInvokeAnyExtensionCapabilitiesTest.php new file mode 100644 index 00000000..fa0e05c0 --- /dev/null +++ b/tests/Unit/Extension/DoNotInvokeAnyExtensionCapabilitiesTest.php @@ -0,0 +1,33 @@ +getCalls()); + $capabilities->clearRecordedCalls(); + self::assertEquals([], $capabilities->getCalls()); + } + + public function testGetCalls(): void + { + file_get_contents(__FILE__); + self::assertEquals([], (new DoNotInvokeAnyExtensionCapabilities())->getCalls()); + } + + public function testVersion(): void + { + self::assertNull((new DoNotInvokeAnyExtensionCapabilities())->version()); + } +} From 7eb08791ac84c0e34b695b36aadc5a611a8f55e7 Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Wed, 6 Jul 2022 08:24:11 +0100 Subject: [PATCH 2/4] Allow all plugins to run by default in composer.json --- composer.json | 5 +---- composer.lock | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/composer.json b/composer.json index 47d5fe63..a82ae8c1 100644 --- a/composer.json +++ b/composer.json @@ -80,9 +80,6 @@ "config": { "preferred-install": "dist", "sort-packages": true, - "allow-plugins": { - "dealerdirect/phpcodesniffer-composer-installer": true, - "composer/package-versions-deprecated": true - } + "allow-plugins": true } } diff --git a/composer.lock b/composer.lock index b687a49d..fbf23463 100644 --- a/composer.lock +++ b/composer.lock @@ -11099,5 +11099,5 @@ "platform-dev": { "composer-plugin-api": "^2.0" }, - "plugin-api-version": "2.2.0" + "plugin-api-version": "2.3.0" } From 1215d8dd53d4be436b3ad24cf332f65a3b1e562d Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Wed, 6 Jul 2022 08:27:47 +0100 Subject: [PATCH 3/4] Set path.storage for Laravel/Lumen tests --- .../ScoutApmServiceProviderUsingLaravelTest.php | 8 ++++++++ .../ScoutApmServiceProviderUsingLumenTest.php | 11 +++++++++++ 2 files changed, 19 insertions(+) diff --git a/tests/Unit/Laravel/Providers/ScoutApmServiceProviderUsingLaravelTest.php b/tests/Unit/Laravel/Providers/ScoutApmServiceProviderUsingLaravelTest.php index d5ace058..5d5b7015 100644 --- a/tests/Unit/Laravel/Providers/ScoutApmServiceProviderUsingLaravelTest.php +++ b/tests/Unit/Laravel/Providers/ScoutApmServiceProviderUsingLaravelTest.php @@ -20,8 +20,12 @@ use Scoutapm\Logger\FilteredLogLevelDecorator; use function class_exists; +use function mkdir; use function sprintf; use function sys_get_temp_dir; +use function uniqid; + +use const DIRECTORY_SEPARATOR; /** @covers \Scoutapm\Laravel\Providers\ScoutApmServiceProvider */ final class ScoutApmServiceProviderUsingLaravelTest extends ScoutApmServiceProviderTestBase @@ -64,6 +68,10 @@ static function () use ($application): FilteredLogLevelDecorator { } ); + $storagePath = sys_get_temp_dir() . DIRECTORY_SEPARATOR . uniqid('scoutapm-test-laravel-storage', true) . DIRECTORY_SEPARATOR; + mkdir($storagePath); + $application->useStoragePath($storagePath); + $application->singleton( HttpKernelInterface::class, function () use ($application): HttpKernelInterface { diff --git a/tests/Unit/Laravel/Providers/ScoutApmServiceProviderUsingLumenTest.php b/tests/Unit/Laravel/Providers/ScoutApmServiceProviderUsingLumenTest.php index 449e80e0..4b8c64fc 100644 --- a/tests/Unit/Laravel/Providers/ScoutApmServiceProviderUsingLumenTest.php +++ b/tests/Unit/Laravel/Providers/ScoutApmServiceProviderUsingLumenTest.php @@ -18,8 +18,13 @@ use Scoutapm\Logger\FilteredLogLevelDecorator; use function class_exists; +use function method_exists; +use function mkdir; use function sprintf; use function sys_get_temp_dir; +use function uniqid; + +use const DIRECTORY_SEPARATOR; /** @covers \Scoutapm\Laravel\Providers\ScoutApmServiceProvider */ final class ScoutApmServiceProviderUsingLumenTest extends ScoutApmServiceProviderTestBase @@ -80,6 +85,12 @@ static function () use ($application): FilteredLogLevelDecorator { } ); + if (method_exists($application, 'useStoragePath')) { + $storagePath = sys_get_temp_dir() . DIRECTORY_SEPARATOR . uniqid('scoutapm-test-lumen-storage', true) . DIRECTORY_SEPARATOR; + mkdir($storagePath); + $application->useStoragePath($storagePath); + } + $application->make('view'); $application->singleton( 'view', From a3f64f4f516b8c9c160e7c3176d6931b4bd83f2b Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Wed, 6 Jul 2022 09:18:56 +0100 Subject: [PATCH 4/4] Configure Composer to allow plugins when cold-installing projects --- .github/workflows/continuous-integration.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml index a87fbce8..caf0767a 100644 --- a/.github/workflows/continuous-integration.yml +++ b/.github/workflows/continuous-integration.yml @@ -459,6 +459,8 @@ jobs: extensions: ${{ matrix.extensions }} env: fail-fast: true + - name: "Composer allow-plugins to allow Laravel to actually install" + run: composer config --global allow-plugins true - name: "Install Laravel quickstart project" run: "composer create-project laravel/laravel:${{ matrix.laravel-version}} test-app --prefer-dist" - name: "Add scout-apm-php as a repository" @@ -646,6 +648,8 @@ jobs: extensions: ${{ matrix.extensions }} env: fail-fast: true + - name: "Composer allow-plugins to allow Laravel to actually install" + run: composer config --global allow-plugins true - name: "Install Lumen quickstart project" run: "composer create-project laravel/lumen:${{ matrix.lumen-version}} test-app --prefer-dist" - name: "Add scout-apm-php as a repository"