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

ext/pymongo: Add instrumentor interface #598

Closed

Conversation

codeboten
Copy link
Contributor

@codeboten codeboten commented Apr 18, 2020

Mauricio: I took over this one.

This PR implements the Instrumentor interface for the Pymongo integration. It also implements a way to disable instrumentation.

I compared with the DataDog donated implementation available at https://github.com/open-telemetry/opentelemetry-python-contrib/tree/master/reference/ddtrace/contrib/pymongo, the already existing implementation is good enough and we do not need to take any from that in this case.

This PR conflicts with #602, I'll keep as draft until that one is merged.

@codeboten codeboten added WIP Work in progress instrumentation Related to the instrumentation of third party libraries or frameworks labels Apr 18, 2020
@mauriciovasquezbernal mauriciovasquezbernal changed the title [WIP] add instrumentor interface to pymongo ext/pymongo: aAdd instrumentor interface Apr 21, 2020
@mauriciovasquezbernal mauriciovasquezbernal changed the title ext/pymongo: aAdd instrumentor interface ext/pymongo: Add instrumentor interface Apr 21, 2020
@mauriciovasquezbernal mauriciovasquezbernal force-pushed the instrumentation-pymongo branch 2 times, most recently from 62f28eb to 68fb7c5 Compare April 22, 2020 15:01
Alex Boten and others added 3 commits April 23, 2020 07:26
The current Pymongo integration uses the monitoring.register() [1] to hook
the different internal calls of Pymongo. This integration doesn't allow to
unregister a monitor. This commit workaround that limitation by adding an
enable flag to the CommandTracer class and adds a logic to disable the
integration.  This solution is not perfect becasue there will be some overhead
even when the instrumentation is disabled, but that's what we can do with
the current approach.

[1] https://api.mongodb.com/python/current/api/pymongo/monitoring.html#pymongo.monitoring.register
Pop span instead of getting and then removing it.
@mauriciovasquezbernal mauriciovasquezbernal removed the WIP Work in progress label Apr 23, 2020
@mauriciovasquezbernal mauriciovasquezbernal marked this pull request as ready for review April 23, 2020 14:13
@mauriciovasquezbernal mauriciovasquezbernal requested a review from a team April 23, 2020 14:13
@mauriciovasquezbernal
Copy link
Member

To make the review process more simple, i.e, avoiding me to review my own changes I'm opening a new PR #612.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
instrumentation Related to the instrumentation of third party libraries or frameworks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants