Skip to content

Include metadata like session_id in logs #7145

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 4 commits into from
Mar 7, 2025

Conversation

raymyers
Copy link
Contributor

@raymyers raymyers commented Mar 7, 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 that this introduces.


Give a summary of what the PR does, explaining any non-trivial design decisions.

The goal is to improve observability and be able to answer questions like "what happened in this session" or "how many sessions are experiences errors".

This PR includes add metadata in logging when available, such as session_id and user_id. These will show up as fields in json logging mode, added in #7034.

Naming of Session ID: These logs using the field session_id, but other places in the codebase call it conversation_id or sid, I'm open to whichever we want to standardize on.

Deprecating MonitoringListener: This replaces use of MonitoringListener (added recently in #6929) with logs that populate the signal extra field with these values.

  • signal: agent_session_start
  • signal: create_conversation
  • signal: agent_status_error

If this is successful, a future change can likely remove the MonitoringListener abstraction completely.

LoggerAdapter: In Session and AgentSession, the LoggerAdapter pattern was used to add metadata at the class level instead of each log statement. See Logging Cookbook in the official Python docs. We want to include both common fields (like session_id) and additional fields in the extra arg of a single log statement, so LoggerAdapter is subclassed to do the same as Python 3.13's merge_extra option will provide.


Link of any specific issues this addresses.

@raymyers raymyers changed the title Include metadata like session_id logs Include metadata like session_id in logs Mar 7, 2025
@raymyers
Copy link
Contributor Author

raymyers commented Mar 7, 2025

Related: Discussed with @mamoodi that the LOG_JSON mode is probably with documenting so I'm going to make a separate PR adding a logging.md to developer docs.

@raymyers
Copy link
Contributor Author

raymyers commented Mar 7, 2025

Related: Discussed with @mamoodi that the LOG_JSON mode is probably with documenting so I'm going to make a separate PR adding a logging.md to developer docs.

Created #7147, could assign to me if you like.

@raymyers
Copy link
Contributor Author

raymyers commented Mar 7, 2025

This library does a similar thing to the adapter and extra fields formatter in this PR, but I'm not quite convinced that it's worth pulling in a library for.

https://terseus.github.io/python-logging-with-context/

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.

LGTM!

@enyst
Copy link
Collaborator

enyst commented Mar 7, 2025

If you're happy with it, we can take this in?

@raymyers
Copy link
Contributor Author

raymyers commented Mar 7, 2025

If you're happy with it, we can take this in?

@enyst Yep, I think this one is good to go thanks!

@enyst enyst merged commit dc9489d into All-Hands-AI:main Mar 7, 2025
14 checks passed
adityasoni9998 pushed a commit to adityasoni9998/OpenHands that referenced this pull request Mar 12, 2025
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