-
Couldn't load subscription status.
- Fork 117
fix: openentelemetry-auto-laravel threw warning on \Illuminate\Database\Eloquent\Model::destroy called. #390
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
17cdecd
33ec0eb
77af273
5755263
48f140e
a233f08
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -102,16 +102,16 @@ public function test_eloquent_operations(): void | |
| try { | ||
| // Test create | ||
| $created = TestModel::create(['name' => 'test']); | ||
|
|
||
| // Test find | ||
| $found = TestModel::find($created->id); | ||
|
|
||
| // Test update | ||
| $found->update(['name' => 'updated']); | ||
|
|
||
| // Test delete | ||
| $found->delete(); | ||
|
|
||
| return response()->json(['status' => 'ok']); | ||
| } catch (\Exception $e) { | ||
| return response()->json([ | ||
|
|
@@ -127,18 +127,7 @@ public function test_eloquent_operations(): void | |
| } | ||
| $this->assertEquals(200, $response->status()); | ||
|
|
||
| // Verify spans for each Eloquent operation | ||
| /** @var array<int, \OpenTelemetry\SDK\Trace\ImmutableSpan> $spans */ | ||
| $spans = array_values(array_filter( | ||
| iterator_to_array($this->storage), | ||
| fn ($item) => $item instanceof \OpenTelemetry\SDK\Trace\ImmutableSpan | ||
| )); | ||
|
|
||
| // Filter out SQL spans and keep only Eloquent spans | ||
| $eloquentSpans = array_values(array_filter( | ||
| $spans, | ||
| fn ($span) => str_contains($span->getName(), '::') | ||
| )); | ||
| $eloquentSpans = $this->resolveEloquentOperationSpans(); | ||
|
|
||
| // Sort spans by operation type to ensure consistent order | ||
| usort($eloquentSpans, function ($a, $b) { | ||
|
|
@@ -186,6 +175,58 @@ public function test_eloquent_operations(): void | |
| $this->assertSame('delete', $deleteSpan->getAttributes()->get('laravel.eloquent.operation')); | ||
| } | ||
|
|
||
| public function test_eloquent_static_operations(): void | ||
| { | ||
| // Assert storage is empty before interacting with the database | ||
| $this->assertCount(0, $this->storage); | ||
|
|
||
| // Create the test_models table | ||
| DB::statement('CREATE TABLE IF NOT EXISTS test_models ( | ||
| id INTEGER PRIMARY KEY AUTOINCREMENT, | ||
| name TEXT, | ||
| created_at DATETIME, | ||
| updated_at DATETIME | ||
| )'); | ||
|
|
||
| $this->router()->get('/eloquent', function () { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there any reason to use the HTTP kernel to test model functionality? 🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for review! The main reason is to test in the same way as much as possible, because test cases with similar intentions were defined in other cases and added as additional cases, but as you reviewed, the TraceProvider setting is done on the TestCase class, so it seems that the test itself is possible without going through the HTTP kernel. If a better testing method is exists, I will be create a new PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @rozeo - we can split these out later then 👍 |
||
| try { | ||
| // create record for Test destroy | ||
| $created = TestModel::create(['name' => 'test']); | ||
|
|
||
| // Test destroy | ||
| TestModel::destroy([$created->id]); | ||
|
|
||
| return response()->json(['status' => 'ok']); | ||
| } catch (\Exception $e) { | ||
| return response()->json([ | ||
| 'error' => $e->getMessage(), | ||
| 'trace' => $e->getTraceAsString(), | ||
| ], 500); | ||
| } | ||
| }); | ||
|
|
||
| $response = $this->call('GET', '/eloquent'); | ||
| if ($response->status() !== 200) { | ||
| $this->fail('Request failed: ' . $response->content()); | ||
| } | ||
| $this->assertEquals(200, $response->status()); | ||
|
|
||
| $eloquentSpans = $this->resolveEloquentOperationSpans(); | ||
|
|
||
| // Destroy span | ||
| $destroySpans = array_values(array_filter($eloquentSpans, function ($span) { | ||
| return $span->getAttributes()->get('laravel.eloquent.operation') === 'destroy'; | ||
| })); | ||
|
|
||
| $this->assertCount(1, $destroySpans); | ||
|
|
||
| $destroySpan = $destroySpans[0]; | ||
| $this->assertSame('OpenTelemetry\Tests\Contrib\Instrumentation\Laravel\Fixtures\Models\TestModel::destroy', $destroySpan->getName()); | ||
| $this->assertSame('OpenTelemetry\Tests\Contrib\Instrumentation\Laravel\Fixtures\Models\TestModel', $destroySpan->getAttributes()->get('laravel.eloquent.model')); | ||
| $this->assertSame('test_models', $destroySpan->getAttributes()->get('laravel.eloquent.table')); | ||
| $this->assertSame('destroy', $destroySpan->getAttributes()->get('laravel.eloquent.operation')); | ||
| } | ||
|
|
||
| public function test_low_cardinality_route_span_name(): void | ||
| { | ||
| $this->router()->get('/hello/{name}', fn () => null)->name('hello-name'); | ||
|
|
@@ -213,4 +254,25 @@ private function router(): Router | |
| /** @psalm-suppress PossiblyNullReference */ | ||
| return $this->app->make(Router::class); | ||
| } | ||
|
|
||
| /** | ||
| * @return array<int, \OpenTelemetry\SDK\Trace\ImmutableSpan> | ||
| */ | ||
| private function resolveEloquentOperationSpans(): array | ||
| { | ||
| // Verify spans for each Eloquent operation | ||
| /** @var array<int, \OpenTelemetry\SDK\Trace\ImmutableSpan> $spans */ | ||
| $spans = array_values(array_filter( | ||
| iterator_to_array($this->storage), | ||
| fn ($item) => $item instanceof \OpenTelemetry\SDK\Trace\ImmutableSpan | ||
| )); | ||
|
|
||
| // Filter out SQL spans and keep only Eloquent spans | ||
| $eloquentSpans = array_values(array_filter( | ||
| $spans, | ||
| fn ($span) => str_contains($span->getName(), '::') | ||
| )); | ||
|
|
||
| return $eloquentSpans; | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.