diff --git a/composer.json b/composer.json index ebe23301..f8c29c5f 100644 --- a/composer.json +++ b/composer.json @@ -9,6 +9,8 @@ "php": ">=7.1.0,<7.5.0", "ext-json": "*", "ext-sockets": "*", + "ext-zlib": "*", + "ext-openssl": "*", "ocramius/package-versions": "^1.4", "psr/log": "^1.0", "ralouphie/getallheaders": "^2.0.5|^3.0", @@ -24,7 +26,8 @@ "vimeo/psalm": "^3.4" }, "suggest": { - "ext-xdebug": "Required for processing of request headers" + "ext-xdebug": "Required for processing of request headers", + "ext-scoutapm": "Recommended for additional recording capability of IO-bound PHP internal functions" }, "autoload": { "psr-4": { diff --git a/composer.lock b/composer.lock index d0b5f04a..970855e5 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "8f65a1f4fb12631669304baea15e3e31", + "content-hash": "fdca33732dd56bfc62f395cc59df48bb", "packages": [ { "name": "ocramius/package-versions", @@ -3498,7 +3498,9 @@ "platform": { "php": ">=7.1.0,<7.5.0", "ext-json": "*", - "ext-sockets": "*" + "ext-sockets": "*", + "ext-zlib": "*", + "ext-openssl": "*" }, "platform-dev": [], "platform-overrides": { diff --git a/src/Agent.php b/src/Agent.php index 65080187..ab6a538e 100644 --- a/src/Agent.php +++ b/src/Agent.php @@ -184,7 +184,8 @@ private function addSpansFromExtension() : void } foreach ($this->phpExtension->getCalls() as $recordedCall) { - $this->request->startSpan($recordedCall->functionName(), $recordedCall->timeEntered()); + $callSpan = $this->request->startSpan($recordedCall->functionName(), $recordedCall->timeEntered()); + $callSpan->tag('desc', $recordedCall->filteredArguments()); $this->request->stopSpan($recordedCall->timeExited()); } } diff --git a/src/Extension/RecordedCall.php b/src/Extension/RecordedCall.php index 01517e1d..b8e338c4 100644 --- a/src/Extension/RecordedCall.php +++ b/src/Extension/RecordedCall.php @@ -20,18 +20,30 @@ final class RecordedCall /** @var float */ private $timeExited; - private function __construct(string $function, float $timeTakenInSeconds, float $timeEntered, float $timeExited) - { + /** @var mixed[] */ + private $arguments; + + /** @param mixed[] $arguments */ + private function __construct( + string $function, + float $timeTakenInSeconds, + float $timeEntered, + float $timeExited, + array $arguments + ) { $this->function = $function; $this->timeTakenInSeconds = $timeTakenInSeconds; $this->timeEntered = $timeEntered; $this->timeExited = $timeExited; + $this->arguments = $arguments; } /** - * @param string[]|float[]|array $extensionCall + * @param string[]|float[]|array $extensionCall * * @return RecordedCall + * + * @psalm-param array{function:string, entered:float, exited: float, time_taken: float, argv: mixed[]} $extensionCall */ public static function fromExtensionLoggedCallArray(array $extensionCall) : self { @@ -39,12 +51,14 @@ public static function fromExtensionLoggedCallArray(array $extensionCall) : self Assert::keyExists($extensionCall, 'entered'); Assert::keyExists($extensionCall, 'exited'); Assert::keyExists($extensionCall, 'time_taken'); + Assert::keyExists($extensionCall, 'argv'); return new self( (string) $extensionCall['function'], (float) $extensionCall['time_taken'], (float) $extensionCall['entered'], - (float) $extensionCall['exited'] + (float) $extensionCall['exited'], + $extensionCall['argv'] ); } @@ -67,4 +81,21 @@ public function timeExited() : float { return $this->timeExited; } + + /** + * We should never return the full set of arguments, only specific arguments for specific functions. This is to + * avoid potentially spilling personally identifiable information. + * + * @return mixed[] + */ + public function filteredArguments() : array + { + if ($this->function === 'file_get_contents') { + return [ + 'url' => (string) $this->arguments[0], + ]; + } + + return []; + } } diff --git a/tests/Integration/AgentTest.php b/tests/Integration/AgentTest.php index 5e140716..7358b427 100644 --- a/tests/Integration/AgentTest.php +++ b/tests/Integration/AgentTest.php @@ -10,6 +10,7 @@ use Scoutapm\Agent; use Scoutapm\Config; use Scoutapm\Connector\SocketConnector; +use Scoutapm\Extension\PotentiallyAvailableExtensionCapabilities; use function file_get_contents; use function getenv; use function gethostname; @@ -69,6 +70,8 @@ public function testLoggingIsSent() : void $agent->connect(); + (new PotentiallyAvailableExtensionCapabilities())->clearRecordedCalls(); + $agent->webTransaction('Yay', static function () use ($agent) : void { file_get_contents(__FILE__); $agent->instrument('Test', 'foo', static function () use ($agent) : void { @@ -129,6 +132,7 @@ public function testLoggingIsSent() : void if (TestHelper::scoutApmExtensionAvailable()) { $fileGetContentsSpanId = $this->assertUnserializedCommandContainsPayload('StartSpan', ['operation' => 'file_get_contents', 'parent_id' => $controllerSpanId], next($commands), 'span_id'); + $this->assertUnserializedCommandContainsPayload('TagSpan', ['span_id' => $fileGetContentsSpanId, 'tag' => 'desc', 'value' => ['url' => __FILE__]], next($commands), null); $this->assertUnserializedCommandContainsPayload('StopSpan', ['span_id' => $fileGetContentsSpanId], next($commands), null); } @@ -136,6 +140,7 @@ public function testLoggingIsSent() : void if (TestHelper::scoutApmExtensionAvailable()) { $fileGetContentsSpanId = $this->assertUnserializedCommandContainsPayload('StartSpan', ['operation' => 'file_get_contents', 'parent_id' => $fooSpanId], next($commands), 'span_id'); + $this->assertUnserializedCommandContainsPayload('TagSpan', ['span_id' => $fileGetContentsSpanId, 'tag' => 'desc', 'value' => ['url' => __FILE__]], next($commands), null); $this->assertUnserializedCommandContainsPayload('StopSpan', ['span_id' => $fileGetContentsSpanId], next($commands), null); } @@ -143,6 +148,7 @@ public function testLoggingIsSent() : void if (TestHelper::scoutApmExtensionAvailable()) { $fileGetContentsSpanId = $this->assertUnserializedCommandContainsPayload('StartSpan', ['operation' => 'file_get_contents', 'parent_id' => $barSpanId], next($commands), 'span_id'); + $this->assertUnserializedCommandContainsPayload('TagSpan', ['span_id' => $fileGetContentsSpanId, 'tag' => 'desc', 'value' => ['url' => __FILE__]], next($commands), null); $this->assertUnserializedCommandContainsPayload('StopSpan', ['span_id' => $fileGetContentsSpanId], next($commands), null); } @@ -154,6 +160,7 @@ public function testLoggingIsSent() : void if (TestHelper::scoutApmExtensionAvailable()) { $fileGetContentsSpanId = $this->assertUnserializedCommandContainsPayload('StartSpan', ['operation' => 'file_get_contents', 'parent_id' => $controllerSpanId], next($commands), 'span_id'); + $this->assertUnserializedCommandContainsPayload('TagSpan', ['span_id' => $fileGetContentsSpanId, 'tag' => 'desc', 'value' => ['url' => __FILE__]], next($commands), null); $this->assertUnserializedCommandContainsPayload('StopSpan', ['span_id' => $fileGetContentsSpanId], next($commands), null); } diff --git a/tests/Unit/Extension/RecordedCallTest.php b/tests/Unit/Extension/RecordedCallTest.php index eabb69af..a928d269 100644 --- a/tests/Unit/Extension/RecordedCallTest.php +++ b/tests/Unit/Extension/RecordedCallTest.php @@ -27,11 +27,56 @@ public function testFromExtensionLoggedCallArray() : void 'entered' => $entered, 'exited' => $exited, 'time_taken' => $timeTaken, + 'argv' => [], ]); self::assertSame($entered, $call->timeEntered()); self::assertSame($exited, $call->timeExited()); self::assertSame($timeTaken, $call->timeTakenInSeconds()); self::assertSame($function, $call->functionName()); + self::assertSame([], $call->filteredArguments()); + } + + /** + * @return string[][]|string[][][]|array)>> + */ + public function filteredArgumentsDataProvider() : array + { + return [ + 'file_get_contents' => [ + 'recordedFunctionName' => 'file_get_contents', + 'expectedFilteredArguments' => ['url' => 'a'], + ], + 'password_hash' => [ + 'recordedFunctionName' => 'password_hashj', + 'expectedFilteredArguments' => [], + ], + ]; + } + + /** + * @param mixed[] $expectedFilteredArguments + * + * @throws Exception + * + * @dataProvider filteredArgumentsDataProvider + */ + public function testOnlyFilteredArgumentsAreReturned( + string $recordedFunctionName, + array $expectedFilteredArguments + ) : void { + $entered = microtime(true) + random_int(1, 5); + $exited = microtime(true) + random_int(6, 10); + + self::assertEquals( + $expectedFilteredArguments, + RecordedCall::fromExtensionLoggedCallArray([ + 'function' => $recordedFunctionName, + 'entered' => $entered, + 'exited' => $exited, + 'time_taken' => $exited - $entered, + 'argv' => ['a', 'b', 'c', 'd', 'e', 'f'], + ])->filteredArguments() + ); } }