Skip to content
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ server.prompt('echo', { message: z.string() }, ({ message }, extra) => ({
],
}));

server.tool('always-error', {}, async () => {
throw new Error('intentional error for span status testing');
});

const transports: Record<string, SSEServerTransport> = {};

mcpRouter.get('/sse', async (_, res) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,23 @@ test('Should record transactions for mcp handlers', async ({ baseURL }) => {

// TODO: When https://github.com/modelcontextprotocol/typescript-sdk/pull/358 is released check for trace id equality between the post transaction and the handler transaction
});

await test.step('error tool sets span status to internal_error', async () => {
const toolTransactionPromise = waitForTransaction('node-express-v5', transactionEvent => {
return transactionEvent.transaction === 'tools/call always-error';
});

try {
await client.callTool({ name: 'always-error', arguments: {} });
} catch {
// Expected: MCP SDK throws when the tool returns a JSON-RPC error
}

const toolTransaction = await toolTransactionPromise;
expect(toolTransaction).toBeDefined();
expect(toolTransaction.contexts?.trace?.op).toEqual('mcp.server');
expect(toolTransaction.contexts?.trace?.status).toEqual('internal_error');
});
});

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ server.prompt('echo', { message: z.string() }, ({ message }, extra) => ({
],
}));

server.tool('always-error', {}, async () => {
throw new Error('intentional error for span status testing');
});

const transports: Record<string, SSEServerTransport> = {};

mcpRouter.get('/sse', async (_, res) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,23 @@ test('Should record transactions for mcp handlers', async ({ baseURL }) => {
expect(promptTransaction.contexts?.trace?.data?.['mcp.method.name']).toEqual('prompts/get');
// TODO: When https://github.com/modelcontextprotocol/typescript-sdk/pull/358 is released check for trace id equality between the post transaction and the handler transaction
});

await test.step('error tool sets span status to internal_error', async () => {
const toolTransactionPromise = waitForTransaction('node-express', transactionEvent => {
return transactionEvent.transaction === 'tools/call always-error';
});

try {
await client.callTool({ name: 'always-error', arguments: {} });
} catch {
// Expected: MCP SDK throws when the tool returns a JSON-RPC error
}

const toolTransaction = await toolTransactionPromise;
expect(toolTransaction).toBeDefined();
expect(toolTransaction.contexts?.trace?.op).toEqual('mcp.server');
expect(toolTransaction.contexts?.trace?.status).toEqual('internal_error');
});
});

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ server.prompt('echo', { message: z.string() }, ({ message }, extra) => ({
],
}));

server.tool('always-error', {}, async () => {
throw new Error('intentional error for span status testing');
});

const transports: Record<string, SSEServerTransport> = {};

mcpRouter.get('/sse', async (_, res) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,23 @@ test('Records transactions for mcp handlers', async ({ baseURL }) => {

// TODO: When https://github.com/modelcontextprotocol/typescript-sdk/pull/358 is released check for trace id equality between the post transaction and the handler transaction
});

await test.step('error tool sets span status to internal_error', async () => {
const toolTransactionPromise = waitForTransaction('tsx-express', transactionEvent => {
return transactionEvent.transaction === 'tools/call always-error';
});

try {
await client.callTool({ name: 'always-error', arguments: {} });
} catch {
// Expected: MCP SDK throws when the tool returns a JSON-RPC error
}

const toolTransaction = await toolTransactionPromise;
expect(toolTransaction).toBeDefined();
expect(toolTransaction.contexts?.trace?.op).toEqual('mcp.server');
expect(toolTransaction.contexts?.trace?.status).toEqual('internal_error');
});
});

/**
Expand Down
6 changes: 5 additions & 1 deletion packages/core/src/integrations/mcp-server/correlation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,19 +81,23 @@ export function storeSpanForRequest(transport: MCPTransport, requestId: RequestI
* @param requestId - Request identifier
* @param result - Execution result for attribute extraction
* @param options - Resolved MCP options
* @param hasError - Whether the JSON-RPC response contained an error
*/
export function completeSpanWithResults(
transport: MCPTransport,
requestId: RequestId,
result: unknown,
options: ResolvedMcpOptions,
hasError = false,
): void {
const spanMap = getOrCreateSpanMap(transport);
const spanData = spanMap.get(requestId);
if (spanData) {
const { span, method } = spanData;

if (method === 'initialize') {
if (hasError) {
span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' });
} else if (method === 'initialize') {
const sessionData = extractSessionDataFromInitializeResponse(result);
const serverAttributes = buildServerAttributesFromInfo(sessionData.serverInfo);

Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/integrations/mcp-server/transport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ export function wrapTransportSend(transport: MCPTransport, options: ResolvedMcpO
}
}

completeSpanWithResults(this, message.id, message.result, options);
completeSpanWithResults(this, message.id, message.result, options, !!message.error);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,68 @@ describe('MCP Server Transport Instrumentation', () => {
// Trigger onclose - should not throw
expect(() => mockTransport.onclose?.()).not.toThrow();
});

it('should set span status to error when JSON-RPC error response is sent', async () => {
const mockSpan = {
setAttributes: vi.fn(),
setStatus: vi.fn(),
end: vi.fn(),
isRecording: vi.fn().mockReturnValue(true),
};
startInactiveSpanSpy.mockReturnValue(mockSpan as any);

await wrappedMcpServer.connect(mockTransport);

// Simulate an incoming tools/call request
const jsonRpcRequest = {
jsonrpc: '2.0',
method: 'tools/call',
id: 'req-err-1',
params: { name: 'always-error' },
};
mockTransport.onmessage?.(jsonRpcRequest, {});

// Simulate the MCP SDK sending back a JSON-RPC error response
const jsonRpcErrorResponse = {
jsonrpc: '2.0',
id: 'req-err-1',
error: { code: -32603, message: 'Internal error: tool threw an exception' },
};
await mockTransport.send?.(jsonRpcErrorResponse as any);

expect(mockSpan.setStatus).toHaveBeenCalledWith({ code: 2, message: 'internal_error' });
expect(mockSpan.end).toHaveBeenCalled();
});

it('should not set error span status for successful JSON-RPC responses', async () => {
const mockSpan = {
setAttributes: vi.fn(),
setStatus: vi.fn(),
end: vi.fn(),
isRecording: vi.fn().mockReturnValue(true),
};
startInactiveSpanSpy.mockReturnValue(mockSpan as any);

await wrappedMcpServer.connect(mockTransport);

const jsonRpcRequest = {
jsonrpc: '2.0',
method: 'tools/call',
id: 'req-ok-1',
params: { name: 'echo' },
};
mockTransport.onmessage?.(jsonRpcRequest, {});

const jsonRpcSuccessResponse = {
jsonrpc: '2.0',
id: 'req-ok-1',
result: { content: [{ type: 'text', text: 'hello' }] },
};
await mockTransport.send?.(jsonRpcSuccessResponse as any);

expect(mockSpan.setStatus).not.toHaveBeenCalled();
expect(mockSpan.end).toHaveBeenCalled();
});
});

describe('Stdio Transport Tests', () => {
Expand Down
Loading