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 unstable OpenTelemetry packages in Aspire Components #2819

Closed
eerhardt opened this issue Mar 12, 2024 · 4 comments
Closed
Assignees
Labels
area-integrations Issues pertaining to Aspire Integrations packages

Comments

@eerhardt
Copy link
Member

eerhardt commented Mar 12, 2024

As our plan is to stabilize .NET Aspire for v8.0.0 we need to remove unstable dependencies.

We have the following dependencies on unstable OpenTelemetry instrumentation packages in our Aspire Components:

  • OpenTelemetry.Instrumentation.EventCounters
  • OpenTelemetry.Instrumentation.SqlClient
  • OpenTelemetry.Instrumentation.StackExchangeRedis

None of these packages has a plan to ship a stable/GA version by the time we ship v8.0.0 of .NET Aspire. We need to formulate a plan for how to handle these dependencies.

OpenTelemetry.Instrumentation.EventCounters

Used by all 5 EntityFramework components and Aspire.Microsoft.Data.SqlClient to turn the existing EventCounters into OTel metrics / Meters.

I think the following 2 options are possible here:

  1. Remove these Metrics for now, and ask the lower level libraries (like efcore, sqlclient, etc) to produce metrics natively using the Meter APIs.
  2. "Vendor"/copy the OpenTelemetry.Instrumentation.EventCounters into our Aspire Components for now until either:
    a. The EventCounters library ships stable.
    b. The underlying libraries get native metrics using the Meter APIs.

OpenTelemetry.Instrumentation.SqlClient

This is used by Aspire.Microsoft.Data.SqlClient (and after #2814 Aspire.Microsoft.EntityFrameworkCore.SqlServer) to produce traces / Activities for the SqlClient ADO.NET provider.

Tracing is more critical than Metrics, so I don't think it is a valid option to remove this instrumentation from the component. I think the only viable option here is to "vendor"/copy the instrumentation library code into the Aspire components.

OpenTelemetry.Instrumentation.StackExchangeRedis

This is used by the 3 Redis components to produce traces / Activities when communicating with a Redis server.

For the same reason above as SqlClient, I don't think it is a valid option to remove this instrumentation from the component. I think the only viable option here is to "vendor"/copy the instrumentation library code into the Aspire components.

cc @joperezr @DamianEdwards @AndriySvyryd @roji @mgravell

@eerhardt eerhardt added this to the preview 5 (Apr) milestone Mar 12, 2024
@eerhardt eerhardt added area-integrations Issues pertaining to Aspire Integrations packages and removed area-templates labels Mar 12, 2024
@roji
Copy link
Member

roji commented Mar 12, 2024

For reference, the issue tracking emitting metrics natively from SqlClient is dotnet/SqlClient#2211 (the one for tracing is dotnet/SqlClient#2210).

@samsp-msft
Copy link
Member

Another possible option for EventCounters is to use the DiagnosticSource hooks that is being used for tracing to collect statistics and fabricate metrics based on those. The metrics can then be designed to match the OTel semantic conventions with dimensions etc.
What are the most important metrics for database scenarios from the client (as the server should probably be emitting its own).

@roji
Copy link
Member

roji commented Mar 13, 2024

What are the most important metrics for database scenarios from the client (as the server should probably be emitting its own).

Here's the OTel experimental semantic conventions for database client metrics. Notably missing from there is command metrics (executions, failures...), which I'd assume would need to be in any finalized spec... For reference, here's the Npgsql built-in implementation which emits both the (experimentally) standardized connection metrics and non-standardized command metrics.

@eerhardt
Copy link
Member Author

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

No branches or pull requests

4 participants