-
Notifications
You must be signed in to change notification settings - Fork 127
@W-23146707 Add telemetry for errors and URL button clicks in MCP app #530
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
Merged
Merged
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
3eeddd6
Record tableau_mcp_event.completed telemetry on MCP app errors
jarhun88 950c0c1
Make MCP app error screen background fully white
jarhun88 781bcd6
Generalize MCP app telemetry to record-event and record link clicks
jarhun88 4c3ed41
Merge branch 'main' into dev/feat/mcp-app-error-telemetry
jarhun88 646a1ac
Bump version to 3.3.0
jarhun88 5479e52
Forward HITL confirm tool-error message to telemetry via extractToolE…
jarhun88 2adb821
@W-23146707: Move app-only tools into a dedicated mcp-apps tool group
jarhun88 5148268
@W-23146707: Validate and bound record-event telemetry input
jarhun88 a39c445
Merge remote-tracking branch 'origin/main' into dev/feat/mcp-app-erro…
jarhun88 5d4ad32
@W-23146707: Bump patch version to 3.5.3
jarhun88 fad9e47
@W-23146707: Ignore .worktrees/ scratch directory
jarhun88 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| import { CallToolResult } from '@modelcontextprotocol/sdk/types.js'; | ||
| import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; | ||
|
|
||
| import { WebMcpServer } from '../../../server.web.js'; | ||
| import { Provider } from '../../../utils/provider.js'; | ||
| import { getMockRequestHandlerExtra } from '../toolContext.mock.js'; | ||
| import { getRecordEventTool } from './recordEvent.js'; | ||
|
|
||
| // Mock getProductTelemetry so we can assert on the forwarder's send(). Note that | ||
| // WebTool.logAndExecute also emits an automatic 'tool_call' event through the same | ||
| // forwarder, so the spy is called for both 'tool_call' and 'tableau_mcp_event'. | ||
| vi.mock('../../../telemetry/productTelemetry/telemetryForwarder.js', async (importOriginal) => { | ||
| const actual = | ||
| await importOriginal< | ||
| typeof import('../../../telemetry/productTelemetry/telemetryForwarder.js') | ||
| >(); | ||
| return { ...actual, getProductTelemetry: vi.fn() }; | ||
| }); | ||
|
|
||
| import { getProductTelemetry } from '../../../telemetry/productTelemetry/telemetryForwarder.js'; | ||
|
|
||
| type Extra = ReturnType<typeof getMockRequestHandlerExtra>; | ||
|
|
||
| describe('getRecordEventTool', () => { | ||
| let sendSpy: ReturnType<typeof vi.fn>; | ||
|
|
||
| beforeEach(() => { | ||
| sendSpy = vi.fn(); | ||
| vi.mocked(getProductTelemetry).mockReturnValue({ send: sendSpy } as never); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| it('should create a tool instance with correct properties', async () => { | ||
| const tool = getRecordEventTool(new WebMcpServer()); | ||
| const annotations = await Provider.from(tool.annotations); | ||
| expect(tool.name).toBe('record-event'); | ||
| expect(annotations?.readOnlyHint).toBe(true); | ||
| expect(annotations?.openWorldHint).toBe(false); | ||
| }); | ||
|
|
||
| it('should set visibility to app-only', () => { | ||
| const tool = getRecordEventTool(new WebMcpServer()); | ||
| expect(tool.meta?.ui?.visibility).toEqual(['app']); | ||
| }); | ||
|
|
||
| it('sends an tableau_mcp_event event with the event_type, message and server context', async () => { | ||
| const extra = getMockRequestHandlerExtra(); | ||
| const result = await getToolResult(extra, { event_type: 'PARSE_ERROR', message: 'bad json' }); | ||
|
|
||
| expect(result.isError).toBe(false); | ||
| expect(sendSpy).toHaveBeenCalledWith( | ||
| 'tableau_mcp_event', | ||
| expect.objectContaining({ | ||
| event_type: 'PARSE_ERROR', | ||
| message: 'bad json', | ||
| podname: extra.config.server, | ||
| is_hyperforce: extra.config.isHyperforce, | ||
| }), | ||
| ); | ||
| }); | ||
|
|
||
| it('defaults message to empty string when omitted', async () => { | ||
| const extra = getMockRequestHandlerExtra(); | ||
| await getToolResult(extra, { event_type: 'EMBED_LOAD_ERROR', message: undefined }); | ||
|
|
||
| expect(sendSpy).toHaveBeenCalledWith( | ||
| 'tableau_mcp_event', | ||
| expect.objectContaining({ event_type: 'EMBED_LOAD_ERROR', message: '' }), | ||
| ); | ||
| }); | ||
| }); | ||
|
|
||
| async function getToolResult( | ||
| extra: Extra, | ||
| args: { event_type: string; message: string | undefined }, | ||
| ): Promise<CallToolResult> { | ||
| const tool = getRecordEventTool(new WebMcpServer()); | ||
| const callback = await Provider.from(tool.callback); | ||
| return await callback(args, extra); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| import { CallToolResult } from '@modelcontextprotocol/sdk/types.js'; | ||
| import { Ok } from 'ts-results-es'; | ||
| import { z } from 'zod'; | ||
|
|
||
| import { getFeatureGate } from '../../../features/init.js'; | ||
| import { WebMcpServer } from '../../../server.web.js'; | ||
| import { getProductTelemetry } from '../../../telemetry/productTelemetry/telemetryForwarder.js'; | ||
| import { WebTool } from '../tool.js'; | ||
|
|
||
| // Starting field set — the final app-supplied schema is expected to grow later. | ||
| const paramsSchema = { | ||
| event_type: z | ||
| .string() | ||
| .describe( | ||
| 'The event type for product telemetry, e.g. TOOL_ERROR, PARSE_ERROR, AUTH_ERROR, EMBED_LOAD_ERROR, MCP_APP_CLICKED.', | ||
| ), | ||
| message: z.string().optional().describe('Optional detail or context for the event.'), | ||
| }; | ||
|
|
||
| /** | ||
| * Records a product-telemetry event from the MCP app UI (errors, user actions, etc.). | ||
| * Called by the app (never the model) via app.callServerTool. Mirrors the | ||
| * server-side 'tool_call' telemetry pattern, enriching the event with request | ||
| * context the browser bundle does not have. | ||
| */ | ||
| export const getRecordEventTool = (server: WebMcpServer): WebTool<typeof paramsSchema> => { | ||
| const recordEventTool = new WebTool({ | ||
| server, | ||
| name: 'record-event', | ||
| description: | ||
| 'Records a product-telemetry event from the MCP app UI (errors, user actions, etc.). This tool is only visible to the app, never the model. It takes an event type and optional detail, forwards a telemetry event, and returns immediately.', | ||
| paramsSchema, | ||
| annotations: { | ||
| title: 'Record Event', | ||
| readOnlyHint: true, | ||
| destructiveHint: false, | ||
| idempotentHint: true, | ||
| openWorldHint: false, | ||
| }, | ||
| meta: { | ||
| ui: { | ||
| visibility: ['app'], // Only visible to the app, not the model | ||
| }, | ||
| }, | ||
| disabled: !getFeatureGate().isFeatureEnabled('mcp-apps'), | ||
| callback: async (args, extra): Promise<CallToolResult> => { | ||
| return recordEventTool.logAndExecute<{ recorded: true }>({ | ||
| extra, | ||
| args, | ||
| callback: async () => { | ||
| const { config } = extra; | ||
|
|
||
| const productTelemetryForwarder = getProductTelemetry( | ||
| config.productTelemetryEndpoint, | ||
| config.productTelemetryEnabled, | ||
| config.server, | ||
| ); | ||
|
|
||
| productTelemetryForwarder.send('tableau_mcp_event', { | ||
| event_type: args.event_type, | ||
| message: args.message ?? '', | ||
| site_luid: extra.getSiteLuid(), | ||
| user_luid: extra.getUserLuid(), | ||
| podname: config.server, | ||
| is_hyperforce: config.isHyperforce, | ||
| }); | ||
|
|
||
| return Ok({ recorded: true as const }); | ||
| }, | ||
| constrainSuccessResult: (result) => ({ type: 'success', result }), | ||
| }); | ||
| }, | ||
| }); | ||
|
|
||
| return recordEventTool; | ||
| }; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| import type { CallToolResult } from '@modelcontextprotocol/sdk/types.js'; | ||
|
|
||
| import { extractToolErrorMessage } from './extractToolErrorMessage.js'; | ||
|
|
||
| describe('extractToolErrorMessage', () => { | ||
| it('returns the text of a single text content block', () => { | ||
| const result: CallToolResult = { | ||
| isError: true, | ||
| content: [{ type: 'text', text: 'Request failed with status code 404' }], | ||
| }; | ||
|
|
||
| expect(extractToolErrorMessage(result)).toBe('Request failed with status code 404'); | ||
| }); | ||
|
|
||
| it('joins multiple text blocks with a space', () => { | ||
| const result: CallToolResult = { | ||
| isError: true, | ||
| content: [ | ||
| { type: 'text', text: 'first' }, | ||
| { type: 'text', text: 'second' }, | ||
| ], | ||
| }; | ||
|
|
||
| expect(extractToolErrorMessage(result)).toBe('first second'); | ||
| }); | ||
|
|
||
| it('ignores non-text content blocks', () => { | ||
| const result = { | ||
| isError: true, | ||
| content: [ | ||
| { type: 'image', data: 'abc', mimeType: 'image/png' }, | ||
| { type: 'text', text: 'only this' }, | ||
| ], | ||
| } as unknown as CallToolResult; | ||
|
|
||
| expect(extractToolErrorMessage(result)).toBe('only this'); | ||
| }); | ||
|
|
||
| it('returns an empty string when there are no text blocks', () => { | ||
| const result = { | ||
| isError: true, | ||
| content: [{ type: 'image', data: 'abc', mimeType: 'image/png' }], | ||
| } as unknown as CallToolResult; | ||
|
|
||
| expect(extractToolErrorMessage(result)).toBe(''); | ||
| }); | ||
|
|
||
| it('returns an empty string when the text is empty or whitespace only', () => { | ||
| const result: CallToolResult = { | ||
| isError: true, | ||
| content: [{ type: 'text', text: ' ' }], | ||
| }; | ||
|
|
||
| expect(extractToolErrorMessage(result)).toBe(''); | ||
| }); | ||
|
|
||
| it('returns an empty string when content is not an array', () => { | ||
| const result = { isError: true, content: undefined } as unknown as CallToolResult; | ||
|
|
||
| expect(extractToolErrorMessage(result)).toBe(''); | ||
| }); | ||
| }); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.