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/requests - crashes when used with socket-io #642

Closed
nirsky opened this issue May 3, 2020 · 3 comments
Closed

ext/requests - crashes when used with socket-io #642

nirsky opened this issue May 3, 2020 · 3 comments
Labels
backlog bug Something isn't working instrumentation Related to the instrumentation of third party libraries or frameworks

Comments

@nirsky
Copy link
Contributor

nirsky commented May 3, 2020

Describe your environment
Python 3.8.2
opentelemetry-sdk==0.6b0
opentelemetry-ext-http-requests==0.6b0 (was able to reproduce on 0.7.dev0 as well)
requests==2.23.0
python-socketio==4.5.1
python-socketio-client==1.1
websocket-client==0.57.0

Steps to reproduce
After enabling requests ext, like this:

http_requests.enable(trace_provider)

when using socketio client and connecting as follow:

import socketio

sio = socketio.Client()
sio.connect("socketio-server-url")

From this line of code (propagators.inject(type(headers).__setitem__, headers)), the following errors is thrown:
AttributeError("type object 'NoneType' has no attribute '__setitem__'")

This happens because for some reason the headers is None.
A possible fix will be adding if headers is not None: before calling the propagators.inject.

What is the expected behavior?
Expected to see the trace being sent.

What is the actual behavior?
app crashed.

Additional context
Happened on both 0.7.dev0 and 0.6b0 versions of the ext.

@nirsky nirsky added the bug Something isn't working label May 3, 2020
@Oberon00
Copy link
Member

Oberon00 commented May 3, 2020

Technically, this looks like a bug in socket-io to me because headers is supposed to be a dict, not None (see https://github.com/python/typeshed/blob/545be37c4035d0df1a64fdeab08dbaec0f294bec/stdlib/3/urllib/request.pyi#L47 and https://docs.python.org/3/library/urllib.request.html#urllib.request.Request).
I guess we should both protect against None in ext/requests and file an issue at socket-io.

@mauriciovasquezbernal mauriciovasquezbernal added ext instrumentation Related to the instrumentation of third party libraries or frameworks labels May 4, 2020
srikanthccv pushed a commit to srikanthccv/opentelemetry-python that referenced this issue Nov 1, 2020
test: fix and add tests
closes open-telemetry#642

Signed-off-by: Olivier Albertini <olivier.albertini@montreal.ca>
@lzchen lzchen removed the ext label Feb 18, 2021
@github-actions
Copy link

This issue was marked stale due to lack of activity. It will be closed in 30 days.

@codeboten
Copy link
Contributor

This was fixed in the requests instrumentation library.

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

No branches or pull requests

5 participants