-
Notifications
You must be signed in to change notification settings - Fork 160
Add core health monitoring infrastructure for vmcp backends #3100
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
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.
Large PR Detected
This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.
How to unblock this PR:
Add a section to your PR description with the following format:
## Large PR Justification
[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformationAlternative:
Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.
See our Contributing Guidelines for more details.
This review will be automatically dismissed once you add the justification section.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3100 +/- ##
==========================================
+ Coverage 56.89% 57.09% +0.19%
==========================================
Files 337 341 +4
Lines 33617 33940 +323
==========================================
+ Hits 19127 19378 +251
- Misses 12899 12959 +60
- Partials 1591 1603 +12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Pull request overview
This PR implements core health monitoring infrastructure for virtual MCP (Model Context Protocol) server backends. It provides a foundation for tracking backend availability, detecting failure patterns, and categorizing different types of failures (unhealthy, degraded, unauthenticated).
Key changes:
- Health checker implementation using ListCapabilities as the health check mechanism with error categorization
- Status tracker with threshold-based failure detection and thread-safe state management
- Periodic health monitor with configurable intervals, automatic failure tracking, and graceful lifecycle management
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/vmcp/health/checker.go | Implements health checking using ListCapabilities with error categorization (auth, timeout, connection failures) |
| pkg/vmcp/health/checker_test.go | Comprehensive tests for health checker including error categorization, timeouts, and concurrent backend scenarios |
| pkg/vmcp/health/status.go | Status tracker for managing backend health states with threshold-based transitions and thread-safe operations |
| pkg/vmcp/health/status_test.go | Extensive tests for status tracking including concurrency, state transitions, and edge cases |
| pkg/vmcp/health/monitor.go | Periodic health monitor with background goroutines, configurable intervals, and lifecycle management |
| pkg/vmcp/health/monitor_test.go | Monitor tests covering start/stop lifecycle, periodic checks, context cancellation, and health summaries |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Implement health checking and status tracking for virtual MCP server backends. This provides the foundation for monitoring backend availability and categorizing failure modes (unhealthy, degraded, unauthenticated). Related-to: #3036
097c9d9 to
175b685
Compare
|
✅ Large PR justification has been provided. The size review has been dismissed and this PR can now proceed with normal review. |
Large PR justification has been provided. Thank you!
11b93a8 to
a0c7244
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.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jhrozek
left a comment
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.
Left a few observations from reviewing this PR. Nothing blocking, just some things to consider.
Implement health checking and status tracking for virtual MCP server backends. This provides the foundation for monitoring backend availability and categorizing failure modes (unhealthy, degraded, unauthenticated).
Large PR Justification
This is an atomic commit, adding the core functionality for vmcp healthchecks. It includes comprehensive testing
Related-to: #3036