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

Make uvicorn logs be JSON. Refactor json_logging config #60

Merged
merged 1 commit into from
Apr 21, 2022

Conversation

pawbu
Copy link
Collaborator

@pawbu pawbu commented Apr 21, 2022

As noted here logging is quite complicated with uvicorn.
I followed the advice here and that worked nicely.

@pawbu pawbu requested a review from a team April 21, 2022 12:05
Copy link
Contributor

@vaskalan vaskalan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! :-)

@pawbu pawbu merged commit 1e07c8e into master Apr 21, 2022
@pawbu pawbu deleted the jsonify-uvicorn-logs branch April 21, 2022 13:05
@FISHMANPET
Copy link

I stumbled on this, since you referenced this issue here and decided to shamelessly copy it. One problem I noticed, in your log_config.yaml, the closing quote in your format strings is off, it should be this: "message": "%(message)s"}. Inside the bracket, to close off the message value, rather than outside the bracket.

However, that still doesn't generate "valid" JSON, specifically in the access logs. An access event message will look like this:
127.0.0.1:59506 - "GET /openapi.json HTTP/1.1" 200 OK including the quotes, so when that gets logged unescaped it becomes invalid JSON. I don't have an answer for that problem yet, but at least thought you should be aware.

@pawbu
Copy link
Collaborator Author

pawbu commented Aug 24, 2022

@FISHMANPET Good catch, thanks. We'll take a look at this issue.

@pawbu
Copy link
Collaborator Author

pawbu commented Aug 25, 2022

@FISHMANPET The fastapi-request-logger from FastAPI is enough for us right now so I removed log_config.yaml from the project.
encode/uvicorn#680 (comment) looks promising though.

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 this pull request may close these issues.

None yet

3 participants