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 #7105: Expose metrics directly on conversation rather than inside events #7107

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

Conversation

enyst
Copy link
Collaborator

@enyst enyst commented Mar 4, 2025

This PR fixes issue #7105 by implementing the reviewer suggestion to expose metrics directly from the conversation/state rather than reconstructing from events.

Changes:

  • Added get_metrics() method to Conversation class that retrieves metrics directly from runtime.state.metrics
  • Updated get_conversation_metrics() endpoint to try runtime state metrics first, falling back to event_stream.get_metrics()
  • Added fallback mechanism for backward compatibility
  • Updated tests to verify both direct state metrics and fallback to event_stream metrics

Tests:

  • Added test for fallback behavior
  • Updated existing tests to verify metrics are retrieved from runtime state first
  • All tests pass with the new implementation

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

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