-
Notifications
You must be signed in to change notification settings - Fork 281
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
Emit OpenTelemetry tracing #2210
Comments
Why is it better to emit third party specific information rather than have their adapter deal with the diagnostic information we already emit? |
@Wraith2 this isn't 3rd-party specific information, but rather the emerging standard for how database clients emit tracing information; fundational components in the .NET ecosystem (such as ASP.NET itself, but also Npgsql) are being instrumented to emit OpenTelemetry tracing directly, which is the best user experience and guarantees both accurate tracing and best performance. This really belongs in the driver rather than some 3rd-party adapter which may or may not have access to the needed information in an efficient way. |
If there is additional information needed in the diagnostic data that we emit then we should identify and add it so that it can be available to the open telemetry adapter. I've already got an issue open at open telemetry about changing the diagnostic information to be strongly typed so it's more aot friendly so adding any new information would be best done along with that work. I don't see why it's beneficial to have open telemetry as s direct dependency/implementation inside sqlclient though. It adds complexity and another set of diagnostic logging (we already have 2) and isn't needed by many people. If we want to make things pay-for-play then an external adapter that people can opt into sounds like the best approach to me. |
There's no OpenTelemetry dependency - everything needed to emit tracing (or metrics) data is already present in the framework (e.g. System.Diagnostics.ActivitySource). In addition, the APIs have been optimized so that their overheads when tracing isn't enabled is negligible. So there's no real pay-per-play aspect here, and no real downside to implementing this in-the-box for people. As I wrote above, this is becoming the de-facto standard for distributed tracing - not just in .NET. Note that this is also a different type of tracing: this isn't about adding tracing events to each and every function, generating very large dumps for method-by-method debugging; it's much more about emitting high-level information about what database commands are being executed, how long they're taking, and reporting various aspects about them. It's indeed probably possible to derive that information from SqlClient's existing tracing, but that kind of approach is always less efficient and requires that users discover and add yet another dependency. I suggest taking a look at how this look for Npgsql; here's the doc page (I know some demos are planned for this in .NET Conf). |
+1. Having libraries natively instrumented is the ideal case. As an example, HttpClient and ASP.NET Core are also moving towards that change. Tracing is considered for @roji How does Npgsql handle experimental semantic conventions part? |
@vishweshbankwar Npgsql implements the current database tracing semantic conventions in their experimental state, and that support is itself documented as experimental in our docs. When the specs mature and reach stable, we'll align to those (users may need to react). |
Upvoting :) ❤️ to see this! |
OpenTelemetry has become the de-facto standard for distributed, cross-platform tracing; it is being rapidly adopted both inside the .NET ecosystem and outside. OTel defines semantic conventions for how a database client traces information; these specifications are currently in experimental state, but stabilization is likely start happening soon.
There is a separate instrumentation library, OpenTelemetry.Instrumentation.SqlClient, which AFAIK uses the DiagnosticSource instrumentation already emitted by SqlClient to then emit OpenTelemetry tracing.
However, ideally SqlClient would emit this tracing data directly, without requiring a 3rd-party library.
The text was updated successfully, but these errors were encountered: