Skip to content

Commit 4825127

Browse files
committed
chore: disallow underscores followed by numbers in metrics names
1 parent 90d368d commit 4825127

8 files changed

Lines changed: 108 additions & 13 deletions

File tree

src/telemetry/ClearcutLogger.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import type {zod, ShapeOutput} from '../third_party/index.js';
1212

1313
import type {ErrorCode} from './errors.js';
1414
import type {LocalState, Persistence} from './persistence.js';
15-
import {sanitizeParams} from './transformation.js';
15+
import {sanitizeParams, stripUnderscoreBeforeNumber} from './transformation.js';
1616
import {
1717
McpClient,
1818
type FlagUsage,
@@ -113,14 +113,18 @@ export class ClearcutLogger {
113113
success: boolean;
114114
latencyMs: number;
115115
}): Promise<void> {
116+
const sanitizedToolName = stripUnderscoreBeforeNumber(args.toolName);
116117
const tool_invocation: ToolInvocation = {
117-
tool_name: args.toolName,
118+
tool_name: sanitizedToolName,
118119
success: args.success,
119120
latency_ms: args.latencyMs,
120121
};
121122
if (Object.keys(args.params).length > 0) {
122123
tool_invocation.tool_params = {
123-
[`${args.toolName}_params`]: sanitizeParams(args.params, args.schema),
124+
[`${sanitizedToolName}_params`]: sanitizeParams(
125+
args.params,
126+
args.schema,
127+
),
124128
};
125129
}
126130

@@ -185,7 +189,9 @@ export class ClearcutLogger {
185189
payload: {
186190
mcp_client: this.#mcpClient,
187191
server_error: {
188-
tool_name: args.toolName ?? '',
192+
tool_name: args.toolName
193+
? stripUnderscoreBeforeNumber(args.toolName)
194+
: '',
189195
error_code: args.errorCode,
190196
},
191197
},

src/telemetry/flagUtils.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import type {cliOptions} from '../bin/chrome-devtools-mcp-cli-options.js';
88
import {toSnakeCase} from '../utils/string.js';
99

10+
import {stripUnderscoreBeforeNumber} from './transformation.js';
1011
import type {FlagUsage} from './types.js';
1112

1213
type CliOptions = typeof cliOptions;
@@ -17,7 +18,9 @@ type CliOptions = typeof cliOptions;
1718
* as an `enum` where the keys of the enum will map to the uppercase `choice`.
1819
*/
1920
function formatEnumChoice(snakeCaseName: string, choice: string): string {
20-
return `${snakeCaseName}_${choice}`.toUpperCase();
21+
return stripUnderscoreBeforeNumber(
22+
`${snakeCaseName}_${choice}`,
23+
).toUpperCase();
2124
}
2225

2326
/**
@@ -41,7 +44,7 @@ export function computeFlagUsage(
4144

4245
for (const [flagName, config] of Object.entries(options)) {
4346
const value = args[flagName];
44-
const snakeCaseName = toSnakeCase(flagName);
47+
const snakeCaseName = stripUnderscoreBeforeNumber(toSnakeCase(flagName));
4548

4649
// If there isn't a default value provided for the flag,
4750
// we're going to log whether it's present on the args user
@@ -82,7 +85,7 @@ export function getPossibleFlagMetrics(options: CliOptions): FlagMetric[] {
8285
const metrics: FlagMetric[] = [];
8386

8487
for (const [flagName, config] of Object.entries(options)) {
85-
const snakeCaseName = toSnakeCase(flagName);
88+
const snakeCaseName = stripUnderscoreBeforeNumber(toSnakeCase(flagName));
8689

8790
// _present is always a possible metric
8891
metrics.push({

src/telemetry/metricsRegistry.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
transformArgType,
1212
getZodType,
1313
PARAM_BLOCKLIST,
14+
stripUnderscoreBeforeNumber,
1415
} from './transformation.js';
1516

1617
/**
@@ -118,7 +119,7 @@ export function generateToolMetrics(tools: ToolDefinition[]): ToolMetric[] {
118119
}
119120

120121
return {
121-
name: tool.name,
122+
name: stripUnderscoreBeforeNumber(tool.name),
122123
args,
123124
};
124125
});

src/telemetry/tool_call_metrics.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -616,7 +616,7 @@
616616
]
617617
},
618618
{
619-
"name": "execute_3p_developer_tool",
619+
"name": "execute3p_developer_tool",
620620
"args": [
621621
{
622622
"name": "tool_name_length",
@@ -629,7 +629,7 @@
629629
]
630630
},
631631
{
632-
"name": "list_3p_developer_tools",
632+
"name": "list3p_developer_tools",
633633
"args": []
634634
}
635635
]

src/telemetry/transformation.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,20 +53,26 @@ export function getZodType(zodType: zod.ZodTypeAny): ZodType {
5353
throw new Error(`Unsupported zod type for tool parameter: ${typeName}`);
5454
}
5555

56+
export function stripUnderscoreBeforeNumber(name: string): string {
57+
return name.replace(/_([0-9])/g, '$1');
58+
}
59+
5660
type LoggedToolCallArgValue = string | number | boolean;
5761

5862
export function transformArgName(zodType: ZodType, name: string): string {
5963
const snakeCaseName = name.replace(
6064
/[A-Z]/g,
6165
letter => `_${letter.toLowerCase()}`,
6266
);
67+
let transformed: string;
6368
if (zodType === 'ZodString') {
64-
return `${snakeCaseName}_length`;
69+
transformed = `${snakeCaseName}_length`;
6570
} else if (zodType === 'ZodArray') {
66-
return `${snakeCaseName}_count`;
71+
transformed = `${snakeCaseName}_count`;
6772
} else {
68-
return snakeCaseName;
73+
transformed = snakeCaseName;
6974
}
75+
return stripUnderscoreBeforeNumber(transformed);
7076
}
7177

7278
export function transformArgType(zodType: ZodType): string {

tests/telemetry/flagUtils.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,18 @@ describe('computeFlagUsage', () => {
106106
const usage = computeFlagUsage(args, mockOptions);
107107
assert.equal(usage.string_flag_present, false);
108108
});
109+
110+
it('sanitizes flag names containing underscores before numbers', () => {
111+
const mock3pOptions = {
112+
experimental3pTool: {
113+
type: 'boolean' as const,
114+
description: 'A 3p flag',
115+
},
116+
} as unknown as typeof cliOptions;
117+
const args = {experimental3pTool: true};
118+
const usage = computeFlagUsage(args, mock3pOptions);
119+
assert.equal(usage.experimental3p_tool, true);
120+
});
109121
});
110122
});
111123

@@ -141,4 +153,19 @@ describe('getPossibleFlagMetrics', () => {
141153
},
142154
]);
143155
});
156+
157+
it('sanitizes flag names containing underscores before numbers', () => {
158+
const mock3pOptions = {
159+
experimental3pTool: {
160+
type: 'boolean' as const,
161+
description: 'A 3p flag',
162+
},
163+
} as unknown as typeof cliOptions;
164+
const metrics = getPossibleFlagMetrics(mock3pOptions);
165+
166+
assert.deepEqual(metrics, [
167+
{name: 'experimental3p_tool_present', flagType: 'boolean'},
168+
{name: 'experimental3p_tool', flagType: 'boolean'},
169+
]);
170+
});
144171
});

tests/telemetry/metricsRegistry.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,26 @@ describe('metricsRegistry', () => {
8282
assert.strictEqual(metrics[0].args[0].name, 'arg_enum');
8383
assert.strictEqual(metrics[0].args[0].argType, 'string');
8484
});
85+
86+
it('should sanitize tool names containing underscores before numbers', () => {
87+
const mockTool: ToolDefinition = {
88+
name: 'list_3p_developer_tools',
89+
description: 'test description',
90+
annotations: {
91+
category: ToolCategory.THIRD_PARTY,
92+
readOnlyHint: true,
93+
},
94+
schema: {},
95+
blockedByDialog: false,
96+
handler: async () => {
97+
// no-op
98+
},
99+
};
100+
101+
const metrics = generateToolMetrics([mockTool]);
102+
assert.strictEqual(metrics.length, 1);
103+
assert.strictEqual(metrics[0].name, 'list3p_developer_tools');
104+
});
85105
});
86106

87107
describe('applyToExistingMetrics', () => {

tests/telemetry/transformation.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import {describe, it} from 'node:test';
1010
import {
1111
bucketizeLatency,
1212
sanitizeParams,
13+
stripUnderscoreBeforeNumber,
14+
transformArgName,
1315
} from '../../src/telemetry/transformation.js';
1416
import {zod} from '../../src/third_party/index.js';
1517

@@ -133,3 +135,33 @@ describe('sanitizeParams', () => {
133135
);
134136
});
135137
});
138+
139+
describe('stripUnderscoreBeforeNumber', () => {
140+
it('removes underscores immediately preceding numbers', () => {
141+
assert.strictEqual(
142+
stripUnderscoreBeforeNumber('list_3p_developer_tools'),
143+
'list3p_developer_tools',
144+
);
145+
assert.strictEqual(
146+
stripUnderscoreBeforeNumber('make_2g_network_request'),
147+
'make2g_network_request',
148+
);
149+
assert.strictEqual(
150+
stripUnderscoreBeforeNumber('no_numbers_here'),
151+
'no_numbers_here',
152+
);
153+
});
154+
});
155+
156+
describe('transformArgName', () => {
157+
it('sanitizes argument names containing underscores before numbers', () => {
158+
assert.strictEqual(
159+
transformArgName('ZodNumber', 'my3pParam'),
160+
'my3p_param',
161+
);
162+
assert.strictEqual(
163+
transformArgName('ZodString', 'my3pParam'),
164+
'my3p_param_length',
165+
);
166+
});
167+
});

0 commit comments

Comments
 (0)