open-telemetry / opentelemetry-python-contrib

OpenTelemetry instrumentation for Python modules
https://opentelemetry.io
Apache License 2.0
739 stars 613 forks source link

Add mysqlclient instrumentor support for sqlcommenting #2941

Closed tammy-baylis-swi closed 4 days ago

tammy-baylis-swi commented 3 weeks ago

Description

Updates mysqlclient 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 (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?

Checklist:

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

tammy-baylis-swi commented 3 weeks ago

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

tammy-baylis-swi commented 2 weeks ago

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 mysqlclient/MySQLdb driver might generate. The driver implementation seems to not support prepared statements (absence in docs here), so it seems to not be an issue.

tammy-baylis-swi commented 6 days ago

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

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