opentelemetry-instrumentation-dbapi: instrument commit and rollback - #4906
Open
bmwalters wants to merge 11 commits into
Open
opentelemetry-instrumentation-dbapi: instrument commit and rollback#4906bmwalters wants to merge 11 commits into
bmwalters wants to merge 11 commits into
Conversation
Aligns transaction spans with the OTel semconv db attribute that names the operation. The span name (COMMIT/ROLLBACK) was previously the only way for a backend to know what operation produced the span; the attribute makes it queryable. Assisted-by: Claude Opus 4.7
The OTel DB semconv spec is silent on transaction lifecycle operations (commit/rollback). Java's JDBC instrumentation gates the same behavior behind an experimental opt-in flag; matching that posture here while the spec is unsettled. Existing tests opt in explicitly; CHANGELOG entry consolidated and marked experimental. Assisted-by: Claude Opus 4.7
The rest of the dbapi instrumentation (DB_NAME, DB_SYSTEM, DB_STATEMENT) uses the pre-migration semconv attributes. Use the matching pre-migration DB_OPERATION (db.operation) on the new commit/rollback spans instead of the post-migration DB_OPERATION_NAME (db.operation.name) for consistency, as suggested by lmolkova. Migration to the new database semantic conventions is out of scope for this PR. Assisted-by: Claude Opus 4.7
Catch exceptions raised by the underlying connection during commit() or rollback(), set the error.type span attribute (per the new database semantic conventions) to the exception's qualified class name, and re-raise. Span status is set to ERROR and the exception is recorded as an event by the start_as_current_span context manager. This matches the pattern used in the click and asyncclick instrumentations. Add failure tests in test_dbapi_integration.py covering the sync path and in test_psycopg_integration.py covering both the sync and async paths. The tests use a method-level throw_exception kwarg on the mock connection methods, mirroring the existing convention used by mock cursor methods. The TracedConnectionProxy commit/rollback methods now forward *args/**kwargs to the underlying connection so the test kwarg reaches the mock; this matches how the proxy already forwards args for cursor(). Suggested by lmolkova. Assisted-by: Claude Opus 4.7
Per maintainer feedback on open-telemetry#4519, the project moved to towncrier for changelog management. Add the entry as a fragment under .changelog/ and revert the direct CHANGELOG.md modification. Assisted-by: Claude Opus 4.7
Migrates the new transaction-span code to the semconv stability pattern introduced on main in open-telemetry#4109: - Replace direct DB_OPERATION / DB_SYSTEM / DB_NAME imports with the _set_db_operation / _set_db_system / _set_db_name helpers from opentelemetry.instrumentation._semconv, so transaction spans emit db.operation or db.operation.name (and equivalents) based on OTEL_SEMCONV_STABILITY_OPT_IN. - Gate error.type on _report_new(sem_conv_mode), matching requests/urllib. - Add new_semconv / both_semconv variants for the transaction tests in test_dbapi_integration.py to cover the migration matrix. - Drop the pre-semconv ERROR_TYPE failure-test assertions in psycopg, since ERROR_TYPE is now gated to new semconv mode and the dbapi-level new_semconv tests cover the gating behavior. Assisted-by: Claude Opus 4.7
- make enable_transaction_spans keyword-only on wrap_connect and instrument_connection to stay under max-positional-arguments - make wrap_cursors keyword-only on the connection proxy helpers to avoid keyword-arg-before-vararg - suppress broad-exception-raised on the mock connection commit/rollback, matching the existing mock cursor pattern - add too-many-lines module pragma - apply ruff-format Assisted-by: Claude Opus 4.8
# Conflicts: # instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py
Assisted-by: Claude Opus 5
bmwalters
force-pushed
the
sql-transactions
branch
from
August 1, 2026 19:15
6fbd5ec to
6f58a60
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds instrumentation for database
commit()androllback()transaction operations across all DB-API instrumentations.Supersedes #4519, which superseded #3964. Prior review discussion is on #4519.
Type of change
Changes
commit()androllback()span instrumentation todbapi.enable_transaction_spansconfiguration flag (experimental; defaultFalse).AsyncTracedConnectionProxyfor async connections (psycopg).OTEL_SEMCONV_STABILITY_OPT_INselects old / new / dup attribute names.ERROR;error.typeis added under new DB semconv.How Has This Been Tested?
test_dbapi_integration.pyfor sync commit/rollback, parameterized across default /database/database/dupsemconv modestest_psycopg_integration.pyfor sync and async commit/rollbacktest_pymysql_functional.pyExported trace example
From a fresh clone, with sqlcommenter fully on and
enable_transaction_spans=True(the new opt-in):Default semconv mode:
{ "name": "INSERT", "kind": "SpanKind.CLIENT", "attributes": { "db.system": "sqlite", "db.statement": "INSERT INTO t VALUES (1) /*db_driver='sqlite3%%3Aunknown',dbapi_level='2.0',dbapi_threadsafety=3,driver_paramstyle='qmark',traceparent='00-5f7bc80c598cfab29da35b2a3028e582-bf7b4514b5d50195-03'*/" } } { "name": "COMMIT", "kind": "SpanKind.CLIENT", "attributes": { "db.system": "sqlite", "db.operation": "COMMIT" } } { "name": "ROLLBACK", "kind": "SpanKind.CLIENT", "attributes": { "db.system": "sqlite", "db.operation": "ROLLBACK" } }Under
OTEL_SEMCONV_STABILITY_OPT_IN=database, all spans use the new attribute names; failed operations additionally carryerror.type. Example failed COMMIT:{ "name": "COMMIT", "kind": "SpanKind.CLIENT", "status": { "status_code": "ERROR", "description": "ProgrammingError: Cannot operate on a closed database." }, "attributes": { "db.system.name": "sqlite", "db.operation.name": "COMMIT", "error.type": "ProgrammingError" }, "events": [ { "name": "exception", "attributes": { "exception.type": "sqlite3.ProgrammingError", "exception.message": "Cannot operate on a closed database.", "exception.escaped": "False" } } ] }Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.