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

Tweak log levels #4729

Merged
merged 6 commits into from
Nov 11, 2024
Merged

Tweak log levels #4729

merged 6 commits into from
Nov 11, 2024

Conversation

enyst
Copy link
Collaborator

@enyst enyst commented Nov 4, 2024

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

  • Include this change in the Release Notes. If checked, you must provide an end-user friendly description for your change below
    Restore visible logs for each action and observation when ran locally.

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

This PR proposes a few tweaks for log levels:

  • display at info the action and the obs (ref slack) when LOG_ALL_EVENTS env var is set
  • send to debug channel most of logging in github.py since that is not active in most use cases and on local it sounds like something's wrong.

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

@enyst enyst requested a review from rbren November 4, 2024 10:46
Copy link
Collaborator

@tofarr tofarr left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@rbren rbren left a comment

Choose a reason for hiding this comment

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

I explicitly do not want actions and observations in the info logs--will follow up

@rbren
Copy link
Collaborator

rbren commented Nov 4, 2024

@enyst we need to keep actions and observations out of the default info logs, because they can contain (a) a LOT of information, and (b) private information

I'd support having an env var that swaps them from debug into info though, like LOG_ALL_EVENTS

@enyst
Copy link
Collaborator Author

enyst commented Nov 4, 2024

@enyst we need to keep actions and observations out of the default info logs, because they can contain (a) a LOT of information, and (b) private information

Okay. Oh, this is an issue on the hosted version, I suppose? On local it looks really strange to not tell the user anything.

Env var sounds good.

@rbren
Copy link
Collaborator

rbren commented Nov 4, 2024

Yeah I'd say it's an issue in any kind of multi-tenant environment.

Also, ideally, the UI would give you all this information, and the server logs should really just be there to know "is the server functioning properly." I think right now it's kind of a hacky way to gain the level of agent observability we want in the UI but don't have.

@enyst
Copy link
Collaborator Author

enyst commented Nov 10, 2024

On further thought, we already have other env vars changing the logging behavior, like LOG_TO_FILE and SKIP_CONTAINER_LOGS somewhere, adding another is not helping predictability. FWIW maybe I didn't lose track yet, but it's going to happen. 😅

Does this work though as a temporary measure? I think we need to think this over.

@enyst enyst requested a review from rbren November 10, 2024 13:37
Copy link
Collaborator

@rbren rbren left a comment

Choose a reason for hiding this comment

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

Unblocking but left one comment!

@enyst enyst enabled auto-merge (squash) November 11, 2024 22:25
@enyst enyst merged commit a45aba5 into main Nov 11, 2024
14 checks passed
@enyst enyst deleted the enyst/log-tweak branch November 11, 2024 22:51
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.

3 participants