feat(telemetry): log devtools data. (#2460)

This puts relevant logs fields into the `ToolInvocationContext` object,
and moves construction logic all into `src/telemetry/transformation.ts`.

Objects are only logged if they have at least one attribute, following
existing convention.

Tests are refactored accordingly.

src/ToolHandler.ts

@@ -11,7 +11,7 @@ import type {DataFormat} from './McpResponse.js';
 import {McpResponse} from './McpResponse.js';
 import {SlimMcpResponse} from './SlimMcpResponse.js';
 import {ClearcutLogger} from './telemetry/ClearcutLogger.js';
-import {bucketizeLatency} from './telemetry/transformation.js';
+import {bucketizeLatency, buildContext} from './telemetry/transformation.js';
 import type {CallToolResult} from './third_party/index.js';
 import {zod} from './third_party/index.js';
 import type {ToolCategory} from './tools/categories.js';
@@ -316,17 +316,14 @@ export class ToolHandler {
         isError: true,
       };
     } finally {
-      const isDevToolsOpen = devToolsData
-        ? Object.keys(devToolsData).length > 0
-        : undefined;
+      const context = buildContext(devToolsData, pageUrl);
       void ClearcutLogger.get()?.logToolInvocation({
         toolName: this.tool.name,
         params,
         schema: this.inputSchema,
         success,
         latencyMs: bucketizeLatency(Date.now() - startTime),
-        isDevToolsOpen,
-        pageUrl,
+        context,
       });
       guard[Symbol.dispose]();
     }

src/telemetry/ClearcutLogger.ts

@@ -9,7 +9,6 @@ import process from 'node:process';
 import {DAEMON_CLIENT_NAME} from '../daemon/utils.js';
 import type {zod, ShapeOutput} from '../third_party/index.js';
 import {logger} from '../utils/logger.js';
-import {isLocalhost} from '../utils/url.js';
 
 import type {ErrorCode} from './errors.js';
 import type {LocalState, Persistence} from './persistence.js';
@@ -114,8 +113,7 @@ export class ClearcutLogger {
     schema: zod.ZodRawShape;
     success: boolean;
     latencyMs: number;
-    isDevToolsOpen?: boolean;
-    pageUrl?: string;
+    context: ToolInvocationContext;
   }): Promise<void> {
     const sanitizedToolName = stripUnderscoreBeforeNumber(args.toolName);
     const tool_invocation: ToolInvocation = {
@@ -123,15 +121,8 @@ export class ClearcutLogger {
       success: args.success,
       latency_ms: args.latencyMs,
     };
-    const context: ToolInvocationContext = {};
-    if (args.isDevToolsOpen !== undefined) {
-      context.is_devtools_open = args.isDevToolsOpen;
-    }
-    if (args.pageUrl !== undefined) {
-      context.is_localhost = isLocalhost(args.pageUrl);
-    }
-    if (Object.keys(context).length > 0) {
-      tool_invocation.context = context;
+    if (Object.keys(args.context).length > 0) {
+      tool_invocation.context = args.context;
     }
     if (Object.keys(args.params).length > 0) {
       tool_invocation.tool_params = {

src/telemetry/transformation.ts

@@ -5,6 +5,10 @@
  */
 
 import type {zod, ShapeOutput} from '../third_party/index.js';
+import type {DevToolsData} from '../tools/ToolDefinition.js';
+import {isLocalhost} from '../utils/url.js';
+
+import type {LoggedDevToolsData, ToolInvocationContext} from './types.js';
 
 const LATENCY_BUCKETS = [50, 100, 250, 500, 1000, 2500, 5000, 10000];
 
@@ -182,3 +186,39 @@ export function sanitizeParams(
   }
   return transformed;
 }
+
+function transformDevToolsData(devToolsData: DevToolsData): LoggedDevToolsData {
+  const logged: LoggedDevToolsData = {};
+  if (devToolsData.cdpBackendNodeId !== undefined) {
+    logged.is_dom_element_selected = true;
+  }
+  if (devToolsData.cdpRequestId !== undefined) {
+    logged.is_network_request_selected = true;
+  }
+  return logged;
+}
+
+export function buildContext(
+  devToolsData?: DevToolsData,
+  pageUrl?: string,
+): ToolInvocationContext {
+  let context: ToolInvocationContext;
+  if (devToolsData === undefined) {
+    context = {is_devtools_open: false};
+  } else {
+    context = {
+      is_devtools_open: Object.keys(devToolsData).length > 0,
+    };
+
+    const loggedDevtoolsData = transformDevToolsData(devToolsData);
+    if (Object.keys(loggedDevtoolsData).length > 0) {
+      context.devtools_data = loggedDevtoolsData;
+    }
+  }
+
+  if (pageUrl !== undefined) {
+    context.is_localhost = isLocalhost(pageUrl);
+  }
+
+  return context;
+}

src/telemetry/types.ts

@@ -26,9 +26,15 @@ export interface ServerError {
 
 export type ServerShutdown = Record<string, never>;
 
+export interface LoggedDevToolsData {
+  is_dom_element_selected?: boolean;
+  is_network_request_selected?: boolean;
+}
+
 export interface ToolInvocationContext {
   is_devtools_open?: boolean;
   is_localhost?: boolean;
+  devtools_data?: LoggedDevToolsData;
 }
 
 export interface ToolInvocation {

tests/ToolHandler.test.ts

@@ -110,12 +110,10 @@ describe('ToolHandler', () => {
     assert.strictEqual(result.isError, undefined);
   });
 
-  it('logs isDevToolsOpen telemetry based on getDevToolsData', async () => {
-    let handlerCalled = false;
-    const tool: DefinedPageTool = {
-      name: 'page_tool',
-      description: 'A page tool',
-      pageScoped: true,
+  it('appends correct context to tool call logs', async () => {
+    const baseTool: ToolDefinition = {
+      name: 'test_tool',
+      description: 'A test tool',
       annotations: {
         category: ToolCategory.NAVIGATION,
         readOnlyHint: true,
@@ -124,46 +122,92 @@ describe('ToolHandler', () => {
       blockedByDialog: false,
       verifyFilesSchema: [],
       handler: async () => {
-        handlerCalled = true;
+        return;
       },
     };
 
-    const mockContext = sinon.createStubInstance(McpContext);
-    mockContext.getDevToolsData.resolves({cdpBackendNodeId: 1});
-    const mockPage = {
-      pptrPage: {
-        isClosed: () => false,
-        url: () => 'http://localhost:9222/',
+    const testCases: Array<{
+      tool: ToolDefinition | DefinedPageTool;
+      devToolsData: Record<string, unknown>;
+      pageUrl?: string;
+      expectedContext: Record<string, unknown>;
+    }> = [
+      {
+        tool: {
+          ...baseTool,
+          name: 'page_tool',
+          pageScoped: true,
+        },
+        devToolsData: {cdpBackendNodeId: 1},
+        pageUrl: 'http://localhost:9222/',
+        expectedContext: {
+          is_devtools_open: true,
+          is_localhost: true,
+          devtools_data: {
+            is_dom_element_selected: true,
+          },
+        },
       },
-    } as unknown as McpPage;
-    mockContext.getSelectedMcpPage.returns(mockPage);
+      {
+        tool: {
+          ...baseTool,
+          name: 'global_tool',
+        },
+        devToolsData: {},
+        pageUrl: undefined,
+        expectedContext: {
+          is_devtools_open: false,
+        },
+      },
+    ];
 
-    const logSpy = sinon.spy();
-    sinon.stub(ClearcutLogger, 'get').returns({
-      logToolInvocation: logSpy,
-    } as unknown as ClearcutLogger);
+    for (const testCase of testCases) {
+      let handlerCalled = false;
+      testCase.tool.handler = async () => {
+        handlerCalled = true;
+      };
 
-    const toolMutex = new Mutex();
-    const serverArgs = parseArguments('1.0.0', ['node', 'script.js'], {
-      CHROME_DEVTOOLS_MCP_NO_USAGE_STATISTICS: 'true',
-    });
+      const mockContext = sinon.createStubInstance(McpContext);
+      mockContext.getDevToolsData.resolves(testCase.devToolsData);
+      if (testCase.pageUrl) {
+        const mockPage = {
+          pptrPage: {
+            isClosed: () => false,
+            url: () => testCase.pageUrl,
+          },
+        } as unknown as McpPage;
+        mockContext.getSelectedMcpPage.returns(mockPage);
+      }
 
-    const toolHandler = new ToolHandler(
-      tool,
-      serverArgs,
-      async () => mockContext,
-      toolMutex,
-    );
+      const logSpy = sinon.spy();
+      sinon.stub(ClearcutLogger, 'get').returns({
+        logToolInvocation: logSpy,
+      } as unknown as ClearcutLogger);
 
-    await toolHandler.handle({});
+      const toolMutex = new Mutex();
+      const serverArgs = parseArguments('1.0.0', ['node', 'script.js'], {
+        CHROME_DEVTOOLS_MCP_NO_USAGE_STATISTICS: 'true',
+      });
 
-    assert.strictEqual(logSpy.calledOnce, true);
-    assert.strictEqual(logSpy.firstCall.args[0].isDevToolsOpen, true);
-    assert.strictEqual(
-      logSpy.firstCall.args[0].pageUrl,
-      'http://localhost:9222/',
-    );
-    assert.strictEqual(handlerCalled, true);
+      const toolHandler = new ToolHandler(
+        testCase.tool,
+        serverArgs,
+        async () => mockContext,
+        toolMutex,
+      );
+
+      await toolHandler.handle({});
+
+      assert.strictEqual(logSpy.calledOnce, true);
+      assert.deepStrictEqual(
+        logSpy.firstCall.args[0].context,
+        testCase.expectedContext,
+      );
+      assert.strictEqual(handlerCalled, true);
+
+      sinon.restore();
+      ClearcutLogger.resetForTesting();
+    }
   });
 
   it('reports unknown registered tool arguments clearly', async () => {

tests/telemetry/ClearcutLogger.test.ts

@@ -50,6 +50,7 @@ describe('ClearcutLogger', () => {
         schema: {},
         success: true,
         latencyMs: 123,
+        context: {},
       });
 
       assert(mockWatchdogClient.send.calledOnce);
@@ -59,7 +60,7 @@ describe('ClearcutLogger', () => {
       assert.strictEqual(msg.payload.tool_invocation?.success, true);
       assert.strictEqual(msg.payload.tool_invocation?.latency_ms, 123);
     });
-    it('sends context with is_devtools_open', async () => {
+    it('sends context when provided', async () => {
       const logger = ClearcutLogger.initialize({
         persistence: mockPersistence,
         appVersion: '1.0.0',
@@ -71,7 +72,10 @@ describe('ClearcutLogger', () => {
         schema: {},
         success: true,
         latencyMs: 123,
-        isDevToolsOpen: true,
+        context: {
+          is_devtools_open: true,
+          is_localhost: false,
+        },
       });
 
       assert(mockWatchdogClient.send.calledOnce);
@@ -79,37 +83,9 @@ describe('ClearcutLogger', () => {
       assert.strictEqual(msg.type, WatchdogMessageType.LOG_EVENT);
       assert.deepStrictEqual(msg.payload.tool_invocation?.context, {
         is_devtools_open: true,
+        is_localhost: false,
       });
     });
-    it('sends context with correct is_localhost based on the URL', async () => {
-      const logger = ClearcutLogger.initialize({
-        persistence: mockPersistence,
-        appVersion: '1.0.0',
-        watchdogClient: mockWatchdogClient,
-      });
-
-      for (const {pageUrl, isLocalhost} of [
-        {pageUrl: 'http://localhost:9222/test', isLocalhost: true},
-        {pageUrl: 'https://example.com/test', isLocalhost: false},
-      ]) {
-        mockWatchdogClient.send.resetHistory();
-        await logger.logToolInvocation({
-          toolName: 'test_tool',
-          params: {},
-          schema: {},
-          success: true,
-          latencyMs: 123,
-          pageUrl,
-        });
-
-        assert(mockWatchdogClient.send.calledOnce);
-        const msg = mockWatchdogClient.send.firstCall.args[0];
-        assert.strictEqual(msg.type, WatchdogMessageType.LOG_EVENT);
-        assert.deepStrictEqual(msg.payload.tool_invocation?.context, {
-          is_localhost: isLocalhost,
-        });
-      }
-    });
     it('sends sanitized params', async () => {
       const logger = ClearcutLogger.initialize({
         persistence: mockPersistence,
@@ -133,6 +109,7 @@ describe('ClearcutLogger', () => {
         schema,
         success: true,
         latencyMs: 123,
+        context: {},
       });
 
       assert(mockWatchdogClient.send.calledOnce);

tests/telemetry/transformation.test.ts

@@ -9,6 +9,7 @@ import {describe, it} from 'node:test';
 
 import {
   bucketizeLatency,
+  buildContext,
   getEnumValues,
   sanitizeParams,
   stripUnderscoreBeforeNumber,
@@ -198,3 +199,57 @@ describe('transformArgName', () => {
     );
   });
 });
+
+describe('buildContext', () => {
+  it('should set is_devtools_open based on devToolsData', () => {
+    assert.deepStrictEqual(buildContext(undefined, undefined), {
+      is_devtools_open: false,
+    });
+    assert.deepStrictEqual(buildContext({}, undefined), {
+      is_devtools_open: false,
+    });
+    assert.deepStrictEqual(buildContext({cdpBackendNodeId: 1}, undefined), {
+      is_devtools_open: true,
+      devtools_data: {
+        is_dom_element_selected: true,
+      },
+    });
+  });
+
+  it('should set is_localhost based on pageUrl', () => {
+    assert.deepStrictEqual(
+      buildContext(undefined, 'http://localhost:9222/test'),
+      {
+        is_devtools_open: false,
+        is_localhost: true,
+      },
+    );
+    assert.deepStrictEqual(
+      buildContext(undefined, 'https://example.com/test'),
+      {
+        is_devtools_open: false,
+        is_localhost: false,
+      },
+    );
+  });
+
+  it('should include devtools_data when present', () => {
+    assert.deepStrictEqual(
+      buildContext(
+        {
+          cdpBackendNodeId: 1,
+          cdpRequestId: 'req-1',
+        },
+        'http://localhost:9222/',
+      ),
+      {
+        is_devtools_open: true,
+        is_localhost: true,
+        devtools_data: {
+          is_dom_element_selected: true,
+          is_network_request_selected: true,
+        },
+      },
+    );
+  });
+});