Skip to content

feat(dbapi): add comment_position option to control SQL comment placement#4561

Open
RiyaChaturvedi37 wants to merge 8 commits into
open-telemetry:mainfrom
RiyaChaturvedi37:feat/sqlcommenter-comment-position-dbapi
Open

feat(dbapi): add comment_position option to control SQL comment placement#4561
RiyaChaturvedi37 wants to merge 8 commits into
open-telemetry:mainfrom
RiyaChaturvedi37:feat/sqlcommenter-comment-position-dbapi

Conversation

@RiyaChaturvedi37
Copy link
Copy Markdown
Contributor

Problem
The SQL comment was always appended to the end of the query, which caused truncation issues in database query logs for long queries.
Solution
Added a comment_position parameter to trace_integration, wrap_connect, instrument_connection, and DatabaseApiIntegration that defaults to "end" (preserving existing behavior per OTel spec). Set comment_position="start" to prepend the comment instead.
Usage
pythonwrap_connect(
name,
mysql.connector,
"connect",
"mysql",
enable_commenter=True,
comment_position="start",
)

Default behavior unchanged
Part of a series of PRs adding comment_position support across DB-API, Django, Flask, SQLAlchemy, psycopg2, psycopg, mysql, mysqlclient, PyMySQL
Follows approach confirmed in #4490

Part of #3583

@RiyaChaturvedi37 RiyaChaturvedi37 requested a review from a team as a code owner May 9, 2026 06:35
@RiyaChaturvedi37
Copy link
Copy Markdown
Contributor Author

This is the first PR in a series adding comment_position support to sqlcommenter instrumentation libraries, as discussed with @tammy-baylis-swi in #4490. Starting with DB-API as the base library to confirm the approach before proceeding to Django, Flask, SQLAlchemy, psycopg2, psycopg, mysql, mysqlclient, and PyMySQL.

@RiyaChaturvedi37 RiyaChaturvedi37 force-pushed the feat/sqlcommenter-comment-position-dbapi branch 4 times, most recently from c098c79 to d504f0a Compare May 9, 2026 07:42
Comment thread CHANGELOG.md Outdated
@github-project-automation github-project-automation Bot moved this to Reviewed PRs that need fixes in Python PR digest May 10, 2026
@RiyaChaturvedi37
Copy link
Copy Markdown
Contributor Author

Fixed all the suggestions @herin049 — used Literal["start", "end"] type with a CommentPositionT alias, simplified the semicolon handling logic in sqlcommenter_utils.py, and cleaned up the changelog. Ready for re-review!

@RiyaChaturvedi37 RiyaChaturvedi37 requested a review from herin049 May 10, 2026 15:57
Comment thread CHANGELOG.md Outdated
@RiyaChaturvedi37 RiyaChaturvedi37 requested a review from herin049 May 11, 2026 06:32
Comment thread CHANGELOG.md Outdated
@RiyaChaturvedi37 RiyaChaturvedi37 force-pushed the feat/sqlcommenter-comment-position-dbapi branch from 236d8e0 to f360254 Compare May 12, 2026 06:10
@RiyaChaturvedi37 RiyaChaturvedi37 force-pushed the feat/sqlcommenter-comment-position-dbapi branch from 9d63a69 to f88ddcc Compare May 12, 2026 06:24
@RiyaChaturvedi37 RiyaChaturvedi37 force-pushed the feat/sqlcommenter-comment-position-dbapi branch from 2cf3dac to 8020903 Compare May 12, 2026 06:34
@RiyaChaturvedi37 RiyaChaturvedi37 requested a review from herin049 May 12, 2026 07:03
@emdneto
Copy link
Copy Markdown
Member

emdneto commented May 12, 2026

Thanks for the PR!

Just a heads-up: we no longer update CHANGELOG.md directly. The changelog is now generated from changelog fragments using Towncrier.

Please add the appropriate changelog fragment for this change instead of editing CHANGELOG.md manually. You can find the instructions and expected format in CONTRIBUTING.md.

@RiyaChaturvedi37
Copy link
Copy Markdown
Contributor Author

The misc/typecheck failure is in util/opentelemetry-util-genai/src/opentelemetry/util/genai/completion_hook.py which was last modified by upstream commits (#4533, #4506) unrelated to my PR. All lint and test checks for my changes are passing. @herin049 @emdneto could you confirm this is a pre-existing issue?

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

Labels

None yet

Projects

Status: Reviewed PRs that need fixes

Development

Successfully merging this pull request may close these issues.

3 participants