Skip to content

Commit 01d8112

Browse files
authored
fix: add bash command parsing for telemetry metrics (aws#2039)
* fix: add bash command parsing for telemetry metrics * fix: handle metric withint executeBash tool itself
1 parent da11663 commit 01d8112

File tree

5 files changed

+223
-4
lines changed

5 files changed

+223
-4
lines changed

server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/executeBash.ts

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import { Features } from '../../types'
1212
import { getWorkspaceFolderPaths } from '@aws/lsp-core/out/util/workspaceUtils'
1313
// eslint-disable-next-line import/no-nodejs-modules
1414
import { existsSync, statSync } from 'fs'
15+
import { parseBaseCommands } from '../utils/commandParser'
16+
import { BashCommandEvent, ChatTelemetryEventName } from '../../../shared/telemetry/types'
1517

1618
export enum CommandCategory {
1719
ReadOnly,
@@ -130,9 +132,15 @@ export class ExecuteBash {
130132
private childProcess?: ChildProcess
131133
private readonly logging: Features['logging']
132134
private readonly workspace: Features['workspace']
133-
constructor(features: Pick<Features, 'logging' | 'workspace'> & Partial<Features>) {
135+
private readonly telemetry: Features['telemetry']
136+
private readonly credentialsProvider: Features['credentialsProvider']
137+
constructor(
138+
features: Pick<Features, 'logging' | 'workspace' | 'telemetry' | 'credentialsProvider'> & Partial<Features>
139+
) {
134140
this.logging = features.logging
135141
this.workspace = features.workspace
142+
this.telemetry = features.telemetry
143+
this.credentialsProvider = features.credentialsProvider
136144
}
137145

138146
public async validate(input: ExecuteBashParams): Promise<void> {
@@ -567,6 +575,7 @@ export class ExecuteBash {
567575
})
568576
}
569577

578+
let success = false
570579
try {
571580
const result = await this.childProcess.run()
572581

@@ -579,7 +588,7 @@ export class ExecuteBash {
579588
const exitStatus = result.exitCode ?? 0
580589
const stdout = stdoutBuffer.join('\n')
581590
const stderr = stderrBuffer.join('\n')
582-
const success = exitStatus === 0 && !stderr
591+
success = exitStatus === 0 && !stderr
583592
const [stdoutTrunc, stdoutSuffix] = ExecuteBash.truncateSafelyWithSuffix(
584593
stdout,
585594
maxToolResponseSize / 3
@@ -611,6 +620,25 @@ export class ExecuteBash {
611620
reject(new Error(`Failed to execute command: ${err.message}`))
612621
}
613622
} finally {
623+
// Extract individual base commands for telemetry purposes
624+
const args = split(params.command)
625+
const baseCommands = parseBaseCommands(args)
626+
baseCommands.forEach(command => {
627+
const metricPayload = {
628+
name: ChatTelemetryEventName.BashCommand,
629+
data: {
630+
credentialStartUrl: this.credentialsProvider.getConnectionMetadata()?.sso?.startUrl,
631+
result: cancellationToken?.isCancellationRequested
632+
? 'Cancelled'
633+
: success
634+
? 'Succeeded'
635+
: 'Failed',
636+
command: command,
637+
} as BashCommandEvent,
638+
}
639+
this.telemetry.emitMetric(metricPayload)
640+
})
641+
614642
await writer?.close()
615643
writer?.releaseLock()
616644
}

server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/toolServer.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -172,8 +172,8 @@ export const QCodeAnalysisServer: Server = ({
172172
return () => {}
173173
}
174174

175-
export const BashToolsServer: Server = ({ logging, workspace, agent, lsp }) => {
176-
const bashTool = new ExecuteBash({ logging, workspace, lsp })
175+
export const BashToolsServer: Server = ({ logging, workspace, agent, lsp, telemetry, credentialsProvider }) => {
176+
const bashTool = new ExecuteBash({ logging, workspace, lsp, telemetry, credentialsProvider })
177177
agent.addTool(
178178
bashTool.getSpec(),
179179
async (input: ExecuteBashParams, token?: CancellationToken, updates?: WritableStream) => {
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
/**
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
import * as assert from 'assert'
7+
import { parseBaseCommands } from './commandParser'
8+
import { split } from 'shlex'
9+
10+
describe('commandParser', () => {
11+
describe('parseBaseCommands', () => {
12+
it('should extract base command from a simple command', () => {
13+
assert.deepStrictEqual(parseBaseCommands(split('cd /home/user/documents')), ['cd'])
14+
})
15+
16+
it('should extract multiple commands separated by &&', () => {
17+
assert.deepStrictEqual(parseBaseCommands(split('echo "Hello World" && ls -la')), ['echo', 'ls'])
18+
})
19+
20+
it('should extract multiple commands separated by ||', () => {
21+
assert.deepStrictEqual(parseBaseCommands(split('grep "pattern" file.txt || echo "Not found"')), [
22+
'grep',
23+
'echo',
24+
])
25+
})
26+
27+
it('should extract multiple commands separated by |', () => {
28+
assert.deepStrictEqual(parseBaseCommands(split('cat file.txt | grep "pattern"')), ['cat', 'grep'])
29+
})
30+
31+
it('should handle commands with quotes', () => {
32+
assert.deepStrictEqual(
33+
parseBaseCommands(split('echo "text with spaces" && grep "pattern with spaces" file.txt')),
34+
['echo', 'grep']
35+
)
36+
})
37+
38+
it('should return empty array for null, undefined, empty input', () => {
39+
assert.deepStrictEqual(parseBaseCommands(null as any), [])
40+
assert.deepStrictEqual(parseBaseCommands(undefined as any), [])
41+
assert.deepStrictEqual(parseBaseCommands(split('')), [])
42+
})
43+
44+
it('should handle commands with semicolons', () => {
45+
assert.deepStrictEqual(parseBaseCommands(split('cd /tmp; ls -la; echo "done"')), ['cd', 'ls', 'echo'])
46+
})
47+
48+
it('should handle commands with sudo prefix', () => {
49+
assert.deepStrictEqual(parseBaseCommands(split('sudo apt-get install package')), ['sudo', 'apt-get'])
50+
assert.deepStrictEqual(parseBaseCommands(split('sudo -u user command')), ['sudo', 'command'])
51+
})
52+
53+
it('should handle commands with time prefix', () => {
54+
assert.deepStrictEqual(parseBaseCommands(split('time curl http://example.com')), ['time', 'curl'])
55+
})
56+
57+
it('should handle commands with path prefixes', () => {
58+
assert.deepStrictEqual(parseBaseCommands(split('/usr/bin/python script.py')), ['python'])
59+
assert.deepStrictEqual(parseBaseCommands(split('./script.sh -arg')), ['script.sh'])
60+
assert.deepStrictEqual(parseBaseCommands(split('../bin/tool --option')), ['tool'])
61+
})
62+
63+
it('should handle commands with sudo and path prefixes', () => {
64+
assert.deepStrictEqual(parseBaseCommands(split('sudo /usr/bin/apt-get update')), ['sudo', 'apt-get'])
65+
})
66+
67+
it('should handle multiple commands with mixed separators', () => {
68+
assert.deepStrictEqual(parseBaseCommands(split('cd /tmp; ls -la | grep "file" && echo "found"')), [
69+
'cd',
70+
'ls',
71+
'grep',
72+
'echo',
73+
])
74+
})
75+
76+
it('should handle commands with other common prefixes', () => {
77+
assert.deepStrictEqual(parseBaseCommands(split('nice -n 10 command')), ['nice', 'command'])
78+
assert.deepStrictEqual(parseBaseCommands(split('nohup command &')), ['nohup', 'command'])
79+
})
80+
81+
it('should handle commands with function calls', () => {
82+
assert.deepStrictEqual(parseBaseCommands(split('function_name args')), ['function_name'])
83+
})
84+
})
85+
})
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
import { splitOperators } from '../tools/executeBash'
2+
3+
/**
4+
* Parses command arguments and extracts only the base commands without arguments or options.
5+
*
6+
* Examples:
7+
* - "cd /home/user/documents" -> ["cd"]
8+
* - "echo 'Hello World' && ls -la" -> ["echo", "ls"]
9+
* - "sudo apt-get install" -> ["sudo", "apt-get"]
10+
* - "time curl http://example.com" -> ["time", "curl"]
11+
* - "command1; command2" -> ["command1", "command2"]
12+
* - "/usr/bin/python script.py" -> ["python"]
13+
* - "./script.sh" -> ["script.sh"]
14+
* - "function_name args" -> ["function_name"]
15+
*
16+
* @param args Array of command arguments
17+
* @returns Array of base commands found in the input args
18+
*/
19+
export function parseBaseCommands(args: string[]): string[] {
20+
if (!args || !Array.isArray(args) || args.length === 0) {
21+
return []
22+
}
23+
24+
const baseCommands: string[] = []
25+
26+
// Process the args to extract base commands
27+
let i = 0
28+
let expectCommand = true // Flag to indicate we're expecting a command
29+
30+
while (i < args.length) {
31+
const arg = args[i]
32+
33+
// Check if this arg is an operator or contains an operator
34+
if (splitOperators.has(arg) || arg.includes(';')) {
35+
expectCommand = true // Next argument should be a command
36+
i++
37+
continue
38+
}
39+
40+
if (expectCommand) {
41+
// Extract the base command
42+
let baseCommand = arg
43+
44+
// Handle path prefixes (/usr/bin/command or ./command)
45+
if (baseCommand.includes('/')) {
46+
baseCommand = baseCommand.split('/').pop() || baseCommand
47+
}
48+
49+
baseCommands.push(baseCommand)
50+
51+
// Special case for sudo, time, etc. - include the actual command too
52+
if (['sudo', 'time', 'nice', 'nohup', 'env'].includes(baseCommand)) {
53+
// Skip any flags/options and their values
54+
let j = i + 1
55+
while (j < args.length) {
56+
// If we find an operator, stop looking for the command
57+
if (splitOperators.has(args[j]) || args[j].includes(';')) {
58+
break
59+
}
60+
61+
// Skip flag and its value if present
62+
if (args[j].startsWith('-')) {
63+
// Handle flags with values (e.g., -u user, -n 10)
64+
if (
65+
j + 1 < args.length &&
66+
!args[j + 1].startsWith('-') &&
67+
!splitOperators.has(args[j + 1]) &&
68+
!args[j + 1].includes(';')
69+
) {
70+
j += 2 // Skip both the flag and its value
71+
} else {
72+
j++ // Skip just the flag
73+
}
74+
continue
75+
}
76+
77+
// Found the actual command
78+
let nextCommand = args[j]
79+
80+
// Handle path prefixes for the command after sudo/time
81+
if (nextCommand.includes('/')) {
82+
nextCommand = nextCommand.split('/').pop() || nextCommand
83+
}
84+
85+
baseCommands.push(nextCommand)
86+
break
87+
}
88+
}
89+
90+
// For all commands, we don't expect another command until we see an operator
91+
expectCommand = false
92+
}
93+
94+
i++
95+
}
96+
97+
return baseCommands
98+
}

server/aws-lsp-codewhisperer/src/shared/telemetry/types.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,7 @@ export enum ChatTelemetryEventName {
210210
ExportTab = 'amazonq_exportTab',
211211
UiClick = 'ui_click',
212212
ActiveUser = 'amazonq_activeUser',
213+
BashCommand = 'amazonq_bashCommand',
213214
}
214215

215216
export interface ChatTelemetryEventMap {
@@ -234,6 +235,7 @@ export interface ChatTelemetryEventMap {
234235
[ChatTelemetryEventName.ExportTab]: ExportTabEvent
235236
[ChatTelemetryEventName.UiClick]: UiClickEvent
236237
[ChatTelemetryEventName.ActiveUser]: ActiveUserEvent
238+
[ChatTelemetryEventName.BashCommand]: BashCommandEvent
237239
}
238240

239241
export type AgencticLoop_InvokeLLMEvent = {
@@ -271,6 +273,12 @@ export type ActiveUserEvent = {
271273
result: string
272274
}
273275

276+
export type BashCommandEvent = {
277+
credentialStartUrl: string
278+
result: string
279+
command: string
280+
}
281+
274282
export type ModifyCodeEvent = {
275283
cwsprChatConversationId: string
276284
cwsprChatMessageId: string

0 commit comments

Comments
 (0)