@W-23146707 Add telemetry for errors and URL button clicks in MCP app

.gitignore

@@ -39,3 +39,5 @@ src/web/apps/dist/
 .work/reports/*.md
 !.work/specs/TEMPLATE.md
 !.work/**/.gitkeep
+
+.worktrees/

package.json

@@ -1,7 +1,7 @@
 {
   "name": "@tableau/mcp-server",
   "description": "Helping agents see and understand data.",
-  "version": "3.5.2",
+  "version": "3.5.3",
   "repository": {
     "type": "git",
     "url": "git+https://github.com/tableau/tableau-mcp.git"

src/server/oauth/scopes.ts

@@ -297,6 +297,11 @@ const toolScopeMap: Record<
     mcp: [],
     api: new Set<TableauApiScope>(),
   },
+  // MCP-app event telemetry relay: no Tableau REST API calls, no content scope required.
+  'record-event': {
+    mcp: [],
+    api: new Set<TableauApiScope>(),
+  },
   // Dispatches on `kind` to ts-events, site-content, job-performance (raw VDS) or stale-content
   // (server-side anti-join). Union of the scopes required by all four kinds.
   'query-admin-insights': {
@@ -362,6 +367,7 @@ async function getEnabledToolNames(): Promise<Set<WebToolName>> {
   // human-gesture confirm steps for their preview tools and only exist when the iframe can render.
   if (!mcpAppsEnabled) {
     enabledTools.delete('get-embed-token');
+    enabledTools.delete('record-event');
     enabledTools.delete('confirm-update-cloud-extract-refresh-task');
     enabledTools.delete('confirm-delete-content');
   }

src/server/passthroughAuthMiddleware.test.ts

@@ -21,6 +21,9 @@ const TOOLS_WITHOUT_API_SCOPES_WITH_PASSTHROUGH_GUARD: ReadonlyArray<WebToolName
   // Consent lifecycle tool: no Tableau REST API call. The tool callback explicitly returns an error
   // for non-Bearer auth types, so passthrough callers are rejected.
   'reset-consent',
+  // MCP app event telemetry: no Tableau REST API call. App-only tool that records telemetry
+  // regardless of auth type. Safe with passthrough auth as it only sends telemetry events.
+  'record-event',
 ];
 
 describe('passthroughAuthMiddleware', () => {

src/telemetry/productTelemetry/telemetryForwarder.ts

@@ -7,7 +7,7 @@ type PropertiesType = { [key: string]: ValidPropertyValueType };
 const DEFAULT_HOST_NAME = 'External';
 const SERVICE_NAME = 'tableau-mcp';
 
-export type TelemetryEventType = 'tool_call';
+export type TelemetryEventType = 'tool_call' | 'tableau_mcp_event';
 
 export type ProductTelemetryBase = {
   endpoint: string;

src/tools/web/recordEvent/recordEvent.test.ts

@@ -0,0 +1,123 @@
+import { CallToolResult } from '@modelcontextprotocol/sdk/types.js';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import { z } from 'zod';
+
+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: '' }),
+    );
+  });
+
+  it('accepts SCREAMING_SNAKE_CASE event_type values', async () => {
+    const schema = z.object(
+      await Provider.from(getRecordEventTool(new WebMcpServer()).paramsSchema),
+    );
+    for (const event_type of [
+      'TOOL_ERROR',
+      'PARSE_ERROR',
+      'AUTH_ERROR',
+      'EMBED_LOAD_ERROR',
+      'MCP_APP_CLICKED',
+    ]) {
+      expect(schema.safeParse({ event_type }).success).toBe(true);
+    }
+  });
+
+  it('rejects event_type that is too long or not SCREAMING_SNAKE_CASE', async () => {
+    const schema = z.object(
+      await Provider.from(getRecordEventTool(new WebMcpServer()).paramsSchema),
+    );
+    expect(schema.safeParse({ event_type: 'A'.repeat(65) }).success).toBe(false); // too long
+    expect(schema.safeParse({ event_type: 'tool_error' }).success).toBe(false); // lowercase
+    expect(schema.safeParse({ event_type: 'TOOL ERROR' }).success).toBe(false); // spaces
+    expect(schema.safeParse({ event_type: '1TOOL_ERROR' }).success).toBe(false); // leading digit
+    expect(schema.safeParse({ event_type: '_TOOL_ERROR' }).success).toBe(false); // leading underscore
+  });
+
+  it('truncates message longer than 1024 characters in the forwarded event', async () => {
+    const extra = getMockRequestHandlerExtra();
+    const longMessage = 'x'.repeat(2000);
+    await getToolResult(extra, { event_type: 'TOOL_ERROR', message: longMessage });
+
+    const sentMessage = sendSpy.mock.calls.find((c) => c[0] === 'tableau_mcp_event')?.[1]?.message;
+    expect(sentMessage).toBe('x'.repeat(1024));
+    expect(sentMessage.length).toBe(1024);
+  });
+});
+
+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);
+  // Mirror the MCP framework: params are validated/transformed against paramsSchema before the
+  // callback runs, so route args through the schema here (this is where message truncation happens).
+  const parsedArgs = z.object(await Provider.from(tool.paramsSchema)).parse(args);
+  return await callback({ event_type: parsedArgs.event_type, message: parsedArgs.message }, extra);
+}

src/tools/web/recordEvent/recordEvent.ts

@@ -0,0 +1,86 @@
+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 = {
+  // Bounded free-form string rather than a hard enum: the event-type set is intentionally
+  // app-extensible (see above), so we reject malformed values but not unknown-yet-valid ones.
+  event_type: z
+    .string()
+    .max(64)
+    .regex(/^[A-Z][A-Z0-9_]*$/, 'event_type must be SCREAMING_SNAKE_CASE (e.g. TOOL_ERROR).')
+    .describe(
+      'The event type for product telemetry, e.g. TOOL_ERROR, PARSE_ERROR, AUTH_ERROR, EMBED_LOAD_ERROR, MCP_APP_CLICKED.',
+    ),
+  // Optional free-text detail: truncate rather than reject so an over-long message never fails
+  // the telemetry call (mirrors the length cap in src/telemetry/clientDisplayName.ts).
+  message: z
+    .string()
+    .transform((s) => s.slice(0, 1024))
+    .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;
+};

src/tools/web/tool.test.ts

@@ -230,6 +230,7 @@ describe('Tool', () => {
           is_hyperforce: false,
           success: true,
           error_code: '',
+          error_message: '',
         }),
       );
     });
@@ -252,6 +253,7 @@ describe('Tool', () => {
           is_hyperforce: false,
           success: false,
           error_code: '500',
+          error_message: 'requestId: 2, error: Callback failed',
         }),
       );
     });
@@ -582,6 +584,16 @@ describe('Tool', () => {
       const parsed = JSON.parse(result.content[0].text);
       expect(parsed.data).toEqual(rawApiData.toString());
       expect(parsed.warning).toContain('Expected string, received object');
+
+      // The passthrough result carries the full API payload but is isError: false, so it must
+      // NOT leak into telemetry's error_message (keyed off isError, not the false `success`).
+      expect(mockTelemetrySend).toHaveBeenCalledWith(
+        'tool_call',
+        expect.objectContaining({
+          success: false,
+          error_message: '',
+        }),
+      );
     });
 
     it('should return isError: false with validation warning for discriminatedUnion schema errors', async () => {

src/tools/web/tool.ts

@@ -12,6 +12,7 @@ import {
 } from '../../telemetry/clientDisplayName.js';
 import { getTelemetryProvider } from '../../telemetry/init.js';
 import { getProductTelemetry } from '../../telemetry/productTelemetry/telemetryForwarder.js';
+import { extractToolErrorMessage } from '../../utils/extractToolErrorMessage.js';
 import { getExceptionMessage } from '../../utils/getExceptionMessage.js';
 import { getHttpStatus } from '../../utils/getHttpStatus.js';
 import { LogAndExecuteParams, Tool, ToolParams } from '../tool.js';
@@ -162,7 +163,7 @@ export class WebTool<Args extends ZodRawShape | undefined = undefined> extends T
 
     let success = false;
     let errorCode = ''; // HTTP status category: "4xx", "5xx", or empty for successful calls
-    let toolResult: CallToolResult;
+    let toolResult: CallToolResult | undefined;
 
     try {
       const result = await callback();
@@ -232,6 +233,10 @@ export class WebTool<Args extends ZodRawShape | undefined = undefined> extends T
         is_hyperforce: config.isHyperforce,
         success,
         error_code: errorCode,
+        // Only populated for genuine error results (isError: true). The ZodiosValidationError
+        // passthrough returns isError: false with the full API payload, so keying off isError
+        // (not !success) keeps successful response data out of telemetry.
+        error_message: toolResult?.isError ? extractToolErrorMessage(toolResult) : '',
         oauth_client_id: sanitizeClientIdForTelemetry(oauthClientId),
         oauth_client_display_name:
           getClientDisplayName(oauthClientId) ?? sanitizeClientIdForTelemetry(oauthClientId),

src/tools/web/toolName.ts

@@ -14,6 +14,7 @@ export const webToolNames = [
   'get-datasource-metadata',
   'resolve-datasource-luid',
   'get-embed-token',
+  'record-event',
   'get-workbook',
   'get-view',
   'get-flow',
@@ -52,6 +53,7 @@ export const webToolGroupNames = [
   'jobs',
   'users',
   'token-management',
+  'mcp-apps',
   'admin-insights',
   'content',
 ] as const;
@@ -94,7 +96,8 @@ export const webToolGroups = {
   ],
   jobs: ['list-jobs'],
   users: ['list-users', 'update-user'],
-  'token-management': ['get-embed-token', 'revoke-access-token', 'reset-consent'],
+  'token-management': ['revoke-access-token', 'reset-consent'],
+  'mcp-apps': ['get-embed-token', 'record-event'],
   'admin-insights': ['query-admin-insights'],
   content: ['delete-content', 'confirm-delete-content'],
 } as const satisfies Record<WebToolGroupName, Array<WebToolName>>;

src/tools/web/tools.ts

@@ -22,6 +22,7 @@ import { getListPulseMetricsFromMetricDefinitionIdTool } from './pulse/listMetri
 import { getListPulseMetricsFromMetricIdsTool } from './pulse/listMetricsFromMetricIds/listPulseMetricsFromMetricIds.js';
 import { getListPulseMetricSubscriptionsTool } from './pulse/listMetricSubscriptions/listPulseMetricSubscriptions.js';
 import { getQueryDatasourceTool } from './queryDatasource/queryDatasource.js';
+import { getRecordEventTool } from './recordEvent/recordEvent.js';
 import { getResetConsentTool } from './resetConsent/resetConsent.js';
 import { getRevokeAccessTokenTool } from './revokeAccessToken/revokeAccessToken.js';
 import { getListUsersTool } from './users/listUsers.js';
@@ -39,6 +40,7 @@ import { getListWorkbooksTool } from './workbooks/listWorkbooks.js';
 export const webToolFactories = [
   getGetDatasourceMetadataTool,
   getEmbedTokenTool,
+  getRecordEventTool,
   getListDatasourcesTool,
   getResolveDatasourceLuidTool,
   getListExtractRefreshTasksTool,