Skip to content

Fix psycopg_pool instrumentation to restore Postgres spans #2460

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix psycopg_pool instrumentation to restore Postgres spans #2460

wants to merge 1 commit into from

Conversation

@JaeHyuckSa
Copy link

@JaeHyuckSa JaeHyuckSa commented Oct 17, 2025

What does this pull request do?

  • instrument psycopg.Connection.connect so that psycopg_pool.ConnectionPool-managed connections are proxied and DB spans are emitted again

Related issues

Closes #2094

@JaeHyuckSa JaeHyuckSa requested a review from a team as a code owner October 17, 2025 18:13
@cla-checker-service
Copy link

cla-checker-service bot commented Oct 17, 2025 •
edited
Loading

💚 CLA has been signed

@github-actions
Copy link

github-actions bot commented Oct 17, 2025

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

@github-actions github-actions bot added agent-python community Issues opened by the community triage Issues awaiting triage labels Oct 17, 2025
@JaeHyuckSa JaeHyuckSa force-pushed the fix/psycopg-pool-instrumentation branch from 0b6bbdd to a1200b5 Compare October 17, 2025 18:17
@JaeHyuckSa JaeHyuckSa mentioned this pull request Oct 20, 2025
Open
xrmx
xrmx reviewed Oct 30, 2025
View reviewed changes
Copy link
Member

@xrmx xrmx left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. We are using - as separator so the test framework name should be called psycopg_pool and reqs and envs file should be renamed. Also in order to run tests in CI you need to add an entry psycopg_pool-newest in .ci/.matrix_framework.yml and .ci/.matrix_framework_full.yml.

@xrmx
Copy link
Member

xrmx commented Nov 4, 2025

@JaeHyuckSa The tests file name was good, the requirements and environments one are wrong. Also the matrix files update is missing.

@JaeHyuckSa JaeHyuckSa force-pushed the fix/psycopg-pool-instrumentation branch 2 times, most recently from 4ed3c00 to c30e88b Compare November 5, 2025 15:35
@JaeHyuckSa JaeHyuckSa force-pushed the fix/psycopg-pool-instrumentation branch from c30e88b to e67babb Compare November 5, 2025 15:38
@JaeHyuckSa
Copy link
Author

JaeHyuckSa commented Nov 5, 2025

@xrmx Thank you for the review. I was going to make some other changes back then but forgot about it… I’ve now reflected everything you mentioned!

xrmx
xrmx reviewed Nov 19, 2025
View reviewed changes
Copy link
Member

@xrmx xrmx left a comment

Choose a reason for hiding this comment

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

Tests are failing, PTAL

tests/instrumentation/psycopg_pool_tests.py
),
min_size=1,
max_size=2,
open=True,
Copy link
Member

@xrmx xrmx Nov 19, 2025

Choose a reason for hiding this comment

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

this doesn't work when testing with python3.6 and maybe on other older python versions because the psycopg version compatible with that (3.0.x) didn't have this

xrmx
xrmx reviewed Nov 19, 2025
View reviewed changes
Copy link
Member

@xrmx xrmx left a comment

Choose a reason for hiding this comment

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

/test matrix

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

Reviewers

@xrmx xrmx xrmx left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

agent-python community Issues opened by the community triage Issues awaiting triage

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

psycopg pool not instrumented

2 participants

@JaeHyuckSa @xrmx