opentelemetry-instrumentation-dbapi: instrument commit and rollback

[bmwalters]

Description

Adds instrumentation for database commit() and rollback() transaction operations across all DB-API instrumentations.

Re-open of #3964, which was auto-closed by the stale bot.

Type of change

• New feature (non-breaking change which adds functionality)

Changes

• Added commit() and rollback() span instrumentation to dbapi module
• Added enable_transaction_spans configuration flag (noted experimental; default: False)
• Created AsyncTracedConnectionProxy for async connections (psycopg)
• Plumbed configuration through all dependent instrumentors: pymysql, mysql, mysqlclient, psycopg, psycopg2, sqlite3, pymssql

How Has This Been Tested?

• Added unit tests for sync commit/rollback in test_dbapi_integration.py
• Added unit tests for sync and async commit/rollback in test_psycopg_integration.py
• Added functional tests for pymysql in test_pymysql_functional.py
• All existing tests pass

Exported trace example

From a fresh clone, with sqlcommenter fully on and enable_transaction_spans=True (the new opt-in):

git clone -b sql-transactions https://github.com/bmwalters/opentelemetry-python-contrib.git
cd opentelemetry-python-contrib
uv sync --frozen --all-packages

cat > /tmp/demo.py <<'PY'
import sqlite3
from opentelemetry import trace
from opentelemetry.instrumentation.dbapi import wrap_connect
from opentelemetry.sdk.trace import TracerProvider
from opentelemetry.sdk.trace.export import ConsoleSpanExporter, SimpleSpanProcessor

provider = TracerProvider()
provider.add_span_processor(SimpleSpanProcessor(ConsoleSpanExporter()))
trace.set_tracer_provider(provider)

# wrap_connect (not the SQLite3Instrumentor) is used so the full sqlcommenter
# config plumbs through; the instrumentor doesn't expose those kwargs.
wrap_connect(
    "demo", sqlite3, "connect", "sqlite",
    enable_commenter=True,
    enable_attribute_commenter=True,
    enable_transaction_spans=True,
)

conn = sqlite3.connect(":memory:")
cur = conn.cursor()
cur.execute("CREATE TABLE t (x INTEGER)")
cur.execute("INSERT INTO t VALUES (1)")
conn.commit()
cur.execute("INSERT INTO t VALUES (2)")
conn.rollback()
conn.close()
PY

uv run python /tmp/demo.py

INSERT:

{
  "name": "INSERT",
  "kind": "SpanKind.CLIENT",
  "attributes": {
    "db.system": "sqlite",
    "db.name": "",
    "db.statement": "INSERT INTO t VALUES (1) /*db_driver='sqlite3%%3Aunknown',dbapi_level='2.0',dbapi_threadsafety=3,driver_paramstyle='qmark',traceparent='00-39e356b6a8e6adb68f0e712bf67a14f4-fad576ffcb92787c-01'*/"
  }
}

COMMIT:

{
  "name": "COMMIT",
  "kind": "SpanKind.CLIENT",
  "attributes": {
    "db.system": "sqlite",
    "db.name": "",
    "db.operation.name": "COMMIT"
  }
}

ROLLBACK:

{
  "name": "ROLLBACK",
  "kind": "SpanKind.CLIENT",
  "attributes": {
    "db.system": "sqlite",
    "db.name": "",
    "db.operation.name": "ROLLBACK"
  }
}

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

[herin049]

Are these Span names a part of the Spec? Quickly glancing at the spec it doesn't seem like it is, in which case we probably don't want to support adding changes that aren't spec compliant.

[bmwalters]

@herin049 Good question. The DB semconv spec (database-spans.md, sql.md) doesn't model txn operations (only query spans), and a prior proposal open-telemetry/semantic-conventions#1134 was closed by the reporter without any action.

That said, db.operation.name is open-ended ("name of the operation or command being executed", captured "as provided", no fixed enum), and there's prior art in Java JDBC instrumentation which wraps Connection.commit()/rollback() and emits client spans named COMMIT/ROLLBACK with db.operation.name = "COMMIT"/"ROLLBACK". That instrumentation is gated behind an experimental opt-in flag.

Coming back to Python, this PR also matches CursorTracer.get_operation_name's existing convention of using the SQL keyword as the span name (SELECT, INSERT, etc). So I'd say this is a reasonable choice.

Based on this investigation, I did make two follow-up commits just now:

1. Set db.operation.name = "COMMIT" / "ROLLBACK" on the new spans (machine-readable; spec doesn't preclude this).
2. Flip enable_transaction_spans default to False and mark experimental in the CHANGELOG, to mirror the Java instrumentation's choice while the spec is unsettled.
   • I will also note that having this would have shortened the time to find a real production bug in our system.

Wdyt?

[herin049]

@herin049 Good question. The DB semconv spec (database-spans.md, sql.md) doesn't model txn operations (only query spans), and a prior proposal open-telemetry/semantic-conventions#1134 was closed by the reporter without any action.

That said, db.operation.name is open-ended ("name of the operation or command being executed", captured "as provided", no fixed enum), and there's prior art in Java JDBC instrumentation which wraps Connection.commit()/rollback() and emits client spans named COMMIT/ROLLBACK with db.operation.name = "COMMIT"/"ROLLBACK". That instrumentation is gated behind an experimental opt-in flag.

Coming back to Python, this PR also matches CursorTracer.get_operation_name's existing convention of using the SQL keyword as the span name (SELECT, INSERT, etc). So I'd say this is a reasonable choice.

Based on this investigation, I did make two follow-up commits just now:

1. Set db.operation.name = "COMMIT" / "ROLLBACK" on the new spans (machine-readable; spec doesn't preclude this).

2. Flip enable_transaction_spans default to False and mark experimental in the CHANGELOG, to mirror the Java instrumentation's choice while the spec is unsettled.

   • I will also note that having this would have shortened the time to find a real production bug in our system.

Wdyt?

I am not sure what stance the OpenTelemetry Java community has on this, but the stance that Python takes is that we should only add spec compliant signals. I would recommend raising an issue in semantic-conventions first, and then we can discuss adding these to the Python instrumentation libraries.