Skip to content
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

Remove dependencies on OpenTelemetry.Instrumentation.EntityFrameworkCore #2814

Closed
eerhardt opened this issue Mar 12, 2024 · 4 comments · Fixed by #3027
Closed

Remove dependencies on OpenTelemetry.Instrumentation.EntityFrameworkCore #2814

eerhardt opened this issue Mar 12, 2024 · 4 comments · Fixed by #3027
Assignees
Labels
area-components Issues pertaining to Aspire Component packages

Comments

@eerhardt
Copy link
Member

We use the OpenTelemetry.Instrumentation.EntityFrameworkCore nuget package in the following components:

  • Aspire.Microsoft.EntityFrameworkCore.Cosmos
  • Aspire.Microsoft.EntityFrameworkCore.SqlServer
  • Aspire.Oracle.EntityFrameworkCore

As our plan is to stabilize .NET Aspire for v8.0.0 we need to remove unstable dependencies. The OpenTelemetry.Instrumentation.EntityFrameworkCore package doesn't have a stable version, and there is no plan to make it stable by the time we ship v8.0.0. We need to remove this dependency from these 3 components.

@roji said in an offline conversation:

This instrumentation library (main code) basically emits database client tracing (not metrics AFAIK) for command execution. This is something that we usually think belongs more at the lower ADO.NET layer (i.e. SqlClient, Npgsql..) rather than at the EF level - there's info at the lower levels that EF doesn't have, and EF doesn't necessarily have added value in emitting this tracing data.

In other words, an option here is to remove this library, and make it each provider's Aspire ADO.NET component's concern to enable tracing

Npgsql is already instrumented, so that's already there; for SqlClient we could use OpenTelemetry.Instrumentation.SqlClient

We should take this plan to remove this unstable dependency in the above components.

For Aspire.Microsoft.EntityFrameworkCore.SqlServer, it appears pretty straight-forward. We remove this dependency and instead replace it with OpenTelemetry.Instrumentation.SqlClient.

For the other 2 components, we need to work with the underlying ADO.NET layer to have the same instrumentation as Npgsql and SqlClient. (cc @Pilchie @kirankumarkolli for the Cosmos component. @andrevlins for the Oracle component)

cc @AndriySvyryd

@andrevlins
Copy link
Contributor

andrevlins commented Mar 15, 2024

Hi @eerhardt

In the case of Oracle, the ADO part of this change can only be implemented when they release the new version of the ADO library for Oracle (OPD.NET) that supports OTEL and metrics.

It is currently under development and its nuget package is a prerelease version.

If you remember, this was the reason we closed #1004 for now.

@sebastienros
Copy link
Member

@andrevlins do you have some recommendation to follow the progress on the driver? Should I just check on NuGet.org once in a while? I maybe subscribe to the samples repository assuming they would update them?

@Pilchie I have submitted the PR to remove the OpenTelemetry.Instrumentation.EntityFrameworkCore usage with Cosmos, so it only has Azure.Cosmos.Operation activities right now. Will there be some work done to include the ones added by the OTel package?

@Pilchie
Copy link
Member

Pilchie commented Mar 20, 2024

Tagging @kundadebdatta who is going to start looking at Cosmos DB/Aspire stuff soon.

@andrevlins
Copy link
Contributor

@andrevlins do you have some recommendation to follow the progress on the driver? Should I just check on NuGet.org once in a while? I maybe subscribe to the samples repository assuming they would update them?

@sebastienros thas what I'm doing:

  • Checking nuget at least once at least once a week;
  • Following oracle .net team on X (@OracleDotNet);
  • Checking github odp.net samples at least once a week.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-components Issues pertaining to Aspire Component packages
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants