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

feat(pymongo): add PyMongo integration #1590

Merged
merged 13 commits into from Nov 4, 2022

Conversation

Agalin
Copy link
Contributor

@Agalin Agalin commented Aug 31, 2022

Adds breadcrumbs and performance traces for PyMongo queries using an
official monitoring API. Integration is similar to the one available in
OpenTelemetry, tags set to values recommended for attributes by OT as
specified in Span Operations guidelines.

PyMongo version selection explanation:

  • 3.1 - introduction of monitoring API. Only Python 2.7 and 3.6
    supported.
  • 3.12 - latest 3.x release, support for 2.7, 3.6-3.9 (3.7-3.9 added in
    various minor releases between 3.1 and 3.12).
  • 4.0 - no support for 2.7, added support for 3.10.
  • 4.1 - no support for 3.6.0-3.6.1.
  • 4.2 - no support for any 3.6.

Uses MockupDB for tests (Python implementation of MongoDB protocol server). mongomock is a reimplementation of the MongoClient and doesn't support tracing. This way we get real world like results. Note that pymongo has changed queries sent for some requests between 3.1 and 3.12 (probably 3.7 or 3.11) so additional responder is necessary to handle tests with 3.1.

There is no support for GridFS nor server connect/disconnect events. Those can be added in the future although I don't think there is an API similar to pymongo.monitoring for GridFS (so monkeypatching would be necessary in this one).

closes #1585

@Agalin
Copy link
Contributor Author

Agalin commented Aug 31, 2022

BTW I really hope spans like db.find are visible in DB spans panel and similar places, didn't have time to dive into implementation details of span retrieval endpoints.

@sl0thentr0py sl0thentr0py added Status: Backlog New Integration Integrating with a new framework or library labels Aug 31, 2022
@Agalin
Copy link
Contributor Author

Agalin commented Aug 31, 2022

Docs PR: getsentry/sentry-docs#5469

@sl0thentr0py sl0thentr0py self-assigned this Oct 11, 2022
@antonpirker
Copy link
Member

Hey @Agalin and @polina-popova !
Sorry for the delay! And thank you soo much for the great work!
I will have a look at this today, and I will also be the person to help bring this into a release sooner than later :-)

Could you maybe rebase your branch again on the newest master, so everything is up to date? Thanks!

@Agalin
Copy link
Contributor Author

Agalin commented Oct 27, 2022

Awesome. Rebased to master (clean rebase as the only existing file is tox.ini)

Copy link
Member

@antonpirker antonpirker left a comment

Choose a reason for hiding this comment

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

Hey!

Extraordinary work!
Just one change to make sure the personal data of users is save and one change to make it easier for Sentry to identify performance problems with queries.

sentry_sdk/integrations/pymongo.py Show resolved Hide resolved
sentry_sdk/integrations/pymongo.py Outdated Show resolved Hide resolved
sentry_sdk/integrations/pymongo.py Outdated Show resolved Hide resolved
@antonpirker
Copy link
Member

Oh, and please fix also the linting errors. Thanks!

@antonpirker
Copy link
Member

I have now added a new function for nicer PII stripping, could you review my PR @Agalin : operasoftware#1
Thank you very much!

@antonpirker
Copy link
Member

I have to stay home to take care of my daughter today (some childrens sickness) Could you fix the tests please @Agalin ? I think when the tests are green we are ready to merge.

@Agalin
Copy link
Contributor Author

Agalin commented Nov 3, 2022

@antonpirker tests now pass in tox on my machine (tested 2.7 with pymongo 3.1 and 3.8 with 4.2).

@antonpirker
Copy link
Member

Amazing @Agalin ! Thank you so much again for your great work!

@antonpirker antonpirker enabled auto-merge (squash) November 4, 2022 09:05
@antonpirker
Copy link
Member

Hey @Agalin if you now rebase on the new master again, it will be merged automatically (after running the tests once again)

Szymon Soloch and others added 8 commits November 4, 2022 10:11
Adds breadcrumbs and performance traces for PyMongo queries using an
official monitoring API. Integration is similar to the one available in
OpenTelemetry, tags set to values recommended for attributes by OT as
specified in `Span Operations` guidelines.

PyMongo version selection explanation:
* 3.1 - introduction of monitoring API. Only Python 2.7 and 3.6
supported.
* 3.12 - latest 3.x release, support for 2.7, 3.6-3.9 (3.7-3.9 added in
various minor releases between 3.1 and 3.12).
* 4.0 - no support for 2.7, added support for 3.10.
* 4.1 - no support for 3.6.0-3.6.1.
* 4.2 - no support for any 3.6.
auto-merge was automatically disabled November 4, 2022 09:11

Head branch was pushed to by a user without write access

@Agalin
Copy link
Contributor Author

Agalin commented Nov 4, 2022

Ah. My rebase stopped automerge. 😓 @antonpirker can you reapprove?

@antonpirker antonpirker enabled auto-merge (squash) November 4, 2022 10:01
@antonpirker antonpirker merged commit fa1b964 into getsentry:master Nov 4, 2022
@antonpirker
Copy link
Member

Done! (now for the documentation)

@Agalin Agalin deleted the pymongo branch November 4, 2022 11:23
@antonpirker
Copy link
Member

Hey everyone!

This will be released today!

Again: thank you so much for your contribution, it is really great work!
@Agalin If you send me your shipping address to anton.pirker@sentry.io I can send you a little "thank you" gift.

@Agalin
Copy link
Contributor Author

Agalin commented Nov 14, 2022

Yay, it's happening!
Should I add an entry in the main Sentry codebase that adds integration recommendation? It couldn't be done earlier as version number is required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Integration Integrating with a new framework or library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pymongo integration
5 participants