Skip to content

Conversation

@martincostello
Copy link
Member

@martincostello martincostello commented Jun 9, 2025

Fixes #1764

Changes

Takes over from #2280 which went stale. Just need to add tests.

Merge requirement checklist

  • CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
  • Unit tests added/updated
  • Appropriate CHANGELOG.md files updated for non-trivial changes
  • Changes in public API reviewed (if applicable)

@github-actions github-actions bot added the comp:instrumentation.entityframeworkcore Things related to OpenTelemetry.Instrumentation.EntityFrameworkCore label Jun 9, 2025
@codecov
Copy link

codecov bot commented Jun 9, 2025

Codecov Report

Attention: Patch coverage is 94.44444% with 1 line in your changes missing coverage. Please review.

Project coverage is 80.45%. Comparing base (71655ce) to head (9aeb4db).
Report is 894 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...mplementation/EntityFrameworkDiagnosticListener.cs 94.44% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2829      +/-   ##
==========================================
+ Coverage   73.91%   80.45%   +6.54%     
==========================================
  Files         267       14     -253     
  Lines        9615      655    -8960     
==========================================
- Hits         7107      527    -6580     
+ Misses       2508      128    -2380     
Flag Coverage Δ
unittests-Instrumentation.EntityFrameworkCore 62.05% <94.44%> (?)
unittests-Instrumentation.SqlClient 88.26% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...mplementation/EntityFrameworkDiagnosticListener.cs 60.29% <94.44%> (+7.91%) ⬆️

... and 269 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

martincostello added a commit to grafana/grafana-opentelemetry-dotnet that referenced this pull request Jun 9, 2025
Consume changes from open-telemetry/opentelemetry-dotnet-contrib#2829 for test purposes.

Builds on top of #145.
@martincostello
Copy link
Member Author

martincostello commented Jun 10, 2025

From manual testing:

OpenTelemetry.Instrumentation.EntityFrameworkCore 1.12.0-beta.1

image

image

This PR

image

image

@github-actions github-actions bot added the comp:instrumentation.sqlclient Things related to OpenTelemetry.Instrumentation.SqlClient label Jun 10, 2025
@martincostello
Copy link
Member Author

martincostello commented Jun 10, 2025

Tests added based on the existing SqlClient integration tests. Reverting the original change in the first commit will cause the tests to fail.

@martincostello martincostello changed the title [Instrumentation.EFCore] Support together with Instrumentation.SqlClient [Instrumentation.EFCore] Support use with SqlClient Instrumentation Jun 10, 2025
@martincostello martincostello marked this pull request as ready for review June 10, 2025 11:54
Copilot AI review requested due to automatic review settings June 10, 2025 11:54
@martincostello martincostello requested a review from a team as a code owner June 10, 2025 11:54
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for using SqlClient instrumentation with EntityFrameworkCore by updating package references, adjusting activity source usage, and extending integration test coverage.

  • Updated package references in test projects for Microsoft.Data.SqlClient, EFCore providers, and test containers.
  • Added new integration tests and refactored activity source usage in the diagnostic listener.
  • Updated changelog to document support for SqlClient instrumentation.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/OpenTelemetry.Instrumentation.SqlClient.Tests/OpenTelemetry.Instrumentation.SqlClient.Tests.csproj Updated SqlClient package version
test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests.csproj Updated EFCore provider versions and added project references
test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/ItemsContext.cs Added new ItemsContext for EFCore tests using C# primary constructor syntax
test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/Item.cs Added simple POCO for EFCore tests
test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkIntegrationTests.cs Introduced integration tests for raw SQL and DataContext scenarios
test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkDiagnosticListenerTests.cs Refactored test item instantiation to use object initializer syntax
src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs Updated activity source usage to reflect SqlClient vs. EFCore differences
src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md Documented new SqlClient instrumentation support
Comments suppressed due to low confidence (1)

test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkIntegrationTests.cs:290

  • The error message uses an extra '$' inside the string interpolation which results in an unintended literal. Consider removing the extraneous '$' to correctly interpolate the container type name.
throw new InvalidOperationException($"Container type '${this.fixture.DatabaseContainer.GetType().Name}' is not supported.");

@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 7 days.

zivaninstana and others added 8 commits June 24, 2025 08:42
Fix spelling in grammar in comment.
Add integration tests for EFCore using SQL Server and SQLite.
- Fix missing SqlClient instrumentation.
- Fix assertions.
- Fix project reference.
Rewrite the message.
Fix incorrect assertion of a value with itself.
Avoid `ArgumentException` if the `DbContext` is not available in the event payload.

See dotnet/efcore#36286.
Add more `DbCommand` class names for if `DbContext` is not available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp:instrumentation.entityframeworkcore Things related to OpenTelemetry.Instrumentation.EntityFrameworkCore comp:instrumentation.sqlclient Things related to OpenTelemetry.Instrumentation.SqlClient

Projects

None yet

Development

Successfully merging this pull request may close these issues.

EF Core and SqlClient instrumentation enabled at the same time produce wrong spans

3 participants