Skip to content

Conversation

chloe-zh97
Copy link

@chloe-zh97 chloe-zh97 commented Sep 9, 2025

Description

This PR adds support for passing commenter_options to the trace_integration function and DatabaseApiIntegration in the opentelemetry-instrumentation-dbapi package. This allows users to customize SQLCommenter behavior (for example, disabling opentelemetry_values) when instrumenting DB-API drivers.

Fixes #3726

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

  • Added two unit tests test_commenter_options_propagation and test_trace_integration_passes_commenter_options
  • tox -e lint and pytest.
  • tox -e ruff for code formatting

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

Copy link

linux-foundation-easycla bot commented Sep 9, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

@chloe-zh97 chloe-zh97 changed the title add commenter_options in trace_integration [WIP]add commenter_options in trace_integration Sep 9, 2025
@chloe-zh97 chloe-zh97 requested a review from a team as a code owner September 9, 2025 07:20
@tammy-baylis-swi
Copy link
Contributor

Hi @chloe-zh97 thanks for this. A few more requests:

  • Please resolve merge conflicts in Changelog.
  • On your local, please run tox -e ruff then commit the code formatting fixes.
  • If the PR is ready for review, please remove "WIP" from PR title for clairty.

@chloe-zh97 chloe-zh97 closed this Sep 10, 2025
@chloe-zh97 chloe-zh97 force-pushed the issue-3726-trace-integration-support-args branch from 0fcd925 to edb34e6 Compare September 10, 2025 04:56
@chloe-zh97 chloe-zh97 reopened this Sep 10, 2025
@chloe-zh97 chloe-zh97 changed the title [WIP]add commenter_options in trace_integration add commenter_options in trace_integration Sep 10, 2025
@chloe-zh97
Copy link
Author

  • tox -e lint

Thanks, done.

Copy link
Contributor

@tammy-baylis-swi tammy-baylis-swi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm! The Maintainers will also have a look at this PR.

@tammy-baylis-swi tammy-baylis-swi requested a review from a team September 10, 2025 16:57
@chloe-zh97
Copy link
Author

Lgtm! The Maintainers will also have a look at this PR.

Okay, thank you!

CHANGELOG.md Outdated
([#3734](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3734))
- `opentelemetry-instrumentation`: botocore: upgrade moto package from 5.0.9 to 5.1.11
([#3736](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3736))
- `opentelemetry-instrumentation-dbapi`: Add support for `commenter_options` in `trace_integration` to control SQLCommenter behavior
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be moved to unreleased

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, done.

capture_parameters: bool = False,
enable_commenter: bool = False,
db_api_integration_factory: type[DatabaseApiIntegration] | None = None,
commenter_options: dict[str, Any] | None = None,
Copy link
Contributor

@xrmx xrmx Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this has a default I would move it as last element so that previous callers using args would still be fine

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, done.

@chloe-zh97 chloe-zh97 requested a review from xrmx September 20, 2025 02:00
@chloe-zh97 chloe-zh97 closed this Sep 23, 2025
@chloe-zh97 chloe-zh97 force-pushed the issue-3726-trace-integration-support-args branch from eb3f469 to b232b9a Compare September 23, 2025 19:53
@github-project-automation github-project-automation bot moved this from Ready for review to Done in @xrmx's Python PR digest Sep 23, 2025
@chloe-zh97 chloe-zh97 reopened this Sep 23, 2025
@chloe-zh97
Copy link
Author

Hi @xrmx, could you help review again, many thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

DB-API Instrumentor trace_integration doesn't support all wrap_connect args.
7 participants