Skip to content

Commit

Permalink
Rename the span name of Laravel HTTP requests to include the low card…
Browse files Browse the repository at this point in the history
…inality 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
  • Loading branch information
LauJosefsen authored Dec 4, 2023
1 parent 720adad commit 72b53c9
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 3 deletions.
11 changes: 11 additions & 0 deletions src/Instrumentation/Laravel/src/HttpInstrumentation.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand All @@ -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());
Expand All @@ -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 */
Expand Down

0 comments on commit 72b53c9

Please sign in to comment.