- Notifications
You must be signed in to change notification settings - Fork 155
Add backend routing capture to vMCP audit logs #2983
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
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@ ## main #2983 +/- ## ========================================== + Coverage 56.18% 56.25% +0.07% ========================================== Files 330 331 +1 Lines 32468 32520 +52 ========================================== + Hits 18243 18295 +52 + Misses 12676 12675 -1 - Partials 1549 1550 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Enhance audit logging to capture which backend workload handled each MCP request (tools/call, resources/read, prompts/get). Backend routing information is now recorded in audit events as backend_name in the metadata.extra field.
02ef850 to 1ffe5f8 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
This PR enhances vMCP audit logging to capture backend routing information, recording which backend workload handled each MCP request (tools/call, resources/read, prompts/get) in the audit event metadata. The implementation uses a mutable context-based approach where the audit middleware creates a BackendInfo struct, and a new backend enrichment middleware populates it by looking up the routing table.
Key Changes
- Added backend enrichment middleware that parses MCP requests and looks up backend names from the routing table
- Extended audit event metadata to include backend_name when available
- Introduced BackendInfo context helper to enable cross-middleware data sharing
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/vmcp/server/server.go | Registers backend enrichment middleware in the handler chain between discovery and telemetry |
| pkg/vmcp/server/backend_enrichment.go | New middleware that extracts backend routing information from MCP requests and populates BackendInfo in context |
| pkg/audit/auditor.go | Adds BackendInfo context helpers and updates addMetadata to include backend_name in audit events |
| pkg/audit/backend_info_test.go | Unit tests for BackendInfo context storage and mutation behavior |
| pkg/audit/auditor_test.go | Updates TestAddMetadata to accommodate new request parameter in addMetadata signature |
| pkg/vmcp/server/integration_test.go | Extends audit logging test to verify backend_name is captured in tools/call and resources/read audit events |
Comments suppressed due to low confidence (1)
pkg/audit/auditor_test.go:528
- The test for addMetadata should verify that backend_name is only added to metadata.extra when BackendInfo is present in the request context with a non-empty BackendName. Consider adding test cases to verify: (1) backend_name is added when BackendInfo exists with a name, (2) backend_name is not added when BackendInfo is missing, and (3) backend_name is not added when BackendInfo.BackendName is empty.
func TestAddMetadata(t *testing.T) { t.Parallel() auditor, err := NewAuditorWithTransport(&Config{}, "sse") require.NoError(t, err) event := NewAuditEvent("test", EventSource{}, OutcomeSuccess, map[string]string{}, "test") duration := 150 * time.Millisecond rw := &responseWriter{ ResponseWriter: httptest.NewRecorder(), body: bytes.NewBufferString("test response"), } req := httptest.NewRequest("GET", "/test", nil) auditor.addMetadata(event, req, duration, rw) require.NotNil(t, event.Metadata.Extra) assert.Equal(t, int64(150), event.Metadata.Extra[MetadataExtraKeyDuration]) assert.Equal(t, "sse", event.Metadata.Extra[MetadataExtraKeyTransport]) assert.Equal(t, 13, event.Metadata.Extra[MetadataExtraKeyResponseSize]) // "test response" length } 💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
JAORMX 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.
Yay! This is exactly how to use the audit log's metada 😀
30e95f4 to 1871f27 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.
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.
| ✅ 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!
Enhance audit logging to capture which backend workload handled each MCP request (tools/call, resources/read, prompts/get). Backend routing information is now recorded in audit events as backend_name in the metadata.extra field.
Partially-closes: #2980
Large PR Justification
This is an atomic PR, including complete tests, that are needed to properly validate the feature.