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

Access logs for mounted apps doesnt not contain the root path #1384

Closed
euri10 opened this issue Feb 21, 2022 · 4 comments · Fixed by #1385
Closed

Access logs for mounted apps doesnt not contain the root path #1384

euri10 opened this issue Feb 21, 2022 · 4 comments · Fixed by #1385

Comments

@euri10
Copy link
Member

euri10 commented Feb 21, 2022

This change broke our monitoring a bit. We use FastAPI and mount a ASGI-app to /graphql route.
Since access logs are not really useful for GraphQL we filter them out in our monitoring to avoid a lot of noise.
Since this change access logs are logging / instead of /graphql which breaks this and makes our logs very noisy again.

Originally posted by @levrik in #1294 (comment)

@euri10
Copy link
Member Author

euri10 commented Feb 21, 2022

thanks for that @levrik

we should maybe use raw_path instead of path in

def get_path_with_query_string(scope: WWWScope) -> str:
    path_with_query_string = urllib.parse.quote(scope["path"])

@levrik
Copy link
Contributor

levrik commented Feb 21, 2022

Thanks a lot for transitioning this into an issue. I'll give your suggestion a try and open a PR.

@levrik
Copy link
Contributor

levrik commented Feb 22, 2022

I've opened a PR (#1385) which switches to raw_path in logging. We've tested this with our application which successfully brought the log filtering back to life.

@Kludex
Copy link
Sponsor Member

Kludex commented Sep 11, 2022

Same issue as pgjones/hypercorn#75.

This is an issue on Starlette's side: encode/starlette#1336.

@Kludex Kludex closed this as completed Sep 11, 2022
mxsasha added a commit to irrdnet/irrd that referenced this issue Aug 22, 2023
mxsasha added a commit to irrdnet/irrd that referenced this issue Aug 22, 2023
mxsasha added a commit to irrdnet/irrd that referenced this issue Aug 22, 2023
mxsasha added a commit to irrdnet/irrd that referenced this issue Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants