-
Notifications
You must be signed in to change notification settings - Fork 219
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 asyncpg instrumentation in Elastic APM #889
Conversation
💚 CLA has been signed |
💚 Build SucceededExpand to view the summary
Build stats
Test stats 🧪
Steps errorsExpand to view the steps failures
|
hey there @basepi! Thanks for checking out my PR. it's not ready yet, so more commits will come (I need to update the tests). |
Nope, shows signed. Sometimes it just lags a bit. Thanks, this looks awesome! |
…hon into add_support_asyncpg
hey @basepi! Could you let me know how I can run CI tests as I am currently getting this error: |
hi @basepi :) I was wondering if you have had some time to have a look at my comment above :) |
thanks for letting me know @sethmlarson 🥇 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hey @beniwohli and @basepi! I have implemented all the requested changes. Please have a look at the PR if it looks good now and let me know if I should improve anything else :) |
@odimko great work on the updates. The tests currently fails because asyncpg isn't able to parse the DSN. Turns out that for asyncpg, the DSN has a different format than psycopg2, using a URL format: https://magicstack.github.io/asyncpg/current/api/index.html#asyncpg.connection.connect You should be able to adapt the |
@beniwohli and @basepi, hey there :) all tests pass successfully except for linting. any suggestions on how I can run it? otherwise, I would appreciate if someone could run it so that the last flailing check will pass :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! Thanks a lot for your contribution and sorry about the sometimes slow answer times!
No problem at all! Thanks for supporting with it 🥇 @beniwohli, may I ask you when it will be released? |
We don't currently have a release date planned, but I expect it will be Soon (tm) since we have a couple of useful features that are currently unreleased (and since we no longer need to include the removal of py3.5 in the release). |
got it, thanks for explaining! |
Good morning @basepi and @beniwohli! just checking in to check what is your plan regarding releasing the next version of the APM package. could you please let me know? |
Very soon. We'd really like to get #927 in, but it needs some test fixes. |
perfect, thanks for the update! |
awesome! thanks a lot! |
Issue elastic#889 added instrumentation for asyncpg, though only `execute` and `executemany` were instrumented. This extends the instrumentation to include `fetch`, `fetchval` and `fetchrow` methods.
Issue #889 added instrumentation for asyncpg, though only `execute` and `executemany` were instrumented. This extends the instrumentation to include `fetch`, `fetchval` and `fetchrow` methods.
* Add a new file * Add AsyncPGInstrumentation to register.py * Fix a typo in register.py * Use correct function for registering * Update the docs + add a template for the tests * Update variable names in the tests * Add asyncpg.sh for the tests * Remove unused docstrings * Improve tests * Remove callproc test * PR suggestions * FIx tests: use URI instead of explicit parameters * FIx tests: implement further PR suggestions * FIx tests: implement more PR suggestions * Use Connection.execute and Connection.executemany instead of Connection._do_execute * Remove unnecessary requirements * Fix a type in the tests * Use yield and proper awaiting in the tests * Use yield and proper awaiting in the tests + fix a typo * Fix tests * Additional fixes to tests * fix typo Co-authored-by: Colton Myers <colton.myers@gmail.com> Co-authored-by: Benjamin Wohlwend <bw@piquadrat.ch>
Issue elastic#889 added instrumentation for asyncpg, though only `execute` and `executemany` were instrumented. This extends the instrumentation to include `fetch`, `fetchval` and `fetchrow` methods.
* Add a new file * Add AsyncPGInstrumentation to register.py * Fix a typo in register.py * Use correct function for registering * Update the docs + add a template for the tests * Update variable names in the tests * Add asyncpg.sh for the tests * Remove unused docstrings * Improve tests * Remove callproc test * PR suggestions * FIx tests: use URI instead of explicit parameters * FIx tests: implement further PR suggestions * FIx tests: implement more PR suggestions * Use Connection.execute and Connection.executemany instead of Connection._do_execute * Remove unnecessary requirements * Fix a type in the tests * Use yield and proper awaiting in the tests * Use yield and proper awaiting in the tests + fix a typo * Fix tests * Additional fixes to tests * fix typo Co-authored-by: Colton Myers <colton.myers@gmail.com> Co-authored-by: Benjamin Wohlwend <bw@piquadrat.ch>
Issue elastic#889 added instrumentation for asyncpg, though only `execute` and `executemany` were instrumented. This extends the instrumentation to include `fetch`, `fetchval` and `fetchrow` methods.
* Add a new file * Add AsyncPGInstrumentation to register.py * Fix a typo in register.py * Use correct function for registering * Update the docs + add a template for the tests * Update variable names in the tests * Add asyncpg.sh for the tests * Remove unused docstrings * Improve tests * Remove callproc test * PR suggestions * FIx tests: use URI instead of explicit parameters * FIx tests: implement further PR suggestions * FIx tests: implement more PR suggestions * Use Connection.execute and Connection.executemany instead of Connection._do_execute * Remove unnecessary requirements * Fix a type in the tests * Use yield and proper awaiting in the tests * Use yield and proper awaiting in the tests + fix a typo * Fix tests * Additional fixes to tests * fix typo Co-authored-by: Colton Myers <colton.myers@gmail.com> Co-authored-by: Benjamin Wohlwend <bw@piquadrat.ch>
Issue elastic#889 added instrumentation for asyncpg, though only `execute` and `executemany` were instrumented. This extends the instrumentation to include `fetch`, `fetchval` and `fetchrow` methods.
What does this pull request do?
Adds support for the
asyncpg
database interface library for Python and PostgreSQL by implementing corresponding instrumentation in the APM agent.Related issues
#755
Closes issues
#755