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

Add mysql-connector instrumentor support for sqlcommenting #3023

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

tammy-baylis-swi
Copy link
Contributor

@tammy-baylis-swi tammy-baylis-swi commented Nov 19, 2024

Description

Updates mysql-connector instrumentor to support sqlcommenting.

To leverage existing code and reduce duplication, this updates DB-API integration instrumentor with new abstract classes BaseTracedConnectionProxy and BaseTracedCursorProxy, while keeping uses by aiopg, psycopg2, psycopg instrumentors the same.

To perform checks specific to mysql-connector, its instrumentor has new custom subclasses and methods that extend from DB-API integration, sort of like what psycopg instrumentor does. This is to enable_commenter at the db client cursor level (instead of DB-API level) and support mixes of cursor types instrumented. See below for More background.

Fixes #2902

Replaces other PR #2943

More about db client cursors and sqlcommenting

If an app-side, mysql-connector cursor is set with prepared=True, mysql-connector will perform native prepared statement execution (see docs here). If sqlcommenting is enabled with mysql-connector, native prepared statement execution would result in inaccuracy and a notable performance penalty. In the example MySQL general log below, we see multiple pairs of Prepare + Execute with the same statement after performing a parameterized query 3 times when prepared=True:

2024-11-19T01:22:04.911237Z	  770 Prepare	SELECT * FROM city WHERE id = ?
2024-11-19T01:22:04.911404Z	  770 Reset stmt
2024-11-19T01:22:04.911743Z	  770 Execute	SELECT * FROM city WHERE id = '1818'
2024-11-19T01:22:05.587278Z	  770 Prepare	SELECT * FROM city WHERE id = ?
2024-11-19T01:22:05.587438Z	  770 Reset stmt
2024-11-19T01:22:05.587728Z	  770 Execute	SELECT * FROM city WHERE id = '1818'
2024-11-19T01:22:06.179149Z	  770 Prepare	SELECT * FROM city WHERE id = ?
2024-11-19T01:22:06.179237Z	  770 Reset stmt
2024-11-19T01:22:06.179513Z	  770 Execute	SELECT * FROM city WHERE id = '1818'

Adding the same sqlcomment to both Prepare and Execute statements would break the 1:1 correlation between sqlcomment and span. It would also take more resources to append sqlcomment to both.

If queries are submitted as single Query statements (when prepared=False), then the 1:1 correlation is maintained:

2024-11-19T01:21:39.188226Z	  770 Query	SELECT * FROM city WHERE id = '1818'
2024-11-19T01:21:40.129791Z	  770 Query	SELECT * FROM city WHERE id = '1818'
2024-11-19T01:21:40.922572Z	  770 Query	SELECT * FROM city WHERE id = '1818'

Therefore, new helper is_mysql_connector_cursor_prepared checks and disables sqlcommenting only for that cursor if prepared=True. Other cursors will still be traced.

Currently, native prepared statement execution of MySQL statements is only a mysql-connector feature, and not in mysqlclient nor PyMySQL.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Unit tests

  • tox -e py312-test-instrumentation-dbapi
  • tox -e py312-test-instrumentation-mysql-0
  • tox -e py312-test-instrumentation-mysql-1

Manual testing

  • App instrumented with mysql-connector
  • App instrumented with mysqlclient, to check that DB-API functionality still works
  • App instrumented with psycopg2, to check that DB-API inheritance still works elsewhere

Does This PR Require a Core Repo Change?

  • Yes. - Link to PR:
  • No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

@tammy-baylis-swi tammy-baylis-swi changed the title Add mysql-connector instrumentor support for sqlcommenting WIP/PoC Add mysql-connector instrumentor support for sqlcommenting Nov 21, 2024
@tammy-baylis-swi tammy-baylis-swi changed the title WIP/PoC Add mysql-connector instrumentor support for sqlcommenting Add mysql-connector instrumentor support for sqlcommenting Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add sqlcommenting support to MySQL driver instrumentors
1 participant