-
-
Notifications
You must be signed in to change notification settings - Fork 8.5k
[grid] Add Node session-history endpoint and write to local file for other utility to consume #15879
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
base: trunk
Are you sure you want to change the base?
Conversation
…other utility to consume
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Latest suggestions up to b182d49
Previous suggestions✅ Suggestions up to commit 9436e06
✅ Suggestions up to commit 830b966
|
Co-authored-by: qodo-merge-pro[bot] <151058649+qodo-merge-pro[bot]@users.noreply.github.com>
Co-authored-by: qodo-merge-pro[bot] <151058649+qodo-merge-pro[bot]@users.noreply.github.com>
CI Feedback 🧐(Feedback updated until commit 0b19339)A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
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.
I don't understand the goal of the PR.
I've also thought about keeping a history of the sessions, but my thought was to add an extra column with the session status to the current structure, and retrieve them through a query parameters through the same endpoint.
User description
🔗 Related Issues
💥 What does this PR do?
/se/grid/node/session-history
to access all sessions belong to that Node (include sessionId, startTime, stopTime)curl http://localhost:5555/se/grid/node/session-history --header 'X-REGISTRATION-SECRET;'
--status-to-file
to write Node status JSON to the given location (for internal utility consumption), it is triggered together with eventNodeHeartBeatEvent
--session-history-to-file
to write Node session history JSON to the given location (for internal utility consumption), it is triggered when eventsSessionStartedEvent
andSessionStopedEvent
fire.🔧 Implementation Notes
Other utility running sidecare with Node (e.g, video recording), is doing a query to the Node endpoint (/status) in a very short interval (e.g, 2s) to catch up sessions frequently.
Instead of access via remote endpoint, provide another option to write local file (since those processes are running in the same host, others can consume the resource), and the output is updated by event-driven (it's more effective than a static interval hitting the status endpoint).
💡 Additional Considerations
🔄 Types of changes
PR Type
Enhancement, Tests
Description
Add Node endpoint
/se/grid/node/session-history
for session trackingImplement Node config options to write status and session history to file
Track session start/stop events and persist to file
Add comprehensive unit tests for new features
Changes walkthrough 📝
11 files
Add SessionHistoryEntry for session tracking metadata
Introduce SessionStartedEvent for session lifecycle
Add SessionStatus enum for session outcome
Add HTTP handler for session-history endpoint
Add getSessionHistory() and session-history endpoint
Add CLI/config flags for status and session history files
Add accessors for status and session history file config
Implement file writing and session history tracking in LocalNode
Pass status/session history file config to LocalNode
Fire SessionStartedEvent on session creation
Implement getSessionHistory() for remote nodes
4 files
Add tests for SessionClosedEvent and SessionStatus
Add tests for session-history endpoint handler
Add tests for NodeOptions status/session history config
Add tests for file writing and session history in LocalNode