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

Fix issue #7147: Document Logging #7737

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

openhands-agent
Copy link
Contributor

@openhands-agent openhands-agent commented Apr 7, 2025

This pull request fixes #7147.

The issue has been successfully resolved. The changes directly address all requirements from the original issue by:

  1. Creating a comprehensive new documentation file docs/modules/usage/logging.md that covers all requested environment variables:
  • LOG_JSON
  • LOG_JSON_LEVEL_KEY
  • DEBUG
  • DEBUG_LLM
  • DEBUG_RUNTIME
  • LOG_ALL_EVENTS
  1. Including detailed documentation about the SensitiveDataFilter functionality, which was specifically requested in the issue.

  2. Adding the logging documentation to the sidebar navigation in sidebars.ts, making it accessible within the documentation structure.

The documentation goes beyond the minimum requirements by providing:

  • Detailed explanations of each logging feature
  • Log file structure information
  • Security considerations
  • Color coding information
  • Best practices for different environments

The changes provide a complete, well-structured documentation of OpenHands' logging system, fully addressing the original request for documentation of the logging behavior and related concepts.

Automatic fix generated by OpenHands 🙌


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:1d9429d-nikolaik   --name openhands-app-1d9429d   docker.all-hands.dev/all-hands-ai/openhands:1d9429d


## Security Features

### SensitiveDataFilter
Copy link
Collaborator

Choose a reason for hiding this comment

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

@openhands-agent Use the GitHub API to read the diff of this PR, PR 7737, and the issue it fixes.

The PR contents look correct, however SensitiveDataFilter is an older implementation that masks sensitive values in logs. Recently, we have implemented another way, using set_secrets() in stream.py. Read it, follow its use to understand it.

Then update this documentation with your conclusions. Does it fully replace this one? How can users make use of that one, if there's any way?

Keep it clear and to the point. No bragging or fluff, this is technical documentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Openhands fix success summary

The changes successfully address the feedback by making several concrete improvements to the logging documentation:

  1. Added comprehensive documentation about the newer set_secrets() implementation from stream.py, including:

    • Clear example code showing how to use it
    • Explanation of how it replaces exact matches with <secret_hidden>
    • Description of its more precise control compared to pattern matching
  2. Restructured the sensitive data section to show both approaches:

    • Renamed section to "Sensitive Data Protection" to encompass both methods
    • Clearly separated and labeled both approaches
    • Maintained the existing SensitiveDataFilter documentation while adding the new content
  3. Added practical guidance on when to use each approach:

    • SensitiveDataFilter for general protection
    • set_secrets() for precise control over specific values
  4. Kept the documentation technical and concise, avoiding unnecessary language while providing the essential information users need to understand and implement either approach.

The changes directly fulfill the feedback's request to document both implementations while explaining their respective uses, maintaining technical accuracy and clarity throughout.

@openhands-agent
Copy link
Contributor Author

OpenHands started fixing the pr! You can monitor the progress here.

@openhands-agent
Copy link
Contributor Author

Based on the provided changes, it appears that OpenHands has successfully addressed all the major documentation issues regarding sensitive data handling. Here's a concise assessment:

✅ Successfully Resolved:

  • Comprehensive documentation of both set_secrets() and SensitiveDataFilter
  • Clear separation and explanation of both approaches
  • Practical usage guidance with examples
  • Technical accuracy maintained
  • Concise and focused content

🔍 No apparent remaining issues to address.

The changes appear to provide a complete solution that effectively documents both implementations while maintaining clarity and technical accuracy. Users now have clear guidance on when and how to use each approach for sensitive data protection.

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.

Document Logging
2 participants