-
Notifications
You must be signed in to change notification settings - Fork 636
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
Add PyMySQL instrumentor support for sqlcommenting #2942
Add PyMySQL instrumentor support for sqlcommenting #2942
Conversation
Going to close this and put in a fresh PR later. There's more that I want to add. |
Re-opening this PR for review, when others have a moment please 😺 I researched how this instrumentation's support of sqlcommenting would affect prepared statements that PyMySQL driver might generate. The driver implementation seems to not support prepared statements (absence in docs here), so it seems to not be an issue. |
Working on revisions similar to #2941 at the moment. Will update when done and ready. EDIT: This is done. |
Description
Updates PyMySQL instrumentor to support sqlcommenting.
Depends on # 2897
Fixes # 2902
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Added unit tests. Used local installations of opentelemetry-instrumentation-dbapi (in ##2897) and upstream dependencies with this updated downstream instrumentor on a Flask app that use PyMySQL to query MySQL with general logs enabled to check sqlcomments.
Example general log entry after this update, which now includes sqlcomment:
2024-10-30T19:33:46.830661Z 25 Query SELECT * FROM city WHERE id = '1818' /*db_driver='pymysql%3A1.4.6',dbapi_level='2.0',dbapi_threadsafety=1,driver_paramstyle='pyformat',mysql_client_version='1.4.6',traceparent='00-f8688332bc5467bfa8fe1a9753d7271a-f7add29c81e79082-01'*/
Example corresponding exported span, whose span ID matches sqlcomment traceparent span ID:
Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.