-
Notifications
You must be signed in to change notification settings - Fork 900
add mcp metrics #3828
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: main
Are you sure you want to change the base?
add mcp metrics #3828
Conversation
rockwotj
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.
This is getting some changes with respect to the SDK we use here: #3826 hopefully that provides better middleware to hook into (it was needed for authz).
15fad5b to
b4c5f14
Compare
Use the official go-sdk middleware infrastructure (AddReceivingMiddleware) instead of manual instrumentation. This tracks all MCP method calls and provides detailed tool-specific metrics (invocations, duration, sizes, errors) across all transports (stdio, SSE, streamable HTTP).
b4c5f14 to
9815ec5
Compare
|
@rockwotj claude made the updates, can you check it out? :) |
internal/mcp/metrics/metrics.go
Outdated
| @@ -0,0 +1,131 @@ | |||
| // Copyright 2024 Redpanda Data, Inc. | |||
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.
internal/mcp/metrics/metrics.go
Outdated
| m.toolInvocations.Incr(1, toolName, "error") | ||
| m.toolErrors.Incr(1, toolName, "invocation_error") |
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 why we need both metrics here
|
@rockwotj addressed |
9c6bcfe to
75f1db6
Compare
75f1db6 to
4b3edc8
Compare
|
@rockwotj okay, 🤞 now :) |

No description provided.