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

Duplication in API logging #3237

Closed
obulat opened this issue Oct 23, 2023 · 1 comment · Fixed by #3617
Closed

Duplication in API logging #3237

obulat opened this issue Oct 23, 2023 · 1 comment · Fixed by #3617
Assignees
Labels
🤖 aspect: dx Concerns developers' experience with the codebase 🧰 goal: internal improvement Improvement that benefits maintainers, not users good first issue New-contributor friendly help wanted Open to participation from the community 🟩 priority: low Low priority and doesn't need to be rushed 🧱 stack: api Related to the Django API 🐍 tech: python Involves Python

Comments

@obulat
Copy link
Contributor

obulat commented Oct 23, 2023

Problem

In the django container logs, we also have two very similar logs for one request. The main difference I see between them is the lack of request id in one of them, so I'm not sure if it's possible to combine them:

| [2023-10-03 15:15:09,480 - gunicorn.access - 363][INFO] 127.0.0.1 - - [03/Oct/2023:15:15:09 +0000] "GET /v1/audio/2ecd1fbf-8122-47e8-84db-50c8ce19c04a/ HTTP/1.0" 200 1682 "-" "Openverse/0.1 (https://openverse.org; openverse@wordpress.org)" | task_id | django

| [2023-10-03 15:15:09,479 - log_request_id.middleware - 55][INFO] <task_id> method=GET path=/v1/audio/2ecd1fbf-8122-47e8-84db-50c8ce19c04a/ status=200

We should probably silence the gunicorn access log because it logs the same information as the log_request_id.middleware, but it doesn't have access to the request id.

Note

This issue was written when we still used Gunicorn. We use uvicorn now. The description is up-to-date for our current stack.

Description

Disable uvicorn's access logging by setting no_access_log=True in api/run.py in the call to start uvicorn.

@obulat obulat added 🟩 priority: low Low priority and doesn't need to be rushed 💬 talk: discussion Open for discussions and feedback 🧰 goal: internal improvement Improvement that benefits maintainers, not users 🧱 stack: api Related to the Django API labels Oct 23, 2023
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Openverse Backlog Oct 23, 2023
@sarayourfriend sarayourfriend added 🤖 aspect: dx Concerns developers' experience with the codebase and removed 💬 talk: discussion Open for discussions and feedback labels Dec 1, 2023
@sarayourfriend
Copy link
Collaborator

This was solved in production by setting the log level higher than INFO. We've dropped it back down to info, so to solve this, we need some small configuration changes. Namely, we just need to disable uvicorn's request logging: https://www.uvicorn.org/settings/#logging

I'll update the issue description with instructions.

@sarayourfriend sarayourfriend added good first issue New-contributor friendly help wanted Open to participation from the community 🐍 tech: python Involves Python and removed good first issue New-contributor friendly help wanted Open to participation from the community 🐍 tech: python Involves Python labels Dec 1, 2023
@ngken0995 ngken0995 self-assigned this Jan 2, 2024
@openverse-bot openverse-bot moved this from 📋 Backlog to 📅 To Do in Openverse Backlog Jan 2, 2024
@openverse-bot openverse-bot moved this from 📅 To Do to 🏗 In Progress in Openverse Backlog Jan 2, 2024
@openverse-bot openverse-bot moved this from 🏗 In Progress to ✅ Done in Openverse Backlog Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖 aspect: dx Concerns developers' experience with the codebase 🧰 goal: internal improvement Improvement that benefits maintainers, not users good first issue New-contributor friendly help wanted Open to participation from the community 🟩 priority: low Low priority and doesn't need to be rushed 🧱 stack: api Related to the Django API 🐍 tech: python Involves Python
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants