Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions cmd/docker-mcp/internal/gateway/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ func (g *Gateway) mcpServerToolHandler(serverConfig *catalog.ServerConfig, serve
attribute.String("mcp.server.name", serverConfig.Name),
attribute.String("mcp.server.type", serverType),
attribute.String("mcp.tool.name", req.Params.Name),
attribute.String("mcp.client.name", req.Session.InitializeParams().ClientInfo.Name),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Name is safest here. We do have Title and Version as well and are meant to be the human-readable ones for displays in UIs. Should we start logging these parameters when clients connect to the gateway so we're more accustomed to how these different clients identify themselves? I think it'd be worth logging one client connected message and showing the name, title and version of the client.

I still think the Name is the right thing to add to the event.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I think that makes sense. Created an issue: #141

),
)

Expand Down Expand Up @@ -107,6 +108,7 @@ func (g *Gateway) mcpServerToolHandler(serverConfig *catalog.ServerConfig, serve
attribute.String("mcp.server.name", serverConfig.Name),
attribute.String("mcp.server.type", serverType),
attribute.String("mcp.tool.name", req.Params.Name),
attribute.String("mcp.client.name", req.Session.InitializeParams().ClientInfo.Name),
),
)

Expand Down Expand Up @@ -153,7 +155,7 @@ func (g *Gateway) mcpServerPromptHandler(serverConfig *catalog.ServerConfig, ser
defer span.End()

// Record prompt get counter
telemetry.RecordPromptGet(ctx, req.Params.Name, serverConfig.Name)
telemetry.RecordPromptGet(ctx, req.Params.Name, serverConfig.Name, req.Session.InitializeParams().ClientInfo.Name)

client, err := g.clientPool.AcquireClient(ctx, serverConfig, getClientConfig(nil, req.Session, server))
if err != nil {
Expand All @@ -168,7 +170,7 @@ func (g *Gateway) mcpServerPromptHandler(serverConfig *catalog.ServerConfig, ser

// Record duration
duration := time.Since(startTime).Milliseconds()
telemetry.RecordPromptDuration(ctx, req.Params.Name, serverConfig.Name, float64(duration))
telemetry.RecordPromptDuration(ctx, req.Params.Name, serverConfig.Name, float64(duration), req.Session.InitializeParams().ClientInfo.Name)

if err != nil {
span.RecordError(err)
Expand Down Expand Up @@ -214,7 +216,7 @@ func (g *Gateway) mcpServerResourceHandler(serverConfig *catalog.ServerConfig, s
defer span.End()

// Record counter with server attribution
telemetry.RecordResourceRead(ctx, req.Params.URI, serverConfig.Name)
telemetry.RecordResourceRead(ctx, req.Params.URI, serverConfig.Name, req.Session.InitializeParams().ClientInfo.Name)

client, err := g.clientPool.AcquireClient(ctx, serverConfig, getClientConfig(nil, req.Session, server))
if err != nil {
Expand All @@ -229,7 +231,7 @@ func (g *Gateway) mcpServerResourceHandler(serverConfig *catalog.ServerConfig, s

// Record duration regardless of error
duration := time.Since(startTime).Milliseconds()
telemetry.RecordResourceDuration(ctx, req.Params.URI, serverConfig.Name, float64(duration))
telemetry.RecordResourceDuration(ctx, req.Params.URI, serverConfig.Name, float64(duration), req.Session.InitializeParams().ClientInfo.Name)

if err != nil {
span.RecordError(err)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func TestPromptHandlerTelemetry(t *testing.T) {

// Record prompt call
ctx := context.Background()
telemetry.RecordPromptGet(ctx, promptName, serverConfig.Name)
telemetry.RecordPromptGet(ctx, promptName, serverConfig.Name, "test-client")

// Collect metrics
var rm metricdata.ResourceMetrics
Expand Down Expand Up @@ -79,6 +79,8 @@ func TestPromptHandlerTelemetry(t *testing.T) {
attribute.String("mcp.prompt.name", promptName))
assert.Contains(t, attrs,
attribute.String("mcp.server.origin", serverConfig.Name))
assert.Contains(t, attrs,
attribute.String("mcp.client.name", "test-client"))
}
}
}
Expand Down Expand Up @@ -110,7 +112,7 @@ func TestPromptHandlerTelemetry(t *testing.T) {

// Record prompt duration
ctx := context.Background()
telemetry.RecordPromptDuration(ctx, promptName, serverConfig.Name, duration)
telemetry.RecordPromptDuration(ctx, promptName, serverConfig.Name, duration, "test-client")

// Collect metrics
var rm metricdata.ResourceMetrics
Expand Down Expand Up @@ -142,6 +144,8 @@ func TestPromptHandlerTelemetry(t *testing.T) {
attribute.String("mcp.prompt.name", promptName))
assert.Contains(t, attrs,
attribute.String("mcp.server.origin", serverConfig.Name))
assert.Contains(t, attrs,
attribute.String("mcp.client.name", "test-client"))
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ func TestResourceHandlerTelemetry(t *testing.T) {
// Test data
ctx := context.Background()
resourceURI := "file:///test/resource.txt"
clientName := "test-client"
serverConfig := &catalog.ServerConfig{
Name: "test-server",
Spec: catalog.Server{
Expand All @@ -63,7 +64,7 @@ func TestResourceHandlerTelemetry(t *testing.T) {
}

// Record resource read
telemetry.RecordResourceRead(ctx, resourceURI, serverConfig.Name)
telemetry.RecordResourceRead(ctx, resourceURI, serverConfig.Name, clientName)

// Verify metrics were collected
var rm metricdata.ResourceMetrics
Expand Down Expand Up @@ -92,6 +93,7 @@ func TestResourceHandlerTelemetry(t *testing.T) {
attrs := sum.DataPoints[0].Attributes
hasURI := false
hasServer := false
hasClient := false
for _, attr := range attrs.ToSlice() {
if attr.Key == "mcp.resource.uri" {
assert.Equal(t, resourceURI, attr.Value.AsString())
Expand All @@ -101,9 +103,14 @@ func TestResourceHandlerTelemetry(t *testing.T) {
assert.Equal(t, serverConfig.Name, attr.Value.AsString())
hasServer = true
}
if attr.Key == "mcp.client.name" {
assert.Equal(t, clientName, attr.Value.AsString())
hasClient = true
}
}
assert.True(t, hasURI, "Should have resource URI attribute")
assert.True(t, hasServer, "Should have server origin attribute")
assert.True(t, hasClient, "Should have client name attribute")
}
}
}
Expand All @@ -126,10 +133,11 @@ func TestResourceHandlerTelemetry(t *testing.T) {
ctx := context.Background()
resourceURI := "file:///test/resource.txt"
serverName := "test-server"
clientName := "test-client"
duration := 42.5 // milliseconds

// Record duration
telemetry.RecordResourceDuration(ctx, resourceURI, serverName, duration)
telemetry.RecordResourceDuration(ctx, resourceURI, serverName, duration, clientName)

// Verify metrics
var rm metricdata.ResourceMetrics
Expand Down Expand Up @@ -284,9 +292,10 @@ func TestResourceTemplateHandlerTelemetry(t *testing.T) {
ctx := context.Background()
uriTemplate := "file:///test/{id}/resource.txt"
serverName := "test-server"
clientName := "test-client"

// Record resource template read
telemetry.RecordResourceTemplateRead(ctx, uriTemplate, serverName)
telemetry.RecordResourceTemplateRead(ctx, uriTemplate, serverName, clientName)

// Verify metrics
var rm metricdata.ResourceMetrics
Expand Down Expand Up @@ -484,6 +493,7 @@ func TestMcpServerResourceHandlerInstrumentation(t *testing.T) {
Image: "test/image:latest",
},
}
clientName := "test-client"
params := &mcp.ReadResourceParams{
URI: "file:///test/resource.txt",
}
Expand All @@ -498,14 +508,14 @@ func TestMcpServerResourceHandlerInstrumentation(t *testing.T) {
startTime := time.Now()

// Record counter (as handler would)
telemetry.RecordResourceRead(ctx, params.URI, serverConfig.Name)
telemetry.RecordResourceRead(ctx, params.URI, serverConfig.Name, clientName)

// Simulate some work
time.Sleep(10 * time.Millisecond)

// Record duration (as handler would)
duration := time.Since(startTime).Milliseconds()
telemetry.RecordResourceDuration(ctx, params.URI, serverConfig.Name, float64(duration))
telemetry.RecordResourceDuration(ctx, params.URI, serverConfig.Name, float64(duration), clientName)

// End span
span.End()
Expand Down Expand Up @@ -560,6 +570,7 @@ func TestMcpServerResourceHandlerInstrumentation(t *testing.T) {
Image: "test/image:latest",
},
}
clientName := "test-client"
params := &mcp.ReadResourceParams{
URI: "file:///test/missing.txt",
}
Expand All @@ -572,7 +583,7 @@ func TestMcpServerResourceHandlerInstrumentation(t *testing.T) {
)

// Record counter
telemetry.RecordResourceRead(ctx, params.URI, serverConfig.Name)
telemetry.RecordResourceRead(ctx, params.URI, serverConfig.Name, clientName)

// Simulate error
err := errors.New("resource not found")
Expand Down
5 changes: 5 additions & 0 deletions cmd/docker-mcp/internal/gateway/handlers_telemetry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,13 +170,15 @@ func TestTelemetryMetricRecording(t *testing.T) {
serverName := "test-server"
serverType := "docker"
toolName := "test-tool"
clientName := "test-client"

// Record metrics as the handler would
telemetry.ToolCallCounter.Add(ctx, 1,
metric.WithAttributes(
attribute.String("mcp.server.name", serverName),
attribute.String("mcp.server.type", serverType),
attribute.String("mcp.tool.name", toolName),
attribute.String("mcp.client.name", clientName),
),
)

Expand All @@ -186,6 +188,7 @@ func TestTelemetryMetricRecording(t *testing.T) {
attribute.String("mcp.server.name", serverName),
attribute.String("mcp.server.type", serverType),
attribute.String("mcp.tool.name", toolName),
attribute.String("mcp.client.name", clientName),
),
)

Expand All @@ -210,6 +213,7 @@ func TestTelemetryMetricRecording(t *testing.T) {
assertMetricAttribute(t, attrs, "mcp.server.name", serverName)
assertMetricAttribute(t, attrs, "mcp.server.type", serverType)
assertMetricAttribute(t, attrs, "mcp.tool.name", toolName)
assertMetricAttribute(t, attrs, "mcp.client.name", clientName)

case "mcp.tool.duration":
foundHistogram = true
Expand Down Expand Up @@ -303,6 +307,7 @@ func TestToolCallDurationMeasurement(t *testing.T) {
attribute.String("mcp.server.name", "test-server"),
attribute.String("mcp.server.type", "docker"),
attribute.String("mcp.tool.name", "test-tool"),
attribute.String("mcp.client.name", "test-client"),
),
)

Expand Down
12 changes: 8 additions & 4 deletions cmd/docker-mcp/internal/interceptors/telemetry.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,24 @@ func TelemetryMiddleware() mcp.Middleware {
telemetry.RecordInitialize(ctx, params)
tracked = true
case "tools/list":
session := req.GetSession().(*mcp.ServerSession)
ctx, span = telemetry.StartListSpan(ctx, "tools")
telemetry.RecordListTools(ctx)
telemetry.RecordListTools(ctx, session.InitializeParams().ClientInfo.Name)
tracked = true
case "prompts/list":
session := req.GetSession().(*mcp.ServerSession)
ctx, span = telemetry.StartListSpan(ctx, "prompts")
telemetry.RecordListPrompts(ctx)
telemetry.RecordListPrompts(ctx, session.InitializeParams().ClientInfo.Name)
tracked = true
case "resources/list":
session := req.GetSession().(*mcp.ServerSession)
ctx, span = telemetry.StartListSpan(ctx, "resources")
telemetry.RecordListResources(ctx)
telemetry.RecordListResources(ctx, session.InitializeParams().ClientInfo.Name)
tracked = true
case "resourceTemplates/list":
session := req.GetSession().(*mcp.ServerSession)
ctx, span = telemetry.StartListSpan(ctx, "resourceTemplates")
telemetry.RecordListResourceTemplates(ctx)
telemetry.RecordListResourceTemplates(ctx, session.InitializeParams().ClientInfo.Name)
tracked = true
}

Expand Down
Loading
Loading