open-telemetry / opentelemetry-python-contrib

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

DB-API instrumentor populates span after sqlcomment creation, not before #2935

Closed tammy-baylis-swi closed 6 days ago

tammy-baylis-swi commented 4 weeks ago

Description

Re-orders database query span population by DB-API instrumentor so that db.statement span attribute includes any sqlcomments generated.

Fixes https://github.com/open-telemetry/opentelemetry-python-contrib/issues/2936

Type of change

Please delete options that are not relevant.

How Has This Been Tested?

Added unit test. Locally installed this version of opentelemetry-instrumentation-dbapi, upstream dependencies, and downstream instrumentors for psycopg2, psycopg that use all of the above. Instrumented two Flask app that query PostgreSQL with general logs enabled to check sqlcomments: one via psycopg2, one with psycopg2. sqlcommenter enabled as per docs (here and here). Spans were exported to a locally-running otel-collector to inspect attributes including db.statement.

After changes, example span from psycopg2 query with updated db.statement:

ScopeSpans #0
ScopeSpans SchemaURL:
InstrumentationScope opentelemetry.instrumentation.psycopg2 0.49b0.dev
Span #0
    Trace ID       : acbdad825030e4fc58260b2bd9861eb3
    Parent ID      : b1b7f10bb5c841ff
    ID             : 451dac2c63520272
    Name           : SELECT
    Kind           : Client
    Start time     : 2024-10-29 19:17:38.333191847 +0000 UTC
    End time       : 2024-10-29 19:17:38.335095514 +0000 UTC
    Status code    : Unset
    Status message :
Attributes:
     -> db.system: Str(postgresql)
     -> db.name: Str(world-db)
     -> db.statement: Str(SELECT * FROM city WHERE id = %s /*db_driver='psycopg2%%3A2.9.9',traceparent='00-acbdad825030e4fc58260b2bd9861eb3-451dac2c63520272-01'*/)
     -> db.user: Str(world)
     -> net.peer.name: Str(postgres-world-db)
     -> net.peer.port: Int(5432)

which matches sqlcomment in corresponding postgresql general log:

2024-10-29 19:17:30 UTC,4/3,0,LOG,00000,"statement: SELECT * FROM city WHERE id = '1818' /*db_driver='psycopg2%3A2.9.9',traceparent='00-acbdad825030e4fc58260b2bd9861eb3-451dac2c63520272-01'*/",,,,,,,,,"","client backend",,0

Before changes, db.statement does not include sqlcomment (trimmed for clarity):

Attributes:
     -> db.statement: Str(SELECT * FROM city WHERE id = %s)

Does This PR Require a Core Repo Change?

Checklist:

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