-
Notifications
You must be signed in to change notification settings - Fork 8
feat: expose metrics for MCP calls #38
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
Changes from all commits
e845c33
045028c
5b6f44d
b4ba469
33ec9b7
71e1898
770a040
a09342a
eebbf5d
1acfbea
59e362b
634c829
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,5 @@ | ||
| name: argocd-mcp-server-e2e | ||
|
|
||
| version: '3.8' | ||
| services: | ||
| argocd-server-mock: | ||
| image: argocd-server-mock | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| package metrics | ||
|
|
||
| import ( | ||
| "github.com/prometheus/client_golang/prometheus" | ||
| "github.com/prometheus/client_golang/prometheus/promauto" | ||
| ) | ||
|
|
||
| var ( | ||
|
|
||
| // MCPCallsTotal counts total MCP calls by method, name (for `tools/call`) and success | ||
| MCPCallsTotal = promauto.NewCounterVec( | ||
| prometheus.CounterOpts{ | ||
| Name: "mcp_calls_total", | ||
| Help: "Total number of MCP calls", | ||
| }, | ||
| []string{"method", "name", "success"}, | ||
| ) | ||
|
Comment on lines
+11
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should have separate metrics for
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also it would be nice to have
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. but since we have labels for the Also, that's what is usually done with HTTP metrics: you have labels for the method ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought that having separated metrics for tools and methods would help with creating separate dashboards , since I guess we would be mostly interested into looking at the tools charts ( more than the methods ones ), but maybe I'm wrong.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe we ca have a single metric and use the labels to focus on tool calls per name, etc. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sounds good to me 👍 |
||
|
|
||
| // MCPCallDuration measures the duration of MCP calls by method, name (for `tools/call`) and success | ||
| MCPCallDuration = promauto.NewHistogramVec( | ||
| prometheus.HistogramOpts{ | ||
| Name: "mcp_call_duration_seconds", | ||
| Help: "Duration of MCP calls in seconds", | ||
| Buckets: prometheus.DefBuckets, | ||
| }, | ||
| []string{"method", "name", "success"}, | ||
| ) | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| package server | ||
|
|
||
| import ( | ||
| "context" | ||
| "log/slog" | ||
| "strconv" | ||
| "time" | ||
|
|
||
| "github.com/codeready-toolchain/argocd-mcp-server/internal/metrics" | ||
| "github.com/modelcontextprotocol/go-sdk/mcp" | ||
| ) | ||
|
|
||
| func NewMetricsMiddleware(logger *slog.Logger) mcp.Middleware { | ||
| return func(next mcp.MethodHandler) mcp.MethodHandler { | ||
| return func(ctx context.Context, method string, req mcp.Request) (mcp.Result, error) { | ||
| logger.Debug("metrics-middleware: received request", "method", method, "params", req.GetParams()) | ||
| // measure the duration of the request | ||
| start := time.Now() | ||
| // call the next middleware | ||
| result, err := next(ctx, method, req) | ||
| // measure the duration of the request | ||
| duration := time.Since(start) | ||
| var tool string | ||
| if p, ok := req.GetParams().(*mcp.CallToolParamsRaw); ok { | ||
| tool = p.Name | ||
| } | ||
| success := err == nil | ||
| if r, ok := result.(*mcp.CallToolResult); ok { | ||
| logger.Debug("metrics-middleware: call tool result", "is-error", r.IsError) | ||
| success = success && !r.IsError | ||
| } | ||
| // increment/update the metrics | ||
| metrics.MCPCallsTotal.WithLabelValues(method, tool, strconv.FormatBool(success)).Inc() | ||
| metrics.MCPCallDuration.WithLabelValues(method, tool, strconv.FormatBool(success)).Observe(float64(duration.Seconds())) | ||
| return result, err | ||
| } | ||
| } | ||
| } |
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 think we usually expose the metrics using it's own server and different port , but I guess this is fine for the mcp server that is not exposed publicly.
Uh oh!
There was an error while loading. Please reload this page.
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've opened a discussion on Slack for this specific question