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

opentelemetry.ext.http_requests should allow de-registering a specific Session #479

Closed
lizthegrey opened this issue Mar 12, 2020 · 5 comments · Fixed by #573
Closed

opentelemetry.ext.http_requests should allow de-registering a specific Session #479

lizthegrey opened this issue Mar 12, 2020 · 5 comments · Fixed by #573

Comments

@lizthegrey
Copy link
Member

Is your feature request related to a problem?
See workaround implemented https://github.com/honeycombio/opentelemetry-exporter-python/blob/master/src/opentelemetry/ext/honeycomb/__init__.py#L53 that should be made into a library function of http_requests

Describe the solution you'd like
In addition to disable() that disables system-wide, it should be possible to de-register a specific instantiated Session object without all the boilerplate.

Describe alternatives you've considered
We also could allow making disable() taking a module or class name that the wrapper would check against before deciding whether to instrument or pass through directly.

@c24t
Copy link
Member

c24t commented Mar 12, 2020

Good call, any instrumentation code that uses requests under the hood will need this, and plenty of reasons for application code too.

Looking forward to the honeycomb exporter!

@lizthegrey
Copy link
Member Author

@mauriciovasquezbernal
Copy link
Member

@lizthegrey hi Liz. I'm curious, why did you have to de-register a session? I suppose it was not because of double instrumentation, the current implementation of the exporter logic disables tracing when exporting spans. Is there a performance reason?

@lizthegrey
Copy link
Member Author

lizthegrey commented Apr 13, 2020

@lizthegrey hi Liz. I'm curious, why did you have to de-register a session? I suppose it was not because of double instrumentation, the current implementation of the exporter logic disables tracing when exporting spans. Is there a performance reason?

We found that the exporter logic did not disable tracing for us while our exporter was running, causing our request objects to improperly be traced.

@mauriciovasquezbernal
Copy link
Member

Could you confirm this behavior with the latest version?, I implemented a test to check this feature and it's passing on #573.

srikanthccv pushed a commit to srikanthccv/opentelemetry-python that referenced this issue Nov 1, 2020
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 a pull request may close this issue.

3 participants