[BUGFIX] #36 Added fallback when backtrace does not contain caller na…

…me (#43) * [BUGFIX] #36 Changed `getCallerClass`, on Backtracer trait to return null if the backtrace is empty. Changed DatebaseQueryCollector and CacheCollector to add '(too deeply nested)' to their respective segment name, if the caller name is deeper than 50 calls in the backtrace. Added tests for all affected classes. * [BUGFIX] #36 Applied suggested diff. --------- Co-authored-by: Nicolas D'Amours <nicolas@folkshr.com>
Napp · Nov 1, 2023 · cc751a1 · cc751a1
1 parent 93bb071
commit cc751a1
Show file tree

Hide file tree

Showing 6 changed files with 279 additions and 46 deletions.
diff --git a/src/Collectors/Backtracer.php b/src/Collectors/Backtracer.php
@@ -20,8 +20,12 @@ public function getBacktrace(): array
         return array_slice(array_filter($sources), 0, 10);
     }
 
-    public function getCallerClass(array $backtrace): string
+    public function getCallerClass(array $backtrace): ?string
     {
+        if (sizeof($backtrace) === 0) {
+            return null;
+        }
+
         $arr = explode('\\', $backtrace[0]);
 
         return end($arr);

diff --git a/src/Collectors/CacheCollector.php b/src/Collectors/CacheCollector.php
@@ -1,43 +1,46 @@
-<?php
-
-declare(strict_types=1);
-
-namespace Napp\Xray\Collectors;
-
-use Illuminate\Cache\Events\CacheHit;
-use Illuminate\Cache\Events\CacheMissed;
-use Illuminate\Cache\Events\KeyForgotten;
-use Illuminate\Cache\Events\KeyWritten;
-use Illuminate\Redis\Events\CommandExecuted;
-
-class CacheCollector extends EventsCollector
-{
-    public function registerEventListeners(): void
-    {
-        $this->app->events->listen(CacheHit::class, function (CacheHit $cache) {
-            $this->handleQueryReport($cache->key, 'Cache hit');
-        });
-        $this->app->events->listen(CacheMissed::class, function (CacheMissed $cache) {
-            $this->handleQueryReport($cache->key, 'Cache miss');
-        });
-        $this->app->events->listen(KeyWritten::class, function (KeyWritten $cache) {
-            $this->handleQueryReport($cache->key, 'Cache set');
-        });
-        $this->app->events->listen(KeyForgotten::class, function (KeyForgotten $cache) {
-            $this->handleQueryReport($cache->key, 'Cache delete');
-        });
-        $this->app->events->listen(CommandExecuted::class, function (CommandExecuted $cache) {
-            $this->handleQueryReport($cache->command, 'Cache redis command executed');
-        });
-    }
-
-    protected function handleQueryReport(string $cacheKey, string $eventName): void
-    {
-        $backtrace = $this->getBacktrace();
-        $this
-            ->addSegment($eventName . ' at ' . $this->getCallerClass($backtrace))
-            ->addAnnotation('Key', $cacheKey)
-            ->addMetadata('backtrace', $backtrace)
-            ->end();
-    }
-}
+<?php
+
+declare(strict_types=1);
+
+namespace Napp\Xray\Collectors;
+
+use Illuminate\Cache\Events\CacheHit;
+use Illuminate\Cache\Events\CacheMissed;
+use Illuminate\Cache\Events\KeyForgotten;
+use Illuminate\Cache\Events\KeyWritten;
+use Illuminate\Redis\Events\CommandExecuted;
+
+class CacheCollector extends EventsCollector
+{
+    public function registerEventListeners(): void
+    {
+        $this->app->events->listen(CacheHit::class, function (CacheHit $cache) {
+            $this->handleQueryReport($cache->key, 'Cache hit');
+        });
+        $this->app->events->listen(CacheMissed::class, function (CacheMissed $cache) {
+            $this->handleQueryReport($cache->key, 'Cache miss');
+        });
+        $this->app->events->listen(KeyWritten::class, function (KeyWritten $cache) {
+            $this->handleQueryReport($cache->key, 'Cache set');
+        });
+        $this->app->events->listen(KeyForgotten::class, function (KeyForgotten $cache) {
+            $this->handleQueryReport($cache->key, 'Cache delete');
+        });
+        $this->app->events->listen(CommandExecuted::class, function (CommandExecuted $cache) {
+            $this->handleQueryReport($cache->command, 'Cache redis command executed');
+        });
+    }
+
+    protected function handleQueryReport(string $cacheKey, string $eventName): void
+    {
+        $backtrace = $this->getBacktrace();
+
+        $eventSuffix = sizeof($backtrace) > 0 ? ('at ' . $this->getCallerClass($backtrace)) : '(too deeply nested)';
+
+        $this
+            ->addSegment("$eventName $eventSuffix")
+            ->addAnnotation('Key', $cacheKey)
+            ->addMetadata('backtrace', $backtrace)
+            ->end();
+    }
+}
diff --git a/src/Collectors/DatabaseQueryCollector.php b/src/Collectors/DatabaseQueryCollector.php
@@ -20,7 +20,7 @@ public function registerEventListeners(): void
             $this->handleQueryReport($sql, $query->bindings, $query->time, $query->connection);
         });
 
-        $this->bindingsEnabled = config('xray.db_bindings');
+        $this->checkForEnabledBindings();
     }
 
     protected function handleQueryReport(string $sql, array $bindings, float $time, Connection $connection): void
@@ -30,9 +30,12 @@ protected function handleQueryReport(string $sql, array $bindings, float $time,
         }
 
         $backtrace = $this->getBacktrace();
+
+        $eventSuffix = sizeof($backtrace) > 0 ? ('at ' . $this->getCallerClass($backtrace)) : '(too deeply nested)';
+
         $this->current()->addSubsegment(
             (new SqlSegment())
-                ->setName($connection->getName() . ' at ' . $this->getCallerClass($backtrace))
+                ->setName($connection->getName() . ' ' . $eventSuffix)
                 ->setDatabaseType($connection->getDriverName())
                 ->setQuery($sql)
                 ->addMetadata('backtrace', $backtrace)
@@ -41,6 +44,11 @@ protected function handleQueryReport(string $sql, array $bindings, float $time,
         );
     }
 
+    protected function checkForEnabledBindings(): void
+    {
+        $this->bindingsEnabled = config('xray.db_bindings');
+    }
+
     private function parseBindings(string $sql, array $bindings, Connection $connection): string
     {
         $sql = str_replace(['%', '?'], ['%%', '%s'], $sql);

diff --git a/tests/Collectors/BacktracerTest.php b/tests/Collectors/BacktracerTest.php
@@ -0,0 +1,34 @@
+<?php
+
+namespace Napp\Xray\Tests\Collectors;
+
+use Napp\Xray\Collectors\Backtracer;
+use PHPUnit\Framework\TestCase;
+class BacktracerTest extends TestCase
+{
+    public function test_it_should_not_crash_when_backtrace_is_empty() {
+        // given a segment that implements the Backtracer trait
+        $implementationClass = new class {
+            use Backtracer;
+
+            // and an override of the `getBacktrace` function to return an empty array
+            public function getBacktrace(): array
+            {
+                return [];
+            }
+
+            // and a function that calls the `getCallerClass` function
+            public function getResult() {
+                $backtrace = $this->getBacktrace();
+
+                return $this->getCallerClass($backtrace);
+            }
+        };
+
+        // when calling the getResult class on the implementation
+        $result = $implementationClass->getResult();
+
+        // then it should return null
+        $this->assertNull($result);
+    }
+}
diff --git a/tests/Collectors/CacheCollectorTest.php b/tests/Collectors/CacheCollectorTest.php
@@ -0,0 +1,77 @@
+<?php
+
+namespace Napp\Xray\Tests\Collectors;
+
+use Illuminate\Cache\Events\CacheHit;
+use Monolog\Test\TestCase;
+use \Illuminate\Foundation\Application;
+use Napp\Xray\Collectors\CacheCollector;
+use Pkerrigan\Xray\Segment;
+
+class CacheCollectorTest extends TestCase
+{
+    public function test_it_should_handle_null_callerClass() {
+        // given a mock implementation of the Application Object
+        $mockApplication = new CacheCollectorTestMockApplication();
+
+        // given a CacheCollector object
+        $cacheCollectorMock = new class($mockApplication) extends CacheCollector {
+            // and an overridden getBacktrace function that returns an empty array
+            public function getBacktrace(): array
+            {
+                return [];
+            }
+
+            // and a custom public function that returns the segments
+            public function getSegments(): array {
+                return $this->segments;
+            }
+        };
+
+        // and an event for this cacheCollectorMock
+        $givenEvent = new CacheHit("irrelevent", "irrelevent");
+
+        // when dispatching the event on the mock application
+        $mockApplication->events->dispatch($givenEvent);
+
+        // then a segment should have been dispatched
+        $this->assertCount(1, $cacheCollectorMock->getSegments());
+
+        // and the segment name should contain "too deeply nested"
+        /** @var Segment $firstSegment */
+        $firstSegment = array_values($cacheCollectorMock->getSegments())[0];
+        $firstSegmentData = $firstSegment->jsonSerialize();
+
+        $this->assertStringContainsString("too deeply nested", $firstSegmentData['name']);
+        $this->assertStringNotContainsString(" at ", $firstSegmentData['name']);
+    }
+}
+
+class CacheCollectorTestMockApplication extends Application {
+
+    public $events;
+    public function __construct()
+    {
+        $this->events = new CacheCollectorTestMockEvents();
+    }
+}
+
+class CacheCollectorTestMockEvents {
+    protected $listeners = [];
+
+    public function listen(string $event, $callback) {
+        $this->listeners[$event] = $callback;
+    }
+
+    public function dispatch($eventObject) {
+        $eventClass = get_class($eventObject);
+
+        if(!array_key_exists($eventClass, $this->listeners)) {
+            throw new \Exception("Unit test exception, $eventClass was never registered");
+        }
+
+        $eventCallback = $this->listeners[$eventClass];
+
+        $eventCallback($eventObject);
+    }
+}
diff --git a/tests/Collectors/DatabaseQueryCollectorTest.php b/tests/Collectors/DatabaseQueryCollectorTest.php
@@ -0,0 +1,107 @@
+<?php
+
+namespace Napp\Xray\Tests\Collectors;
+
+use Illuminate\Database\Connection;
+use Illuminate\Database\Events\QueryExecuted;
+use Illuminate\Foundation\Application;
+use Napp\Xray\Collectors\DatabaseQueryCollector;
+use PHPUnit\Framework\TestCase;
+use Pkerrigan\Xray\Segment;
+
+class DatabaseQueryCollectorTest extends TestCase
+{
+    public function test_it_should_handle_null_callerClass()
+    {
+        // given a mock implementation of the Application Object
+        $mockApplication = new DatabaseQueryCollectorTestMockApplication();
+
+
+        // given a DatabaseQueryCollector object
+        $databaseQueryCollectorMock = new class($mockApplication) extends DatabaseQueryCollector {
+            protected $currentSegment;
+            // and an overridden getBacktrace function that returns an empty array
+            public function getBacktrace(): array
+            {
+                return [];
+            }
+
+            protected function checkForEnabledBindings(): void {}
+
+            // and a defined current segment
+            public function current(): Segment
+            {
+                if(is_null($this->currentSegment)) {
+                    $this->currentSegment = new Segment();
+                    $this->currentSegment->begin();
+                }
+
+                return $this->currentSegment;
+            }
+        };
+
+        // and an event for this DatabaseQueryCollectorMock
+        $connectionMock = $this->getMockBuilder(Connection::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $connectionMock->method('getName')
+            ->willReturn('irrelevent');
+
+        $connectionMock->method('getDriverName')
+            ->willReturn('irrelevent');
+
+        $givenEvent = new QueryExecuted("irrelevent", [], 1000, $connectionMock);
+
+        // when dispatching the event on the mock application
+        $mockApplication->events->dispatch($givenEvent);
+
+        // then a sub segment should have been dispatched
+        $currentSegment = $databaseQueryCollectorMock->current();
+        $currentSegmentData = $currentSegment->jsonSerialize();
+
+        $this->assertCount(1, $currentSegmentData['subsegments']);
+
+        // and the sub segment name should contain "too deeply nested"
+        /** @var Segment $firstSubSegment */
+        $firstSubSegment = $currentSegmentData['subsegments'][0];
+        $firstSubSegmentData = $firstSubSegment->jsonSerialize();
+
+        $this->assertStringContainsString("too deeply nested", $firstSubSegmentData['name']);
+        $this->assertStringNotContainsString(" at ", $firstSubSegmentData['name']);
+    }
+}
+
+class DatabaseQueryCollectorTestMockApplication extends Application
+{
+
+    public $events;
+
+    public function __construct()
+    {
+        $this->events = new DatabaseQueryCollectorTestMockEvents();
+    }
+}
+
+class DatabaseQueryCollectorTestMockEvents
+{
+    protected $listeners = [];
+
+    public function listen(string $event, $callback)
+    {
+        $this->listeners[$event] = $callback;
+    }
+
+    public function dispatch($eventObject)
+    {
+        $eventClass = get_class($eventObject);
+
+        if (!array_key_exists($eventClass, $this->listeners)) {
+            throw new \Exception("Unit test exception, $eventClass was never registered");
+        }
+
+        $eventCallback = $this->listeners[$eventClass];
+
+        $eventCallback($eventObject);
+    }
+}