From 72b53c924bada662caaf0ee34637d412178cf7f3 Mon Sep 17 00:00:00 2001
From: Lau Ernebjerg Josefsen <44977457+LauJosefsen@users.noreply.github.com>
Date: Mon, 4 Dec 2023 01:53:01 +0100
Subject: [PATCH] Rename the span name of Laravel HTTP requests to include the
 low cardinality route name if possible. (#219)

* Added low cardinality laravel route to the span name of the outer span
* Added not found test, and added logic to always ensure leading slash is present
* Add named route to route URI test
* Suppress psalm non empty string
---
 .../Laravel/src/HttpInstrumentation.php       | 11 ++++++++
 .../LaravelInstrumentationTest.php            | 28 +++++++++++++++++--
 2 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/src/Instrumentation/Laravel/src/HttpInstrumentation.php b/src/Instrumentation/Laravel/src/HttpInstrumentation.php
index 6e7a4f3a..487415f4 100644
--- a/src/Instrumentation/Laravel/src/HttpInstrumentation.php
+++ b/src/Instrumentation/Laravel/src/HttpInstrumentation.php
@@ -6,6 +6,7 @@
 
 use Illuminate\Contracts\Http\Kernel;
 use Illuminate\Http\Request;
+use Illuminate\Support\Facades\Route;
 use OpenTelemetry\API\Globals;
 use OpenTelemetry\API\Instrumentation\CachedInstrumentation;
 use OpenTelemetry\API\Trace\Span;
@@ -79,6 +80,16 @@ public static function register(CachedInstrumentation $instrumentation): void
                     $span->setAttribute(TraceAttributes::NETWORK_PROTOCOL_VERSION, $response->getProtocolVersion());
                     $span->setAttribute(TraceAttributes::HTTP_RESPONSE_BODY_SIZE, $response->headers->get('Content-Length'));
                 }
+                if(($route = Route::getCurrentRoute()?->uri()) !== null) {
+                    $request = ($params[0] instanceof Request) ? $params[0] : null;
+
+                    if (! str_starts_with($route, '/')) {
+                        $route = '/' . $route;
+                    }
+
+                    /** @psalm-suppress ArgumentTypeCoercion */
+                    $span->updateName(sprintf('%s %s', $request?->method() ?? 'unknown', $route));
+                }
 
                 $span->end();
             }
diff --git a/src/Instrumentation/Laravel/tests/Integration/LaravelInstrumentationTest.php b/src/Instrumentation/Laravel/tests/Integration/LaravelInstrumentationTest.php
index d13a8dda..8400f517 100644
--- a/src/Instrumentation/Laravel/tests/Integration/LaravelInstrumentationTest.php
+++ b/src/Instrumentation/Laravel/tests/Integration/LaravelInstrumentationTest.php
@@ -21,8 +21,8 @@ public function test_request_response(): void
         $this->assertEquals(200, $response->status());
         $this->assertCount(1, $this->storage);
         $span = $this->storage[0];
-        $this->assertSame('GET', $span->getName());
-    
+        $this->assertSame('GET /', $span->getName());
+
         $response = Http::get('opentelemetry.io');
         $this->assertEquals(200, $response->status());
         $span = $this->storage[1];
@@ -47,7 +47,7 @@ public function test_cache_log_db(): void
         $this->assertEquals(200, $response->status());
         $this->assertCount(2, $this->storage);
         $span = $this->storage[1];
-        $this->assertSame('GET', $span->getName());
+        $this->assertSame('GET /hello', $span->getName());
         $this->assertSame('http://localhost/hello', $span->getAttributes()->get(TraceAttributes::URL_FULL));
         $this->assertCount(5, $span->getEvents());
         $this->assertSame('cache set', $span->getEvents()[0]->getName());
@@ -64,6 +64,28 @@ public function test_cache_log_db(): void
         $this->assertSame('sqlite', $span->getAttributes()->get('db.system'));
     }
 
+    public function test_low_cardinality_route_span_name(): void
+    {
+        $this->router()->get('/hello/{name}', fn () => null)->name('hello-name');
+
+        $this->assertCount(0, $this->storage);
+        $response = $this->call('GET', '/hello/opentelemetry');
+        $this->assertEquals(200, $response->status());
+        $this->assertCount(1, $this->storage);
+        $span = $this->storage[0];
+        $this->assertSame('GET /hello/{name}', $span->getName());
+    }
+
+    public function test_route_span_name_if_not_found(): void
+    {
+        $this->assertCount(0, $this->storage);
+        $response = $this->call('GET', '/not-found');
+        $this->assertEquals(404, $response->status());
+        $this->assertCount(1, $this->storage);
+        $span = $this->storage[0];
+        $this->assertSame('GET', $span->getName());
+    }
+
     private function router(): Router
     {
         /** @psalm-suppress PossiblyNullReference */