Support enable_commenter for instrument_connection by psycopg(2 by tammy-baylis-swi · Pull Request #3071 · open-telemetry/opentelemetry-python-contrib

tammy-baylis-swi · 2024-12-05T23:28:50Z

Description

Depends on #3043 to stop errors with manual tests (instrumenting an app and calling instrument_connection). Fixes docs and there will be merge conflicts.

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?

tox -e py312-test-instrumentation-psycopg2
tox -e py312-test-instrumentation-psycopg
Instrumented db client app that calls Psycopg2Instrumentor.instrument_connection
Instrumented db client app that calls PsycopgInstrumentor.instrument_connection

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

tammy-baylis-swi · 2024-12-07T01:53:21Z

Sorry to drag in several participants automatically! I committed changes I didn't mean to, and it's fixed now.

This reverts commit e3d7941.

.../opentelemetry-instrumentation-psycopg/src/opentelemetry/instrumentation/psycopg/__init__.py

tammy-baylis-swi · 2026-02-19T01:31:23Z

Hi @open-telemetry/opentelemetry-python-contrib-approvers it's been a while for this one. May I please have a review?

Related PR #3043 has been approved though not yet merged.

herin049 · 2026-02-19T01:52:54Z

instrumentation/opentelemetry-instrumentation-psycopg/tests/test_psycopg_integration.py

+            trace_id = format(span.get_span_context().trace_id, "032x")
+            self.assertEqual(
+                MockCursor.execute.call_args[0][0],
+                f"Select 1 /*db_driver='psycopg%%3Afoobar',dbapi_level='123',dbapi_threadsafety='123',driver_paramstyle='test',libpq_version='foobaz',traceparent='00-{trace_id}-{span_id}-01'*/",


Can we update this to use {trace_flags:02x} instead of hard coding 01 for the trace flags since this will change with the introduction of the random trace id flag. Context: open-telemetry/opentelemetry-python#4854

Great call. I've done this in the other PR (#4267)

MikeGoldsmith · 2026-02-19T13:11:20Z

.../opentelemetry-instrumentation-psycopg/src/opentelemetry/instrumentation/psycopg/__init__.py

+        enable_commenter: bool = False,
+        commenter_options: dict = None,
+        enable_attribute_commenter: bool = False,
+    ):


Should we keep the old return type of -> ConnectionT?

Yes! I've done this in other PR (#4267)

MikeGoldsmith · 2026-02-19T13:13:11Z

...pentelemetry-instrumentation-psycopg2/src/opentelemetry/instrumentation/psycopg2/__init__.py

+        enable_commenter: bool = False,
+        commenter_options: dict = None,
+        enable_attribute_commenter=None,
+    ):


Same here, should we keep -> ConnectionT?

For psycopg2 maybe not as it doesn't support the same Async* types that psycopg(3) does.

MikeGoldsmith · 2026-02-19T13:15:58Z

...pentelemetry-instrumentation-psycopg2/src/opentelemetry/instrumentation/psycopg2/__init__.py

            connection._is_instrumented_by_opentelemetry = False

        if not connection._is_instrumented_by_opentelemetry:
            setattr(


Will this conflict with your updates in #3043?

Won't be doing that PR anymore

MikeGoldsmith · 2026-02-19T13:18:20Z

.../opentelemetry-instrumentation-psycopg/src/opentelemetry/instrumentation/psycopg/__init__.py

-    base_factory: type[psycopg.Cursor] | None = None,
-    tracer_provider: TracerProvider | None = None,
+    db_api: dbapi.DatabaseApiIntegration = None,
+    base_factory: psycopg.Cursor = None,


I think we should keep as base_factory: type[psycopg.Cursor] | None = None

Yes! Kept in other PR (#4267)

MikeGoldsmith · 2026-02-19T13:22:20Z

...pentelemetry-instrumentation-psycopg2/src/opentelemetry/instrumentation/psycopg2/__init__.py

-def _new_cursor_factory(db_api=None, base_factory=None, tracer_provider=None):
+def _new_cursor_factory(
+    db_api: dbapi.DatabaseApiIntegration = None,
+    base_factory: pg_cursor = None,


I think would be something like base_factory: typing.Optional[typing.Type[pg_cursor]] = None

Yes! Put in other PR (#4267)

tammy-baylis-swi · 2026-02-26T22:36:20Z

Closing this because I've closed dependent #3043

Will be replaced by #4267 when ready.

enable_commenter for instrument_connection psycopg(2)

d2da8cd

github-actions bot assigned federicobond Dec 5, 2024

github-actions bot requested a review from federicobond December 5, 2024 23:29

tammy-baylis-swi added 2 commits December 5, 2024 15:32

Changelog

4783f3a

Add psycopg2 tests

bf49c8c

tammy-baylis-swi force-pushed the enable-commenter-instrument-connection branch from b102323 to bf49c8c Compare December 7, 2024 01:35

github-actions bot assigned codeboten, NathanielRN, nikosokolik, oxeye-nikolay, shalevr and srprash Dec 7, 2024

tammy-baylis-swi unassigned federicobond, codeboten, NathanielRN, srprash, shalevr, nikosokolik and oxeye-nikolay Dec 7, 2024

github-actions bot assigned federicobond Dec 7, 2024

Add psycopg tests

e148c9f

tammy-baylis-swi force-pushed the enable-commenter-instrument-connection branch from 4f969ef to e148c9f Compare December 7, 2024 01:52

tammy-baylis-swi added 2 commits December 6, 2024 18:16

Update typehints

58daeb6

Document psycopg(2) instrument_connection

8e8b759

tammy-baylis-swi marked this pull request as ready for review December 7, 2024 02:23

tammy-baylis-swi requested a review from a team as a code owner December 7, 2024 02:23

tammy-baylis-swi added 3 commits December 10, 2024 16:28

Consistent pg_connection alias usage

e3d7941

Revert "Consistent pg_connection alias usage"

a94a591

This reverts commit e3d7941.

Use qualified import not alias

0e24d6d

tammy-baylis-swi and others added 7 commits December 19, 2024 10:33

Merge branch 'main' into enable-commenter-instrument-connection

64941f9

Merge branch 'main' into enable-commenter-instrument-connection

e6ca8fe

Fix

84d82d0

Merge branch 'main' into enable-commenter-instrument-connection

65ac19a

Merge branch 'main' into enable-commenter-instrument-connection

1ca0cd0

Merge branch 'main' into enable-commenter-instrument-connection

afff465

Merge branch 'main' into enable-commenter-instrument-connection

fd11f1a

lzchen reviewed Jan 14, 2025

View reviewed changes

.../opentelemetry-instrumentation-psycopg/src/opentelemetry/instrumentation/psycopg/__init__.py Show resolved Hide resolved

tammy-baylis-swi and others added 3 commits January 14, 2025 09:26

Merge branch 'main' into enable-commenter-instrument-connection

90051b7

psycopg(2) instrument_connection support enable_attr_commenter

3587846

Merge branch 'main' into enable-commenter-instrument-connection

a738d5e

tammy-baylis-swi requested a review from lzchen January 14, 2025 17:55

tammy-baylis-swi and others added 8 commits January 15, 2025 09:54

Merge branch 'main' into enable-commenter-instrument-connection

eeab531

Fix imports after main changes

e3db4a8

Merge branch 'main' into enable-commenter-instrument-connection

1526aaa

Merge branch 'main' into enable-commenter-instrument-connection

e081502

Changelog

48c8a9f

Merge branch 'main' into enable-commenter-instrument-connection

5e81f06

Merge branch 'main' into enable-commenter-instrument-connection

cdfc2e5

Add to async_factory

458d297

tammy-baylis-swi requested review from a team and removed request for federicobond February 19, 2026 01:29

herin049 reviewed Feb 19, 2026

View reviewed changes

MikeGoldsmith reviewed Feb 19, 2026

View reviewed changes

tammy-baylis-swi added the do not merge label Feb 25, 2026

tammy-baylis-swi mentioned this pull request Feb 26, 2026

Support enable_commenter for instrument_connection (version 2) #4267

Draft

14 tasks

tammy-baylis-swi closed this Feb 26, 2026

github-project-automation bot moved this from Ready for review to Done in @xrmx's Python PR digest Feb 26, 2026

Conversation

tammy-baylis-swi commented Dec 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

How Has This Been Tested?

Does This PR Require a Core Repo Change?

Checklist:

Uh oh!

tammy-baylis-swi commented Dec 7, 2024

Uh oh!

Uh oh!

tammy-baylis-swi commented Feb 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tammy-baylis-swi commented Feb 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

tammy-baylis-swi commented Dec 5, 2024 •

edited

Loading