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

HTTPXClientInstrumentor mixes async and non async hooks #1201

Closed
alec-deason opened this issue Jul 22, 2022 · 3 comments · Fixed by #1920
Closed

HTTPXClientInstrumentor mixes async and non async hooks #1201

alec-deason opened this issue Jul 22, 2022 · 3 comments · Fixed by #1920
Labels
bug Something isn't working help wanted Extra attention is needed instrumentation

Comments

@alec-deason
Copy link

Describe your environment
Python 3.10.3
opentelemetry-api==1.12.0rc2
opentelemetry-sdk==1.12.0rc2
opentelemetry-instrumentation-httpx==0.32b0

Steps to reproduce
Run this example:

 from opentelemetry.instrumentation.httpx import HTTPXClientInstrumentor
 import httpx, asyncio

 def httpx_request_hook(span, request):
     print("test")

 HTTPXClientInstrumentor().instrument(request_hook=httpx_request_hook)

 # This succeeds
 with httpx.Client() as client:
     client.get("http://example.com")

 async def test_func():
     async with httpx.AsyncClient() as client:
         await client.get("http://example.com")

 # this fails with `TypeError: object NoneType can't be used in 'await' expression`
 asyncio.run(test_func())

What is the expected behavior?
After instrumentation it should be possible to successfully use httpx in both sync and async contexts.

What is the actual behavior?
If a non-async hook is provided then an exception is raised when the AsyncClient is used. If an async hook is provided then it never executes when the non-async Client is used.

Additional context
It seems to me this should either allow both async and sync hooks to be provided separately. Or it should require that the hook be non-blocking and only support non-async hooks.

@alec-deason alec-deason added the bug Something isn't working label Jul 22, 2022
@alec-deason
Copy link
Author

I'm willing to write up a PR with either of the suggestions in "Additional context" or some other option if someone closer to the project wants to weigh in on what the preferred approach would be.

@srikanthccv
Copy link
Member

srikanthccv commented Jul 22, 2022

+1 to former suggestion.

@srikanthccv srikanthccv added help wanted Extra attention is needed instrumentation labels Sep 9, 2022
@srikanthccv
Copy link
Member

@alec-deason send a PR for the fix :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed instrumentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants