From cc751a16d817ace6076e4424c7899fb5d213096e Mon Sep 17 00:00:00 2001
From: nicDamours <n.damours.00@gmail.com>
Date: Wed, 1 Nov 2023 15:25:52 -0400
Subject: [PATCH] [BUGFIX] #36 Added fallback when backtrace does not contain
 caller name (#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>
---
 src/Collectors/Backtracer.php                 |   6 +-
 src/Collectors/CacheCollector.php             |  89 ++++++++-------
 src/Collectors/DatabaseQueryCollector.php     |  12 +-
 tests/Collectors/BacktracerTest.php           |  34 ++++++
 tests/Collectors/CacheCollectorTest.php       |  77 +++++++++++++
 .../Collectors/DatabaseQueryCollectorTest.php | 107 ++++++++++++++++++
 6 files changed, 279 insertions(+), 46 deletions(-)
 create mode 100644 tests/Collectors/BacktracerTest.php
 create mode 100644 tests/Collectors/CacheCollectorTest.php
 create mode 100644 tests/Collectors/DatabaseQueryCollectorTest.php

diff --git a/src/Collectors/Backtracer.php b/src/Collectors/Backtracer.php
index cc8c5c6..4b6ff8b 100644
--- 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
index 0f3ad43..79cea3e 100644
--- 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
index daf09e1..dc5b222 100644
--- 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
new file mode 100644
index 0000000..f61c8d3
--- /dev/null
+++ 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);
+    }
+}
\ No newline at end of file
diff --git a/tests/Collectors/CacheCollectorTest.php b/tests/Collectors/CacheCollectorTest.php
new file mode 100644
index 0000000..431a8d0
--- /dev/null
+++ 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);
+    }
+}
\ No newline at end of file
diff --git a/tests/Collectors/DatabaseQueryCollectorTest.php b/tests/Collectors/DatabaseQueryCollectorTest.php
new file mode 100644
index 0000000..b209e64
--- /dev/null
+++ 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);
+    }
+}
\ No newline at end of file