diff --git a/CHANGELOG.md b/CHANGELOG.md index eecd703c..be4651f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ All notable changes to this project will be documented in this file, in reverse - [#236](https://github.com/scoutapp/scout-apm-php/pull/236) Replace `doctrine/cache` with `cache/array-adapter` in tests - Note - this change only affects our internal tests, the `Agent` still depends on *any* PSR-16 compatible cache (including `doctrine/cache`) +- [#237](https://github.com/scoutapp/scout-apm-php/pull/237) Made 'leaf span' recording more efficient so child spans are not recorded at all ### Deprecated diff --git a/src/Events/Request/Request.php b/src/Events/Request/Request.php index da11e5f1..22635bfc 100644 --- a/src/Events/Request/Request.php +++ b/src/Events/Request/Request.php @@ -49,6 +49,8 @@ class Request implements CommandWithChildren private $requestUriOverride; /** @var int */ private $spanCount = 0; + /** @var int */ + private $leafNodeDepth = 0; /** * @psalm-var ConfigKey::URI_REPORTING_* * @var string @@ -270,6 +272,12 @@ public function startSpan(string $operation, ?float $overrideTimestamp = null, b throw SpanLimitReached::forOperation($operation, self::MAX_COMPLETE_SPANS); } + if ($this->currentCommand instanceof Span && $this->currentCommand->isLeaf()) { + $this->leafNodeDepth++; + + return $this->currentCommand; + } + $this->spanCount++; $span = new Span($this->currentCommand, $operation, $this->id, $overrideTimestamp, $leafSpan); @@ -299,6 +307,12 @@ public function stopSpan(?float $overrideTimestamp = null): void return; } + if ($command->isLeaf() && $this->leafNodeDepth > 0) { + $this->leafNodeDepth--; + + return; + } + $command->stop($overrideTimestamp); $this->currentCommand = $command->parent(); diff --git a/src/Events/Span/Span.php b/src/Events/Span/Span.php index dd8aa32e..015110c5 100644 --- a/src/Events/Span/Span.php +++ b/src/Events/Span/Span.php @@ -85,6 +85,11 @@ public function parent(): CommandWithChildren return $this->parent; } + public function isLeaf(): bool + { + return $this->leafSpan; + } + /** * Do not call this directly - use Request#stopSpan() or Agent#stopSpan() to correctly handle bookkeeping * @@ -186,10 +191,6 @@ public function jsonSerialize(): array ]; foreach ($this->children as $child) { - if ($this->leafSpan && $child instanceof Span) { - continue; - } - foreach ($child->jsonSerialize() as $value) { $commands[] = $value; } diff --git a/tests/Unit/Events/Request/RequestTest.php b/tests/Unit/Events/Request/RequestTest.php index 5cad2c4c..e466c52e 100644 --- a/tests/Unit/Events/Request/RequestTest.php +++ b/tests/Unit/Events/Request/RequestTest.php @@ -11,9 +11,11 @@ use Scoutapm\Events\Request\Request; use Scoutapm\Events\Request\RequestId; use Scoutapm\Events\Span\Span; +use Scoutapm\UnitTests\TestHelper; use function array_key_exists; use function array_map; +use function assert; use function json_decode; use function json_encode; use function next; @@ -345,4 +347,31 @@ public function testRequestIsTaggedWithQueueTime(string $headerName, string $hea unset($_SERVER[$headerName]); } + + public function testSpansAreNotRecordedBelowLeafSpans(): void + { + $request = $this->requestFromConfiguration(); + + $request->startSpan('Foo', null, true); + $request->startSpan('ShouldNotBeRecorded1'); + $request->startSpan('ShouldNotBeRecorded2'); + $request->startSpan('ShouldNotBeRecorded3'); + $request->stopSpan(); + $request->stopSpan(); + $request->startSpan('ShouldNotBeRecorded4'); + $request->stopSpan(); + $request->stopSpan(); + $request->startSpan('ShouldNotBeRecorded5'); + $request->stopSpan(); + $request->stopSpan(); + + self::assertSame(1, $request->collectedSpans()); + + $firstSpan = TestHelper::firstChildForCommand($request); + assert($firstSpan instanceof Span); + self::assertSame('Foo', $firstSpan->getName()); + + $children = TestHelper::childrenForCommand($firstSpan); + self::assertCount(0, $children); + } } diff --git a/tests/Unit/Events/Span/SpanTest.php b/tests/Unit/Events/Span/SpanTest.php index ef74928e..ff786655 100644 --- a/tests/Unit/Events/Span/SpanTest.php +++ b/tests/Unit/Events/Span/SpanTest.php @@ -33,6 +33,12 @@ public function testCanBeInitialized(): void self::assertNull($span->getStopTime()); } + public function testThatASpanKnowsItIsALeafNode(): void + { + $span = new Span($this->mockParent, 'name', RequestId::new(), null, true); + self::assertTrue($span->isLeaf()); + } + /** @throws Exception */ public function testCanBeStopped(): void { diff --git a/tests/Unit/TestHelper.php b/tests/Unit/TestHelper.php new file mode 100644 index 00000000..a48af4ea --- /dev/null +++ b/tests/Unit/TestHelper.php @@ -0,0 +1,44 @@ +setAccessible(true); + $children = $childrenProperty->getValue($commandWithChildren); + + Assert::isArray($children); + Assert::allIsInstanceOf($children, Command::class); + + return $children; + } + + public static function firstChildForCommand(CommandWithChildren $commandWithChildren): Command + { + $children = self::childrenForCommand($commandWithChildren); + + Assert::greaterThanEq(count($children), 1); + + return reset($children); + } +}