From 4d8df196a490669fcaf10f777e7890134a77bf53 Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Thu, 6 Nov 2025 10:14:09 +0100 Subject: [PATCH 1/2] fix(mcp): replace orphaned spans with proper transactions Previously, MCP tool calls created orphaned spans without parent transactions, causing Sentry to display '' instead of meaningful transaction names. This change updates the tracing implementation to create proper transactions following OpenTelemetry MCP Semantic Conventions: - Transaction name: 'tools/call {tool_name}' - Operation: 'mcp.server' - Proper hub management to ensure context propagation - All MCP attributes correctly attached to transactions Resolves the issue of unlabeled transactions appearing in Sentry, making MCP tool analytics properly visible and filterable by tool name. --- internal/cli/mcp/sentry.go | 58 +++++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 26 deletions(-) diff --git a/internal/cli/mcp/sentry.go b/internal/cli/mcp/sentry.go index 10908a6..1889710 100644 --- a/internal/cli/mcp/sentry.go +++ b/internal/cli/mcp/sentry.go @@ -43,7 +43,7 @@ const ( ) // WithSentryTracing wraps an MCP tool handler with Sentry tracing. -// It creates spans following OpenTelemetry MCP semantic conventions and +// It creates transactions following OpenTelemetry MCP semantic conventions and // captures tool execution results and errors. // // Example usage: @@ -56,56 +56,62 @@ const ( // })) func WithSentryTracing[In, Out any](toolName string, handler mcp.ToolHandlerFor[In, Out]) mcp.ToolHandlerFor[In, Out] { return func(ctx context.Context, req *mcp.CallToolRequest, args In) (*mcp.CallToolResult, Out, error) { - // Create span for tool execution - span := sentry.StartSpan(ctx, OpMCPServer) - defer span.Finish() + // Get the current hub from context or create a new one + hub := sentry.GetHubFromContext(ctx) + if hub == nil { + hub = sentry.CurrentHub().Clone() + ctx = sentry.SetHubOnContext(ctx, hub) + } - // Set span name following MCP conventions: "tools/call {tool_name}" - span.Description = fmt.Sprintf("tools/call %s", toolName) + // Create transaction for tool execution following MCP conventions + // Transaction name: "tools/call {tool_name}" (e.g., "tools/call get_action_parameters") + transactionName := fmt.Sprintf("tools/call %s", toolName) + transaction := sentry.StartTransaction(ctx, + transactionName, + sentry.WithOpName(OpMCPServer), + sentry.WithTransactionSource(sentry.SourceCustom), + ) + defer transaction.Finish() // Set common MCP attributes - span.SetData(AttrMCPMethodName, "tools/call") - span.SetData(AttrMCPToolName, toolName) - span.SetData(AttrMCPTransport, TransportStdio) - span.SetData(AttrNetworkTransport, NetworkTransportPipe) - span.SetData(AttrNetworkProtocolVer, JSONRPCVersion) + transaction.SetData(AttrMCPMethodName, "tools/call") + transaction.SetData(AttrMCPToolName, toolName) + transaction.SetData(AttrMCPTransport, TransportStdio) + transaction.SetData(AttrNetworkTransport, NetworkTransportPipe) + transaction.SetData(AttrNetworkProtocolVer, JSONRPCVersion) // Set Sentry-specific attributes - span.SetData("sentry.origin", OriginMCPFunction) - span.SetData("sentry.source", SourceMCPRoute) + transaction.SetData("sentry.origin", OriginMCPFunction) + transaction.SetData("sentry.source", SourceMCPRoute) // Extract and set request ID if available if req != nil { // The CallToolRequest may have metadata we can extract // For now, we'll use reflection to check if there's an ID field - setRequestMetadata(span, req) + setRequestMetadata(transaction, req) } // Extract and set tool arguments - setToolArguments(span, args) + setToolArguments(transaction, args) - // Execute the handler with the span's context - ctx = span.Context() + // Execute the handler with the transaction's context + ctx = transaction.Context() result, data, err := handler(ctx, req, args) // Capture error if present if err != nil { - span.Status = sentry.SpanStatusInternalError - span.SetData(AttrMCPToolResultIsError, true) + transaction.Status = sentry.SpanStatusInternalError + transaction.SetData(AttrMCPToolResultIsError, true) // Capture the error to Sentry with context - hub := sentry.GetHubFromContext(ctx) - if hub == nil { - hub = sentry.CurrentHub() - } hub.CaptureException(err) } else { - span.Status = sentry.SpanStatusOK - span.SetData(AttrMCPToolResultIsError, false) + transaction.Status = sentry.SpanStatusOK + transaction.SetData(AttrMCPToolResultIsError, false) // Extract result metadata if result != nil { - setResultMetadata(span, result) + setResultMetadata(transaction, result) } } From f1a76a19d00f001716954802d69dc54723c461ca Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Thu, 6 Nov 2025 10:25:55 +0100 Subject: [PATCH 2/2] fix(github): trim whitespace from action references Fixes issue where action references with trailing newlines or other whitespace characters would fail with URL parsing errors. The ParseActionRef function now trims all leading and trailing whitespace (spaces, tabs, newlines) before parsing the action reference. Added comprehensive unit tests covering: - Trailing newlines - Leading and trailing whitespace - Tabs and newlines - Edge cases (empty strings, whitespace-only strings) --- internal/github/actions.go | 3 + internal/github/actions_test.go | 110 ++++++++++++++++++++++++++++++++ 2 files changed, 113 insertions(+) create mode 100644 internal/github/actions_test.go diff --git a/internal/github/actions.go b/internal/github/actions.go index b464e63..a22e7da 100644 --- a/internal/github/actions.go +++ b/internal/github/actions.go @@ -34,6 +34,9 @@ type ActionRef struct { // - "actions/checkout@v5" -> {Owner: "actions", Repo: "checkout", Version: "v5"} // - "actions/setup-node@v4" -> {Owner: "actions", Repo: "setup-node", Version: "v4"} func ParseActionRef(ref string) (*ActionRef, error) { + // Trim whitespace (including newlines, spaces, tabs) + ref = strings.TrimSpace(ref) + if ref == "" { return nil, fmt.Errorf("action reference cannot be empty") } diff --git a/internal/github/actions_test.go b/internal/github/actions_test.go new file mode 100644 index 0000000..b40cc64 --- /dev/null +++ b/internal/github/actions_test.go @@ -0,0 +1,110 @@ +package github + +import ( + "testing" +) + +func TestParseActionRef(t *testing.T) { + tests := []struct { + name string + input string + wantOwner string + wantRepo string + wantVersion string + wantErr bool + }{ + { + name: "valid action reference", + input: "actions/checkout@v5", + wantOwner: "actions", + wantRepo: "checkout", + wantVersion: "v5", + wantErr: false, + }, + { + name: "valid action with trailing newline", + input: "actions/checkout@v5\n", + wantOwner: "actions", + wantRepo: "checkout", + wantVersion: "v5", + wantErr: false, + }, + { + name: "valid action with leading and trailing whitespace", + input: " actions/checkout@v5 ", + wantOwner: "actions", + wantRepo: "checkout", + wantVersion: "v5", + wantErr: false, + }, + { + name: "valid action with tabs and newlines", + input: "\t\nactions/checkout@v5\n\t", + wantOwner: "actions", + wantRepo: "checkout", + wantVersion: "v5", + wantErr: false, + }, + { + name: "complex action reference with whitespace", + input: "kula-app/wait-for-services-action@v1\n", + wantOwner: "kula-app", + wantRepo: "wait-for-services-action", + wantVersion: "v1", + wantErr: false, + }, + { + name: "empty string", + input: "", + wantErr: true, + }, + { + name: "only whitespace", + input: " \n\t ", + wantErr: true, + }, + { + name: "missing version", + input: "actions/checkout", + wantErr: true, + }, + { + name: "missing repo", + input: "actions@v5", + wantErr: true, + }, + { + name: "invalid format - too many slashes", + input: "actions/github/checkout@v5", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := ParseActionRef(tt.input) + + if tt.wantErr { + if err == nil { + t.Errorf("ParseActionRef() expected error but got none") + } + return + } + + if err != nil { + t.Errorf("ParseActionRef() unexpected error: %v", err) + return + } + + if got.Owner != tt.wantOwner { + t.Errorf("ParseActionRef() Owner = %v, want %v", got.Owner, tt.wantOwner) + } + if got.Repo != tt.wantRepo { + t.Errorf("ParseActionRef() Repo = %v, want %v", got.Repo, tt.wantRepo) + } + if got.Version != tt.wantVersion { + t.Errorf("ParseActionRef() Version = %v, want %v", got.Version, tt.wantVersion) + } + }) + } +}