Skip to content

Commit a3fb3a4

Browse files
authored
fix: openentelemetry-auto-laravel threw warning on \Illuminate\Database\Eloquent\Model::destroy called. (#390)
* add reproducing test asserts * improve test definition * fix format * fix warning on Illuminate\Database\Eloquent\Model::destroy called * fix psalm error * set typehint
1 parent 7d534a0 commit a3fb3a4

File tree

2 files changed

+83
-17
lines changed

2 files changed

+83
-17
lines changed

src/Instrumentation/Laravel/src/Hooks/Illuminate/Database/Eloquent/Model.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,11 @@ private function hookDestroy(): void
189189
hook(
190190
EloquentModel::class,
191191
'destroy',
192-
pre: function ($model, array $params, string $class, string $function, ?string $filename, ?int $lineno) {
192+
pre: function (string $modelClassName, array $params, string $class, string $function, ?string $filename, ?int $lineno) {
193+
// The class-string is passed to the $model argument, because \Illuminate\Database\Eloquent\Model::destroy is static method.
194+
// Therefore, create a class instance from a class-string, and then get the table name from the getTable function.
195+
$model = new $modelClassName();
196+
193197
$builder = $this->instrumentation
194198
->tracer()
195199
->spanBuilder($model::class . '::destroy')

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

Lines changed: 78 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -102,16 +102,16 @@ public function test_eloquent_operations(): void
102102
try {
103103
// Test create
104104
$created = TestModel::create(['name' => 'test']);
105-
105+
106106
// Test find
107107
$found = TestModel::find($created->id);
108-
108+
109109
// Test update
110110
$found->update(['name' => 'updated']);
111-
111+
112112
// Test delete
113113
$found->delete();
114-
114+
115115
return response()->json(['status' => 'ok']);
116116
} catch (\Exception $e) {
117117
return response()->json([
@@ -127,18 +127,7 @@ public function test_eloquent_operations(): void
127127
}
128128
$this->assertEquals(200, $response->status());
129129

130-
// Verify spans for each Eloquent operation
131-
/** @var array<int, \OpenTelemetry\SDK\Trace\ImmutableSpan> $spans */
132-
$spans = array_values(array_filter(
133-
iterator_to_array($this->storage),
134-
fn ($item) => $item instanceof \OpenTelemetry\SDK\Trace\ImmutableSpan
135-
));
136-
137-
// Filter out SQL spans and keep only Eloquent spans
138-
$eloquentSpans = array_values(array_filter(
139-
$spans,
140-
fn ($span) => str_contains($span->getName(), '::')
141-
));
130+
$eloquentSpans = $this->resolveEloquentOperationSpans();
142131

143132
// Sort spans by operation type to ensure consistent order
144133
usort($eloquentSpans, function ($a, $b) {
@@ -186,6 +175,58 @@ public function test_eloquent_operations(): void
186175
$this->assertSame('delete', $deleteSpan->getAttributes()->get('laravel.eloquent.operation'));
187176
}
188177

178+
public function test_eloquent_static_operations(): void
179+
{
180+
// Assert storage is empty before interacting with the database
181+
$this->assertCount(0, $this->storage);
182+
183+
// Create the test_models table
184+
DB::statement('CREATE TABLE IF NOT EXISTS test_models (
185+
id INTEGER PRIMARY KEY AUTOINCREMENT,
186+
name TEXT,
187+
created_at DATETIME,
188+
updated_at DATETIME
189+
)');
190+
191+
$this->router()->get('/eloquent', function () {
192+
try {
193+
// create record for Test destroy
194+
$created = TestModel::create(['name' => 'test']);
195+
196+
// Test destroy
197+
TestModel::destroy([$created->id]);
198+
199+
return response()->json(['status' => 'ok']);
200+
} catch (\Exception $e) {
201+
return response()->json([
202+
'error' => $e->getMessage(),
203+
'trace' => $e->getTraceAsString(),
204+
], 500);
205+
}
206+
});
207+
208+
$response = $this->call('GET', '/eloquent');
209+
if ($response->status() !== 200) {
210+
$this->fail('Request failed: ' . $response->content());
211+
}
212+
$this->assertEquals(200, $response->status());
213+
214+
$eloquentSpans = $this->resolveEloquentOperationSpans();
215+
216+
// Destroy span
217+
$destroySpans = array_values(array_filter($eloquentSpans, function ($span) {
218+
return $span->getAttributes()->get('laravel.eloquent.operation') === 'destroy';
219+
}));
220+
221+
$this->assertCount(1, $destroySpans);
222+
223+
$destroySpan = $destroySpans[0];
224+
$this->assertSame('OpenTelemetry\Tests\Contrib\Instrumentation\Laravel\Fixtures\Models\TestModel::destroy', $destroySpan->getName());
225+
$this->assertSame('OpenTelemetry\Tests\Contrib\Instrumentation\Laravel\Fixtures\Models\TestModel', $destroySpan->getAttributes()->get('laravel.eloquent.model'));
226+
$this->assertSame('test_models', $destroySpan->getAttributes()->get('laravel.eloquent.table'));
227+
$this->assertSame('destroy', $destroySpan->getAttributes()->get('laravel.eloquent.operation'));
228+
}
229+
189230
public function test_low_cardinality_route_span_name(): void
190231
{
191232
$this->router()->get('/hello/{name}', fn () => null)->name('hello-name');
@@ -213,4 +254,25 @@ private function router(): Router
213254
/** @psalm-suppress PossiblyNullReference */
214255
return $this->app->make(Router::class);
215256
}
257+
258+
/**
259+
* @return array<int, \OpenTelemetry\SDK\Trace\ImmutableSpan>
260+
*/
261+
private function resolveEloquentOperationSpans(): array
262+
{
263+
// Verify spans for each Eloquent operation
264+
/** @var array<int, \OpenTelemetry\SDK\Trace\ImmutableSpan> $spans */
265+
$spans = array_values(array_filter(
266+
iterator_to_array($this->storage),
267+
fn ($item) => $item instanceof \OpenTelemetry\SDK\Trace\ImmutableSpan
268+
));
269+
270+
// Filter out SQL spans and keep only Eloquent spans
271+
$eloquentSpans = array_values(array_filter(
272+
$spans,
273+
fn ($span) => str_contains($span->getName(), '::')
274+
));
275+
276+
return $eloquentSpans;
277+
}
216278
}

0 commit comments

Comments
 (0)