Skip to content

Conversation

martincostello
Copy link
Member

@martincostello martincostello commented Jun 6, 2025

Changes

Add EFCore endpoint to assert on OpenTelemetry.Instrumentation.EntityFrameworkCore in OATS.

SQLite is used as the SQL Server variant goes through the existing SQL instrumentation so it misses the point of testing that EFCore specifically is instrumented.

Merge requirement checklist

  • Unit tests added/updated
  • CHANGELOG.md file updated for non-trivial changes
  • Changes in public API reviewed (if applicable)

@martincostello martincostello added the enhancement New feature or request label Jun 6, 2025
Add EFCore endpoint to assert on OpenTelemetry.Instrumentation.EntityFrameworkCore in OATS.
- Try to fix failing query.
- Improve some formatting.
Add leading and trailing slashes.
@martincostello martincostello marked this pull request as ready for review June 6, 2025 08:46
@Copilot Copilot AI review requested due to automatic review settings June 6, 2025 08:46
@martincostello martincostello requested a review from a team as a code owner June 6, 2025 08:46
@martincostello martincostello enabled auto-merge (squash) June 6, 2025 08:46
@martincostello martincostello disabled auto-merge June 6, 2025 08:46
@martincostello martincostello enabled auto-merge (squash) June 6, 2025 08:46
Copy link

@Copilot 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 an EFCore endpoint to the ASP.NET Core example to test OpenTelemetry instrumentation specific to EFCore using SQLite.

  • Added Microsoft.EntityFrameworkCore.Sqlite package and updated Swashbuckle in the project file.
  • Introduced new TodoApp endpoints, repository, and entity models to enable EFCore functionality.
  • Updated docker-compose configuration to include the new endpoint and trace queries.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

File Description
examples/net8.0/aspnetcore/aspnetcore.csproj Added EFCore Sqlite package and upgraded Swashbuckle version.
examples/net8.0/aspnetcore/TodoAppEndpoints.cs Introduced TodoApp endpoints, repository, and EFCore context for the Todo application.
examples/net8.0/aspnetcore/Program.cs Registered the TodoApp endpoints in the application startup.
docker/docker-compose-aspnetcore/oats.yaml Included the new /api/todo/items endpoint and trace validation for EFCore instrumentation.
Comments suppressed due to low confidence (1)

examples/net8.0/aspnetcore/TodoAppEndpoints.cs:103

  • The CompleteItemAsync method returns a nullable bool to indicate a not found item, while DeleteItemAsync returns a non-nullable bool. Consider unifying the return types to provide a consistent API design for repository methods.
public async Task<bool?> CompleteItemAsync(Guid itemId)

@martincostello
Copy link
Member Author

I should add some metric assertions too (e.g. db_client_operation_duration_seconds).

@martincostello martincostello marked this pull request as draft June 9, 2025 08:36
auto-merge was automatically disabled June 9, 2025 08:36

Pull request was converted to draft

@martincostello
Copy link
Member Author

db_client_operation_duration_seconds

db.client.operation.duration wasn't added until version 1.11.0-beta.1 of OpenTelemetry.Instrumentation.SqlClient.

We can add it after updating to the latest NuGet packages.

@martincostello martincostello marked this pull request as ready for review June 9, 2025 09:11
@martincostello martincostello enabled auto-merge (squash) June 9, 2025 09:11
@martincostello martincostello merged commit b4c7f88 into main Jun 9, 2025
11 checks passed
@martincostello martincostello deleted the add-efcore-example branch June 9, 2025 12:47
martincostello added a commit that referenced this pull request Jun 9, 2025
martincostello added a commit that referenced this pull request Jul 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants