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

FastAPI: Conditionally create SERVER spans #451

Closed
owais opened this issue Apr 15, 2021 · 8 comments
Closed

FastAPI: Conditionally create SERVER spans #451

owais opened this issue Apr 15, 2021 · 8 comments
Assignees

Comments

@owais
Copy link
Contributor

owais commented Apr 15, 2021

Part of #445

@owais owais added feature-request good first issue Good for newcomers help wanted Extra attention is needed instrumentation labels Apr 15, 2021
@owais owais removed the good first issue Good for newcomers label Oct 3, 2021
@sanketmehta28
Copy link
Member

Hi @owais : Can you please assign it to me?

@sanketmehta28
Copy link
Member

I can see that we already have resolved the same type of issue for ASGI in PR
As fastapi internally uses asgi middleware, it automatically resolves the same for FastAPI. (unittest is the proof)
So should we close this issue?

@owais
Copy link
Contributor Author

owais commented Jan 27, 2022

Can we add a test to FastAPI package to verify the behaviour so we catch any breakage in future?

@sanketmehta28
Copy link
Member

Test case has already been added as a part of the PR mentioned in above comment

@owais
Copy link
Contributor Author

owais commented Jan 27, 2022

Oh nice! I thought it would contain test for ASGI package. Sounds good. I'll close this one but may be we should add a test case to ASGI package as well?

@owais owais closed this as completed Jan 27, 2022
@sanketmehta28
Copy link
Member

sanketmehta28 commented Jan 27, 2022

Make sense. can you reopen the ASGI issue #455 so that I will raise a PR for the same to add a test case?

@owais
Copy link
Contributor Author

owais commented Jan 27, 2022

The issue has been resolved already. You can open a new issue or just submit a PR without an issue if you'd like.

@sanketmehta28
Copy link
Member

sanketmehta28 commented Feb 3, 2022

@owais : Here is the PR for ASGI unit test: PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants