From 8916f83beea6f0896d4f4ea9d8b30b4366db3ebf Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Tue, 28 Apr 2026 17:55:32 +0200 Subject: [PATCH] Instrument MCP tool calls with semantically appropriate span Signed-off-by: Eric Peterson --- .../mcp-actions-backend-otel-tracing.md | 5 + docs/ai/mcp-actions.md | 36 ++++ plugins/mcp-actions-backend/config.d.ts | 17 ++ .../mcp-actions-backend/src/plugin.test.ts | 7 +- plugins/mcp-actions-backend/src/plugin.ts | 8 + .../src/services/McpService.test.ts | 178 ++++++++++++++++++ .../src/services/McpService.ts | 133 +++++++++---- 7 files changed, 346 insertions(+), 38 deletions(-) create mode 100644 .changeset/mcp-actions-backend-otel-tracing.md diff --git a/.changeset/mcp-actions-backend-otel-tracing.md b/.changeset/mcp-actions-backend-otel-tracing.md new file mode 100644 index 0000000000..c463205370 --- /dev/null +++ b/.changeset/mcp-actions-backend-otel-tracing.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-mcp-actions-backend': patch +--- + +Trace spans are now emitted for MCP `tools/call` invocations, following OpenTelemetry server-side MCP semantic conventions. diff --git a/docs/ai/mcp-actions.md b/docs/ai/mcp-actions.md index 210167e8ae..fb1a805ebf 100644 --- a/docs/ai/mcp-actions.md +++ b/docs/ai/mcp-actions.md @@ -292,3 +292,39 @@ The MCP Actions Backend emits metrics for the following operations: - `mcp.server.session.duration`: The duration of the MCP session from the perspective of the server See the [OpenTelemetry tutorial](../tutorials/setup-opentelemetry.md) to learn how to make these metrics available. + +## Tracing + +The MCP Actions Backend emits a trace span for each `tools/call` invocation via the [Tracing Service](../backend-system/core-services/tracing.md), following the [OpenTelemetry server-side MCP semantic conventions](https://opentelemetry.io/docs/specs/semconv/gen-ai/mcp/#server). Each span uses the name `tools/call `, server kind, and includes the standard MCP attributes (`mcp.method.name`, `gen_ai.tool.name`, `gen_ai.operation.name`). When a tool result returns `isError: true`, the span is marked with `error.type=tool_error`; thrown handler errors are recorded and the span status is set to `ERROR`. + +In addition to those attributes, the Tracing Service automatically attaches the authenticated principal's type as `backstage.principal.type` (one of `user`, `service`, or `none`). Each `tools/call` span is also attributed to the plugin that owns the invoked action via `backstage.plugin.id` (e.g. `catalog`, `scaffolder`) — overriding the default `mcp-actions` value so tracing backends can filter activity by the source plugin rather than by the MCP transport. + +### Capturing the authenticated end user + +The Tracing Service can additionally include the authenticated principal's identity as `enduser.id` (the user entity ref for a user principal, the service subject for a service principal). This is gated behind a backend-wide configuration flag and is **disabled by default**: + +```yaml title="app-config.yaml" +backend: + tracing: + capture: + endUser: true # defaults to false +``` + +This flag is honored by every plugin that creates spans through the [Tracing Service](../backend-system/core-services/tracing.md), not just MCP Actions. + +### Capturing tool arguments and results + +When `backend.tracing.capture.toolPayloads` is enabled, the tool's input arguments and output result are recorded on the span as `gen_ai.tool.call.arguments` and `gen_ai.tool.call.result`. + +```yaml title="app-config.yaml" +backend: + tracing: + capture: + toolPayloads: true # defaults to false +``` + +:::warning +These attributes are marked Opt-In by the OpenTelemetry GenAI semantic conventions because they may contain sensitive information — entity payloads, scaffolder inputs, free-form text, and so on. Only enable this flag if your tracing backend's data handling is appropriate for the kinds of payloads your MCP tools accept and produce. +::: + +See the [OpenTelemetry tutorial](../tutorials/setup-opentelemetry.md) to learn how to make these spans available. diff --git a/plugins/mcp-actions-backend/config.d.ts b/plugins/mcp-actions-backend/config.d.ts index a530f5b038..a20ab6d4b8 100644 --- a/plugins/mcp-actions-backend/config.d.ts +++ b/plugins/mcp-actions-backend/config.d.ts @@ -15,6 +15,23 @@ */ export interface Config { + backend?: { + tracing?: { + capture?: { + /** + * When true, the tool call's input arguments and output result are + * included on the MCP `tools/call` server span as + * `gen_ai.tool.call.arguments` and `gen_ai.tool.call.result`. + * These attributes are marked Opt-In by the OpenTelemetry GenAI + * semantic conventions because they may contain sensitive + * information (entity payloads, scaffolder inputs, free-form + * text). Defaults to false. + */ + toolPayloads?: boolean; + }; + }; + }; + mcpActions?: { /** * Display name for the MCP server. Defaults to "backstage". diff --git a/plugins/mcp-actions-backend/src/plugin.test.ts b/plugins/mcp-actions-backend/src/plugin.test.ts index 31dce6bad5..e64254eb7e 100644 --- a/plugins/mcp-actions-backend/src/plugin.test.ts +++ b/plugins/mcp-actions-backend/src/plugin.test.ts @@ -14,7 +14,10 @@ * limitations under the License. */ import { mockServices, startTestBackend } from '@backstage/backend-test-utils'; -import { metricsServiceMock } from '@backstage/backend-test-utils/alpha'; +import { + metricsServiceMock, + tracingServiceMock, +} from '@backstage/backend-test-utils/alpha'; import { mcpPlugin } from './plugin'; import { actionsRegistryServiceRef } from '@backstage/backend-plugin-api/alpha'; import { createBackendPlugin } from '@backstage/backend-plugin-api'; @@ -54,6 +57,7 @@ describe('Mcp Backend', () => { mcpPlugin, mockPluginWithActions, metricsServiceMock.mock().factory, + tracingServiceMock.mock().factory, mockServices.rootConfig.factory({ data: { backend: { @@ -220,6 +224,7 @@ describe('Mcp Backend', () => { mockCatalogPlugin, mockScaffolderPlugin, metricsServiceMock.mock().factory, + tracingServiceMock.mock().factory, mockServices.rootConfig.factory({ data: { backend: { diff --git a/plugins/mcp-actions-backend/src/plugin.ts b/plugins/mcp-actions-backend/src/plugin.ts index 6baaf8489d..e0ce76dc4d 100644 --- a/plugins/mcp-actions-backend/src/plugin.ts +++ b/plugins/mcp-actions-backend/src/plugin.ts @@ -26,6 +26,7 @@ import { actionsRegistryServiceRef, actionsServiceRef, metricsServiceRef, + tracingServiceRef, } from '@backstage/backend-plugin-api/alpha'; import { parseServerConfigs } from './config'; @@ -49,6 +50,7 @@ export const mcpPlugin = createBackendPlugin({ discovery: coreServices.discovery, config: coreServices.rootConfig, metrics: metricsServiceRef, + tracing: tracingServiceRef, }, async init({ actions, @@ -59,16 +61,22 @@ export const mcpPlugin = createBackendPlugin({ discovery, config, metrics, + tracing, }) { const serverConfigs = parseServerConfigs(config); const namespacedToolNames = config.getOptionalBoolean( 'mcpActions.namespacedToolNames', ); + const captureToolPayloads = + config.getOptionalBoolean('backend.tracing.capture.toolPayloads') ?? + false; const mcpService = await McpService.create({ actions, metrics, namespacedToolNames, + tracingService: tracing, + captureToolPayloads, }); const router = Router(); diff --git a/plugins/mcp-actions-backend/src/services/McpService.test.ts b/plugins/mcp-actions-backend/src/services/McpService.test.ts index 1a7dc507c3..d56bacad2e 100644 --- a/plugins/mcp-actions-backend/src/services/McpService.test.ts +++ b/plugins/mcp-actions-backend/src/services/McpService.test.ts @@ -19,6 +19,7 @@ import { McpService } from './McpService'; import { actionsRegistryServiceMock, metricsServiceMock, + tracingServiceMock, } from '@backstage/backend-test-utils/alpha'; import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js'; import { Client } from '@modelcontextprotocol/sdk/client/index.js'; @@ -49,6 +50,7 @@ describe('McpService', () => { const mcpService = await McpService.create({ actions: mockActionsRegistry, metrics: mockMetrics, + tracingService: tracingServiceMock.mock(), }); const server = mcpService.getServer({ @@ -121,6 +123,7 @@ describe('McpService', () => { const mcpService = await McpService.create({ actions: mockActionsRegistry, metrics: mockMetrics, + tracingService: tracingServiceMock.mock(), }); const server = mcpService.getServer({ @@ -174,6 +177,7 @@ describe('McpService', () => { const mcpService = await McpService.create({ actions: mockActionsRegistry, metrics: mockMetrics, + tracingService: tracingServiceMock.mock(), }); const server = mcpService.getServer({ @@ -238,6 +242,7 @@ describe('McpService', () => { const mcpService = await McpService.create({ actions: actionsRegistryServiceMock(), metrics: mockMetrics, + tracingService: tracingServiceMock.mock(), }); const server = mcpService.getServer({ @@ -306,6 +311,7 @@ describe('McpService', () => { const mcpService = await McpService.create({ actions: mockActionsRegistry, metrics: mockMetrics, + tracingService: tracingServiceMock.mock(), }); const server = mcpService.getServer({ @@ -366,6 +372,7 @@ describe('McpService', () => { const mcpService = await McpService.create({ actions: mockActionsRegistry, metrics: metricsServiceMock.mock(), + tracingService: tracingServiceMock.mock(), }); const server = mcpService.getServer({ @@ -422,6 +429,7 @@ describe('McpService', () => { const mcpService = await McpService.create({ actions: mockActionsRegistry, metrics: metricsServiceMock.mock(), + tracingService: tracingServiceMock.mock(), }); const server = mcpService.getServer({ @@ -509,6 +517,7 @@ describe('McpService', () => { const mcpService = await McpService.create({ actions: fakeActionsService, metrics: metricsServiceMock.mock(), + tracingService: tracingServiceMock.mock(), }); const serverConfig: McpServerConfig = { @@ -542,6 +551,7 @@ describe('McpService', () => { const mcpService = await McpService.create({ actions: fakeActionsService, metrics: metricsServiceMock.mock(), + tracingService: tracingServiceMock.mock(), }); const serverConfig: McpServerConfig = { @@ -583,6 +593,7 @@ describe('McpService', () => { const mcpService = await McpService.create({ actions: fakeActionsService, metrics: metricsServiceMock.mock(), + tracingService: tracingServiceMock.mock(), }); const serverConfig: McpServerConfig = { @@ -621,6 +632,7 @@ describe('McpService', () => { const mcpService = await McpService.create({ actions: fakeActionsService, metrics: metricsServiceMock.mock(), + tracingService: tracingServiceMock.mock(), }); const serverConfig: McpServerConfig = { @@ -659,6 +671,7 @@ describe('McpService', () => { const mcpService = await McpService.create({ actions: fakeActionsService, metrics: metricsServiceMock.mock(), + tracingService: tracingServiceMock.mock(), }); const serverConfig: McpServerConfig = { @@ -711,6 +724,7 @@ describe('McpService', () => { const mcpService = await McpService.create({ actions: actionsRegistryServiceMock(), metrics: metricsServiceMock.mock(), + tracingService: tracingServiceMock.mock(), }); const server = mcpService.getServer({ @@ -734,6 +748,7 @@ describe('McpService', () => { const mcpService = await McpService.create({ actions: actionsRegistryServiceMock(), metrics: metricsServiceMock.mock(), + tracingService: tracingServiceMock.mock(), }); const server = mcpService.getServer({ @@ -763,6 +778,7 @@ describe('McpService', () => { const mcpService = await McpService.create({ actions: actionsRegistryServiceMock(), metrics: metricsServiceMock.mock(), + tracingService: tracingServiceMock.mock(), }); const server = mcpService.getServer({ @@ -805,6 +821,7 @@ describe('McpService', () => { const mcpService = await McpService.create({ actions: mockActionsRegistry, metrics: metricsServiceMock.mock(), + tracingService: tracingServiceMock.mock(), }); const server = mcpService.getServer({ @@ -843,6 +860,7 @@ describe('McpService', () => { const mcpService = await McpService.create({ actions: mockActionsRegistry, metrics: metricsServiceMock.mock(), + tracingService: tracingServiceMock.mock(), namespacedToolNames: false, }); @@ -882,6 +900,7 @@ describe('McpService', () => { const mcpService = await McpService.create({ actions: mockActionsRegistry, metrics: metricsServiceMock.mock(), + tracingService: tracingServiceMock.mock(), }); const server = mcpService.getServer({ @@ -907,4 +926,163 @@ describe('McpService', () => { expect(result.isError).toBeUndefined(); }); }); + + describe('tracing', () => { + async function invokeMockAction(opts: { + tracing: ReturnType; + captureToolPayloads?: boolean; + credentials?: + | ReturnType + | ReturnType; + }) { + const mockActionsRegistry = actionsRegistryServiceMock(); + mockActionsRegistry.register({ + name: 'mock-action', + title: 'Test', + description: 'Test', + schema: { + input: z => z.object({ input: z.string() }), + output: z => z.object({ output: z.string() }), + }, + action: async () => ({ output: { output: 'test' } }), + }); + + const mcpService = await McpService.create({ + actions: mockActionsRegistry, + metrics: metricsServiceMock.mock(), + tracingService: opts.tracing, + captureToolPayloads: opts.captureToolPayloads, + }); + + const server = mcpService.getServer({ + credentials: opts.credentials ?? mockCredentials.user(), + }); + + const client = new Client({ name: 'test client', version: '1.0' }); + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + await Promise.all([ + client.connect(clientTransport), + server.connect(serverTransport), + ]); + + return client.request( + { + method: 'tools/call', + params: { name: 'test.mock-action', arguments: { input: 'val' } }, + }, + CallToolResultSchema, + ); + } + + it('starts a tools/call span with spec attributes, server kind, and the request credentials', async () => { + const tracing = tracingServiceMock.mock(); + const credentials = mockCredentials.user(); + await invokeMockAction({ tracing, credentials }); + + expect(tracing.startActiveSpan).toHaveBeenCalledTimes(1); + const [name, , options] = tracing.startActiveSpan.mock.calls[0]; + expect(name).toBe('tools/call test.mock-action'); + expect(options?.kind).toBe('server'); + expect(options?.attributes).toEqual( + expect.objectContaining({ + 'mcp.method.name': 'tools/call', + 'gen_ai.tool.name': 'test.mock-action', + 'gen_ai.operation.name': 'execute_tool', + }), + ); + expect(options?.attributes).not.toHaveProperty( + 'gen_ai.tool.call.arguments', + ); + expect(options?.credentials).toBe(credentials); + expect(tracing.spans[0].setStatus).not.toHaveBeenCalled(); + }); + + it('overrides backstage.plugin.id on the span to match the action source plugin', async () => { + const tracing = tracingServiceMock.mock(); + await invokeMockAction({ tracing }); + + // The mock action is registered via actionsRegistryServiceMock(), + // which assigns pluginId 'test'. + expect(tracing.spans[0].setAttribute).toHaveBeenCalledWith( + 'backstage.plugin.id', + 'test', + ); + }); + + it('includes tool arguments in the span options and sets the result via setAttribute when captureToolPayloads is true', async () => { + const tracing = tracingServiceMock.mock(); + await invokeMockAction({ tracing, captureToolPayloads: true }); + + const [, , options] = tracing.startActiveSpan.mock.calls[0]; + expect(options?.attributes?.['gen_ai.tool.call.arguments']).toBe( + JSON.stringify({ input: 'val' }), + ); + + const span = tracing.spans[0]; + const resultCall = span.setAttribute.mock.calls.find( + ([key]) => key === 'gen_ai.tool.call.result', + ); + expect(resultCall).toBeDefined(); + const parsed = JSON.parse(resultCall![1] as string); + expect(parsed.content[0].type).toBe('text'); + expect(parsed.content[0].text).toContain('"output": "test"'); + }); + + it('sets error.type=tool_error and ERROR status on the span when the tool returns isError', async () => { + const tracing = tracingServiceMock.mock(); + const mockActionsRegistry = actionsRegistryServiceMock(); + mockActionsRegistry.register({ + name: 'failing-action', + title: 'Failing', + description: 'Throws InputError', + schema: { + input: z => z.object({ value: z.string() }), + output: z => z.object({}), + }, + action: async () => { + throw new InputError('the value was invalid'); + }, + }); + + const mcpService = await McpService.create({ + actions: mockActionsRegistry, + metrics: metricsServiceMock.mock(), + tracingService: tracing, + }); + + const server = mcpService.getServer({ + credentials: mockCredentials.user(), + }); + const client = new Client({ name: 'test client', version: '1.0' }); + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + await Promise.all([ + client.connect(clientTransport), + server.connect(serverTransport), + ]); + + const result = await client.request( + { + method: 'tools/call', + params: { + name: 'test.failing-action', + arguments: { value: 'test' }, + }, + }, + CallToolResultSchema, + ); + expect(result.isError).toBe(true); + + const span = tracing.spans[0]; + expect(span.setAttribute).toHaveBeenCalledWith( + 'error.type', + 'tool_error', + ); + expect(span.setStatus).toHaveBeenCalledWith({ + code: 'error', + message: 'tool_error', + }); + }); + }); }); diff --git a/plugins/mcp-actions-backend/src/services/McpService.ts b/plugins/mcp-actions-backend/src/services/McpService.ts index 49eea11d72..038151c5b6 100644 --- a/plugins/mcp-actions-backend/src/services/McpService.ts +++ b/plugins/mcp-actions-backend/src/services/McpService.ts @@ -25,6 +25,7 @@ import { ActionsServiceAction, MetricsServiceHistogram, MetricsService, + TracingService, } from '@backstage/backend-plugin-api/alpha'; import { version } from '@backstage/plugin-mcp-actions-backend/package.json'; import { NotFoundError } from '@backstage/errors'; @@ -34,18 +35,32 @@ import { handleErrors } from './handleErrors'; import { bucketBoundaries, McpServerOperationAttributes } from '../metrics'; import { FilterRule, McpServerConfig } from '../config'; +function safeStringify(value: unknown): string { + try { + return JSON.stringify(value); + } catch { + return String(value); + } +} + export class McpService { private readonly actions: ActionsService; private readonly namespacedToolNames: boolean; + private readonly tracingService: TracingService; + private readonly captureToolPayloads: boolean; private readonly operationDuration: MetricsServiceHistogram; constructor( actions: ActionsService, metrics: MetricsService, + tracingService: TracingService, namespacedToolNames?: boolean, + captureToolPayloads?: boolean, ) { this.actions = actions; this.namespacedToolNames = namespacedToolNames ?? true; + this.tracingService = tracingService; + this.captureToolPayloads = captureToolPayloads ?? false; this.operationDuration = metrics.createHistogram( 'mcp.server.operation.duration', @@ -60,13 +75,23 @@ export class McpService { static async create({ actions, metrics, + tracingService, namespacedToolNames, + captureToolPayloads, }: { actions: ActionsService; metrics: MetricsService; + tracingService: TracingService; namespacedToolNames?: boolean; + captureToolPayloads?: boolean; }) { - return new McpService(actions, metrics, namespacedToolNames); + return new McpService( + actions, + metrics, + tracingService, + namespacedToolNames, + captureToolPayloads, + ); } getServer({ @@ -136,43 +161,77 @@ export class McpService { let isError = false; try { - const result = await handleErrors(async () => { - const { actions: allActions } = await this.actions.list({ + return await this.tracingService.startActiveSpan( + `tools/call ${params.name}`, + async span => { + const result = await handleErrors(async () => { + const { actions: allActions } = await this.actions.list({ + credentials, + }); + const actions = serverConfig + ? this.filterActions(allActions, serverConfig) + : allActions; + + const action = actions.find( + a => this.getToolName(a) === params.name, + ); + + if (!action) { + throw new NotFoundError(`Action "${params.name}" not found`); + } + + // Re-attribute the span to the plugin that owns the action. + span.setAttribute('backstage.plugin.id', action.pluginId); + + const { output } = await this.actions.invoke({ + id: action.id, + input: params.arguments as JsonObject, + credentials, + }); + + return { + // todo(blam): unfortunately structuredContent is not supported by most clients yet. + // so the validation for the output happens in the default actions registry + // and we return it as json text instead for now. + content: [ + { + type: 'text', + text: [ + '```json', + JSON.stringify(output, null, 2), + '```', + ].join('\n'), + }, + ], + }; + }); + + isError = !!(result as { isError?: boolean })?.isError; + if (this.captureToolPayloads) { + span.setAttribute( + 'gen_ai.tool.call.result', + safeStringify(result), + ); + } + if (isError) { + span.setAttribute('error.type', 'tool_error'); + span.setStatus({ code: 'error', message: 'tool_error' }); + } + return result; + }, + { + kind: 'server', credentials, - }); - const actions = serverConfig - ? this.filterActions(allActions, serverConfig) - : allActions; - - const action = actions.find(a => this.getToolName(a) === params.name); - - if (!action) { - throw new NotFoundError(`Action "${params.name}" not found`); - } - - const { output } = await this.actions.invoke({ - id: action.id, - input: params.arguments as JsonObject, - credentials, - }); - - return { - // todo(blam): unfortunately structuredContent is not supported by most clients yet. - // so the validation for the output happens in the default actions registry - // and we return it as json text instead for now. - content: [ - { - type: 'text', - text: ['```json', JSON.stringify(output, null, 2), '```'].join( - '\n', - ), - }, - ], - }; - }); - - isError = !!(result as { isError?: boolean })?.isError; - return result; + attributes: { + 'mcp.method.name': 'tools/call', + 'gen_ai.tool.name': params.name, + 'gen_ai.operation.name': 'execute_tool', + ...(this.captureToolPayloads && { + 'gen_ai.tool.call.arguments': safeStringify(params.arguments), + }), + }, + }, + ); } catch (err) { errorType = err instanceof Error ? err.name : 'Error'; throw err;