Skip to content

Conversation

tammy-baylis-swi
Copy link
Contributor

@tammy-baylis-swi tammy-baylis-swi commented Oct 30, 2024

Description

Updates mysqlclient instrumentor to support sqlcommenting.

Depends on # 2897

Fixes # 2902

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?

Added unit tests. Used local installations of opentelemetry-instrumentation-dbapi (including updates from #2897) and upstream dependencies with this updated downstream instrumentor on a Flask app that use MySQLdb to query MySQL with general logs enabled to check sqlcomments.

Example general log entry after this update, which now includes sqlcomment:

2024-10-30T18:59:27.490681Z 10 Query SELECT * FROM city WHERE id = '1818' /*db_driver='MySQLdb%3A2.2.4',dbapi_level='2.0',dbapi_threadsafety=1,driver_paramstyle='format',mysql_client_version='3.3.8',traceparent='00-47db46923b76644d56b4ee08562f06d4-840aecaac0ab0b87-01'*/

Example corresponding exported span, whose span ID matches sqlcomment traceparent span ID:

InstrumentationScope opentelemetry.instrumentation.mysqlclient 0.49b0.dev
Span #0
    Trace ID       : 47db46923b76644d56b4ee08562f06d4
    Parent ID      : e27f1caee467892f
    ID             : 840aecaac0ab0b87
    Name           : SELECT
    Kind           : Client
    Start time     : 2024-10-30 18:59:27.490061885 +0000 UTC
    End time       : 2024-10-30 18:59:27.491399426 +0000 UTC
    Status code    : Unset
    Status message :
Attributes:
     -> db.system: Str(mysql)
     -> db.name: Str()
     -> db.statement: Str(SELECT * FROM city WHERE id = %s)
     -> net.peer.port: Int(3306)

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
Copy link
Contributor Author

Going to close this and put in a fresh PR later. There's more that I want to add.

Copy link
Contributor

@lzchen lzchen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might have to fix CHANGELOG and move changes to "unreleased"

@tammy-baylis-swi
Copy link
Contributor Author

tammy-baylis-swi commented Nov 19, 2024

Might have to fix CHANGELOG and move changes to "unreleased"

Addressed in 8d8187e for this one, and again in 1739d8e

Copy link
Member

@aabmass aabmass left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your patience with review!

@aabmass aabmass merged commit beff723 into open-telemetry:main Nov 21, 2024
566 checks passed
@tammy-baylis-swi tammy-baylis-swi deleted the mysqlclient-sqlcomment branch November 21, 2024 16:38
xrmx pushed a commit to xrmx/opentelemetry-python-contrib that referenced this pull request Jan 24, 2025
…y#2941)

* WIP

* Add _DB_DRIVER_ALIASES

* Add mysql_client_version to sqlcomment

* lint

* Fix existing tests

* lint test

* Add PyMySQL dbapi commenter case

* Add test

* Add test

* Add test

* Add tests

* Changelog

* calculate_commenter_data at init of DatabaseApiIntegration

* Add mysqlclient sqlcomment support

* Fix typo

* try-except if NoneType module

* Add unit test

* CHangelog

* mysqlclient instrument_connection with connect_module

* add tests

* more tests

* more tests 2

* lint

* Redesign tests

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

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants