Skip to content
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.

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

add support for psycopg3 #396

Merged
merged 6 commits into from
Jan 10, 2025
Merged

add support for psycopg3 #396

merged 6 commits into from
Jan 10, 2025

Conversation

Dilski
Copy link
Contributor

@Dilski Dilski commented Jul 21, 2023

Issue #, if available: #395

Description of changes: Add support for psycopg3 (now just psycopg)

I have not been able to test this locally, as I can't find any development documentation in this repo.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@Dilski Dilski requested a review from a team as a code owner July 21, 2023 13:46
@carolabadeer
Copy link
Contributor

carolabadeer commented Jul 31, 2023

Hi @Dilski, thanks for contributing this instrumentation!
Some workflow unit tests are failing, can you please take a look and address the issues? Also curious if you are still unable to test this instrumentation end-to-end locally? Hopefully these steps can help:

  • Set up a simple sample application that makes a psycopg3 request, imports & uses the new instrumentation in this PR
  • Set up the AWS X-Ray daemon locally
  • Run the sample app once you have the daemon running as a sidecar. The X-Ray console should reflect the instrumented psycopg3 call

Please let us know if you have any questions or need more help setting up a local end-to-end test. (Referencing #395)

@csteinle
Copy link
Contributor

Hi @Dilski, thanks for contributing this instrumentation! Some workflow unit tests are failing, can you please take a look and address the issues? Also curious if you are still unable to test this instrumentation end-to-end locally? Hopefully these steps can help:

  • Set up a simple sample application that makes a psycopg3 request, imports & uses the new instrumentation in this PR
  • Set up the AWS X-Ray daemon locally
  • Run the sample app once you have the daemon running as a sidecar. The X-Ray console should reflect the instrumented psycopg3 call

Please let us know if you have any questions or need more help setting up a local end-to-end test. (Referencing #395)

Hi @carolabadeer - we've tried to address the issues and have managed to run the test suite at our side. Can we request a re-review?

Copy link
Contributor

@carolabadeer carolabadeer left a comment

Choose a reason for hiding this comment

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

Just a few small comments, but otherwise looking good!

Thanks @Dilski and @csteinle for addressing the CI issues and testing the instrumentation locally. I think it would also be very helpful to add a new section to the ReadMe for this instrumentation. It can contain a short code snippet (similar to these sections) on how to setup and use the psycopg3 instrumentation

aws_xray_sdk/ext/psycopg/patch.py Show resolved Hide resolved
aws_xray_sdk/ext/psycopg/patch.py Show resolved Hide resolved
@jhubberts
Copy link

Hey folks, any progress on this? I'd also like to take advantage of this patching logic once it's in, and am weighing whether it's worth waiting for a contribution back to aws_xray_sdk, or just patching it in my own package.

@Dilski
Copy link
Contributor Author

Dilski commented Oct 9, 2023

@jhubberts this is something we want in, but haven't prioritised/had the time to address feedback on. Any help would be appreciated

@baliame
Copy link

baliame commented Nov 12, 2024

Hi, this seems to have been idle for over a year. Is there a chance support for psycopg3 will be picked up in the near future?

@wangzlei wangzlei merged commit 17b033e into aws:master Jan 10, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants