-
Notifications
You must be signed in to change notification settings - Fork 7k
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
Conversation
Related: Discussed with @mamoodi that the LOG_JSON mode is probably with documenting so I'm going to make a separate PR adding a |
openhands/server/conversation_manager/standalone_conversation_manager.py
Show resolved
Hide resolved
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. |
3101686
to
9b3a6b1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
If you're happy with it, we can take this in? |
@enyst Yep, I think this one is good to go thanks! |
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 itconversation_id
orsid
, 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.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'smerge_extra
option will provide.Link of any specific issues this addresses.