Skip to content

Commit 0936357

Browse files
cedriczielcedricziel
committed
fix: correctly render views and account for exceptions
1 parent d977cdc commit 0936357

File tree

5 files changed

+102
-30
lines changed

5 files changed

+102
-30
lines changed

‎src/Instrumentation/Laravel/src/Hooks/Illuminate/Contracts/Debug/ExceptionHandler.php‎

Lines changed: 8 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -46,24 +46,13 @@ protected function hookRender(): bool
4646
// Try to get the current span
4747
$scope = Context::storage()->scope();
4848
if (!$scope) {
49-
// If no span exists, create a new one for the exception handler
50-
$span = $this->instrumentation
51-
->tracer()
52-
->spanBuilder($spanName)
53-
->setSpanKind(SpanKind::KIND_INTERNAL)
54-
->setAttribute(TraceAttributes::CODE_FUNCTION_NAME, $function)
55-
->setAttribute(TraceAttributes::CODE_NAMESPACE, $class)
56-
->setAttribute(TraceAttributes::CODE_FILEPATH, $filename)
57-
->setAttribute(TraceAttributes::CODE_LINE_NUMBER, $lineno)
58-
->startSpan();
59-
60-
Context::storage()->attach($span->storeInContext(Context::getCurrent()));
61-
} else {
62-
// If a span exists, update its name
63-
$span = Span::fromContext($scope->context());
64-
$span->updateName($spanName);
49+
return;
6550
}
6651

52+
// Get the current span
53+
$span = Span::fromContext($scope->context());
54+
$span->updateName($spanName);
55+
6756
// Record exception information
6857
if ($exception instanceof Throwable) {
6958
$span->recordException($exception);
@@ -103,21 +92,11 @@ protected function hookReport(): bool
10392
// Get the current span (or create a new one)
10493
$scope = Context::storage()->scope();
10594
if (!$scope) {
106-
$span = $this->instrumentation
107-
->tracer()
108-
->spanBuilder($class . '@' . $function)
109-
->setSpanKind(SpanKind::KIND_INTERNAL)
110-
->setAttribute(TraceAttributes::CODE_FUNCTION_NAME, $function)
111-
->setAttribute(TraceAttributes::CODE_NAMESPACE, $class)
112-
->setAttribute(TraceAttributes::CODE_FILEPATH, $filename)
113-
->setAttribute(TraceAttributes::CODE_LINE_NUMBER, $lineno)
114-
->startSpan();
115-
116-
Context::storage()->attach($span->storeInContext(Context::getCurrent()));
117-
} else {
118-
$span = Span::fromContext($scope->context());
95+
return;
11996
}
12097

98+
$span = Span::fromContext($scope->context());
99+
121100
// Record the exception details
122101
$span->recordException($exception);
123102
$span->setStatus(StatusCode::STATUS_ERROR, $exception->getMessage());

‎src/Instrumentation/Laravel/src/Hooks/Illuminate/Foundation/Http/Middleware.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,11 @@ protected function hookMiddlewareClass(string $middlewareClass, ?string $group =
127127
$parent = Context::getCurrent();
128128
$parentSpan = Span::fromContext($parent);
129129

130+
// Don't create a new span if we're handling an exception
131+
if ($params[0] instanceof Throwable) {
132+
return;
133+
}
134+
130135
$span = $this->instrumentation
131136
->tracer()
132137
->spanBuilder($spanName)

‎src/Instrumentation/Laravel/src/Hooks/Illuminate/Routing/Route.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,16 @@ protected function hookRun(): bool
9595
}
9696
);
9797
}
98+
} elseif ($action instanceof \Closure) {
99+
// Add closure information to the existing span
100+
$scope = Context::storage()->scope();
101+
if (!$scope) {
102+
return $params;
103+
}
104+
105+
$span = Span::fromContext($scope->context());
106+
$span->setAttribute('code.function.name', '{closure}');
107+
$span->setAttribute('code.namespace', '');
98108
}
99109

100110
return $params;

‎src/Instrumentation/Laravel/tests/Integration/LaravelInstrumentationTest.php‎

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,8 @@ public function test_cache_log_db(): void
9999
$response = $this->call('GET', '/hello');
100100
$this->assertEquals(200, $response->status());
101101

102+
$this->printSpans();
103+
102104
$this->assertTraceStructure([
103105
[
104106
'name' => 'GET /hello',
@@ -145,6 +147,15 @@ public function test_cache_log_db(): void
145147
],
146148
'kind' => \OpenTelemetry\API\Trace\SpanKind::KIND_CLIENT,
147149
],
150+
[
151+
'name' => 'laravel.view.render',
152+
'attributes' => [
153+
'code.function.name' => 'render',
154+
'code.namespace' => 'Illuminate\View\View',
155+
'view.name' => 'welcome',
156+
],
157+
'kind' => \OpenTelemetry\API\Trace\SpanKind::KIND_INTERNAL,
158+
],
148159
],
149160
],
150161
],
@@ -263,7 +274,7 @@ public function test_route_span_name_if_not_found(): void
263274
$this->assertCount(0, $this->storage);
264275
$response = $this->call('GET', '/not-found');
265276
$this->assertEquals(404, $response->status());
266-
$this->assertCount(3, $this->storage);
277+
$this->assertCount(5, $this->storage);
267278

268279
$this->assertTraceStructure([
269280
[
@@ -297,10 +308,38 @@ public function test_route_span_name_if_not_found(): void
297308
'attributes' => [
298309
'code.function.name' => 'handle',
299310
'code.namespace' => 'Illuminate\Foundation\Http\Middleware\ConvertEmptyStringsToNull',
311+
'code.filepath' => '/usr/src/myapp/src/Instrumentation/Laravel/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/ConvertEmptyStringsToNull.php',
312+
'code.line.number' => 23,
300313
'laravel.middleware.class' => 'Illuminate\Foundation\Http\Middleware\ConvertEmptyStringsToNull',
301314
'http.response.status_code' => 404,
302315
],
303316
'kind' => \OpenTelemetry\API\Trace\SpanKind::KIND_INTERNAL,
317+
'children' => [
318+
[
319+
'name' => 'laravel.view.render',
320+
'attributes' => [
321+
'code.function.name' => 'render',
322+
'code.namespace' => 'Illuminate\View\View',
323+
'code.filepath' => '/usr/src/myapp/src/Instrumentation/Laravel/vendor/laravel/framework/src/Illuminate/View/View.php',
324+
'code.line.number' => 156,
325+
'view.name' => 'errors::404',
326+
],
327+
'kind' => \OpenTelemetry\API\Trace\SpanKind::KIND_INTERNAL,
328+
'children' => [
329+
[
330+
'name' => 'laravel.view.render',
331+
'attributes' => [
332+
'code.function.name' => 'render',
333+
'code.namespace' => 'Illuminate\View\View',
334+
'code.filepath' => '/usr/src/myapp/src/Instrumentation/Laravel/vendor/laravel/framework/src/Illuminate/View/View.php',
335+
'code.line.number' => 156,
336+
'view.name' => 'errors::minimal',
337+
],
338+
'kind' => \OpenTelemetry\API\Trace\SpanKind::KIND_INTERNAL,
339+
],
340+
],
341+
],
342+
],
304343
],
305344
],
306345
],

‎src/Instrumentation/Laravel/tests/Integration/Middleware/MiddlewareTest.php‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,11 @@ public function test_it_records_exceptions_in_middleware(): void
180180
// Laravel should catch the exception and return a 500 response
181181
$this->assertEquals(500, $response->status());
182182

183+
// Verify that we have 6 spans
184+
$this->assertCount(6, $this->storage);
185+
186+
$this->printSpans();
187+
183188
$this->assertTraceStructure([
184189
[
185190
'name' => 'GET /middleware-exception',
@@ -203,6 +208,10 @@ public function test_it_records_exceptions_in_middleware(): void
203208
[
204209
'name' => 'Illuminate\Foundation\Http\Middleware\ValidatePostSize::handle',
205210
'attributes' => [
211+
'code.function.name' => 'handle',
212+
'code.namespace' => 'Illuminate\Foundation\Http\Middleware\ValidatePostSize',
213+
'code.filepath' => '/usr/src/myapp/src/Instrumentation/Laravel/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/ValidatePostSize.php',
214+
'code.line.number' => 19,
206215
'laravel.middleware.class' => 'Illuminate\Foundation\Http\Middleware\ValidatePostSize',
207216
'http.response.status_code' => 500,
208217
],
@@ -213,12 +222,42 @@ public function test_it_records_exceptions_in_middleware(): void
213222
'attributes' => [
214223
'code.function.name' => 'handle',
215224
'code.namespace' => 'Illuminate\Foundation\Http\Middleware\ConvertEmptyStringsToNull',
225+
'code.filepath' => '/usr/src/myapp/src/Instrumentation/Laravel/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/ConvertEmptyStringsToNull.php',
226+
'code.line.number' => 23,
216227
'laravel.middleware.class' => 'Illuminate\Foundation\Http\Middleware\ConvertEmptyStringsToNull',
217228
'exception.class' => 'Exception',
218229
'exception.message' => 'Middleware Exception',
230+
'exception.file' => '/usr/src/myapp/src/Instrumentation/Laravel/tests/Integration/Middleware/MiddlewareTest.php',
231+
'exception.line' => 168,
219232
'http.response.status_code' => 500,
220233
],
221234
'kind' => \OpenTelemetry\API\Trace\SpanKind::KIND_INTERNAL,
235+
'children' => [
236+
[
237+
'name' => 'laravel.view.render',
238+
'attributes' => [
239+
'code.function.name' => 'render',
240+
'code.namespace' => 'Illuminate\View\View',
241+
'code.filepath' => '/usr/src/myapp/src/Instrumentation/Laravel/vendor/laravel/framework/src/Illuminate/View/View.php',
242+
'code.line.number' => 156,
243+
'view.name' => 'errors::500',
244+
],
245+
'kind' => \OpenTelemetry\API\Trace\SpanKind::KIND_INTERNAL,
246+
'children' => [
247+
[
248+
'name' => 'laravel.view.render',
249+
'attributes' => [
250+
'code.function.name' => 'render',
251+
'code.namespace' => 'Illuminate\View\View',
252+
'code.filepath' => '/usr/src/myapp/src/Instrumentation/Laravel/vendor/laravel/framework/src/Illuminate/View/View.php',
253+
'code.line.number' => 156,
254+
'view.name' => 'errors::minimal',
255+
],
256+
'kind' => \OpenTelemetry\API\Trace\SpanKind::KIND_INTERNAL,
257+
],
258+
],
259+
],
260+
],
222261
],
223262
],
224263
],

0 commit comments

Comments
 (0)