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

Rewrite MySql.Data instrumentation to bytecode instrumenation #2542

Closed
Kielek opened this issue May 17, 2023 · 7 comments · Fixed by #2836
Closed

Rewrite MySql.Data instrumentation to bytecode instrumenation #2542

Kielek opened this issue May 17, 2023 · 7 comments · Fixed by #2836
Assignees
Milestone

Comments

@Kielek
Copy link
Contributor

Kielek commented May 17, 2023

MySql.Data v 8.0.33 introduced breaking changes.
It is not feasible to longer use sourcecode instrumentation without some other magic (like Harmony open-telemetry/opentelemetry-dotnet-contrib#1157).

Issue detected by #2478

The goal is to rewrite it to bytecode instrumentation.

@bgrainger
Copy link

IMO the effort would be better spent integrating with MySqlConnector (disclaimer: I am the lead author): #2636.

MySqlConnector natively supports the System.Diagnostics APIs (currently Activity for tracing; metrics coming soon). It's also more highly tuned for performance (which I would assume is important for anyone who's instrumenting their application to track performance).

@Kielek
Copy link
Contributor Author

Kielek commented Jun 12, 2023

Adding support for MySqlConnector is IMO good idea, but we still should have support for MySql.Data.

Based on our research it was one of the most important instrumentation for already existing Tracers.

@zacharycmontoya
Copy link
Contributor

I started a draft PR for this work item, but I may need someone to finish it because of the following personal situations:

  • I'll likely be out-of-office for the rest of the week to take care of a personal emergency
  • I have scheduled vacation for the first half of July

@Kielek
Copy link
Contributor Author

Kielek commented Jun 21, 2023

SIG meetings: lets postpone to rc.2 version. For rc.1 ensure that we have known issue statement.

@muhaook
Copy link

muhaook commented Aug 14, 2023

For info, MySQL connector/NET supports OTel tracing starting 8.1.0: https://dev.mysql.com/doc/connector-net/en/connector-net-programming-telemetry.html It is for .NET 5 and later.

Also, starting 8.1.0, MySQL Enterprise Edition generates OTel trace data on Linux platforms: https://dev.mysql.com/doc/refman/8.1/en/telemetry-trace.html

I did not get a chance to test it, but we may expect the correlation between traces generated by connector/NET and traces generated by MySQL Enterprise Edition

@Kielek
Copy link
Contributor Author

Kielek commented Aug 14, 2023

@muhaook
Copy link

muhaook commented Aug 14, 2023

Looks interesting, especially builder extension: https://github.com/mysql/mysql-connector-net/blob/1341887fc28fc3181e9bf65a8dc6534e82b84284/MySQL.Data.OpenTelemetry/src/TraceProviderBuilderExtension.cs#L7-L12

It does not even require code change and can be enabled with config change OTEL_DOTNET_AUTO_TRACES_ADDITIONAL_SOURCES=connector-net

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment