Skip to content

Reduce the noise from loggers #8833

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

Merged
merged 3 commits into from
Jun 2, 2025
Merged

Reduce the noise from loggers #8833

merged 3 commits into from
Jun 2, 2025

Conversation

tofarr
Copy link
Collaborator

@tofarr tofarr commented Jun 1, 2025

  • This change is worth documenting at https://docs.all-hands.dev/
  • Include this change in the Release Notes. If checked, you must provide an end-user friendly description for your change below

End-user friendly description of the problem this fixes or functionality this introduces.

Reduces log noise by setting verbose loggers to WARNING level, improving the readability of logs and making it easier to spot important messages.


Summarize what the PR does, explaining any non-trivial design decisions.

This PR improves the logging system by reducing noise from various verbose loggers:

  1. Expands the list of loggers that are set to WARNING level beyond just LiteLLM
  2. Adds engineio, socketio and their related loggers to the list of "loquacious loggers"
  3. Replaces individual logger disabling with a more maintainable approach using a list and loop
  4. Sets loggers to WARNING level instead of completely disabling them, ensuring important warnings and errors are still captured

This change makes the logs more readable and focused on important information while still preserving critical messages from all components.


Link of any specific issues this addresses:


To run this PR locally, use the following command:

docker run -it --rm   -p 3000:3000   -v /var/run/docker.sock:/var/run/docker.sock   --add-host host.docker.internal:host-gateway   -e SANDBOX_RUNTIME_CONTAINER_IMAGE=docker.all-hands.dev/all-hands-ai/runtime:4a30e0d-nikolaik   --name openhands-app-4a30e0d   docker.all-hands.dev/all-hands-ai/openhands:4a30e0d

@tofarr tofarr marked this pull request as ready for review June 1, 2025 14:10
]

for logger_name in LOQUACIOUS_LOGGERS:
logging.getLogger(logger_name).setLevel('WARNING')
Copy link
Collaborator

@enyst enyst Jun 1, 2025

Choose a reason for hiding this comment

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

Agree, this is a good idea in general, but FYI litellm was disabled because it can leak API keys in logs, not only because it's a lot of output.

Edited to add: on errors, IIRC. So I think it will (if it's still doing it) on warning too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you - I was not aware of that - I have fully disabled the LiteLLM loggers again and added a note explaining why they were disabled

@tofarr tofarr requested a review from enyst June 2, 2025 14:11
Copy link
Collaborator

@enyst enyst left a comment

Choose a reason for hiding this comment

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

🍰

@tofarr tofarr merged commit 3f41202 into main Jun 2, 2025
20 checks passed
@tofarr tofarr deleted the fix-loquacious-loggers branch June 2, 2025 14:34
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.

2 participants