Skip to content
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

codewhisperer: telemetry metrics apply fix #4007

Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 44 additions & 37 deletions src/codewhisperer/commands/basicCommands.ts
Original file line number Diff line number Diff line change
@@ -4,14 +4,14 @@
*/

import * as vscode from 'vscode'
import { telemetry } from '../../shared/telemetry/telemetry'
import { ApplyFixSource, CodewhispererCodeScanIssueApplyFix, telemetry } from '../../shared/telemetry/telemetry'
import { ExtContext } from '../../shared/extensions'
import { Commands } from '../../shared/vscode/commands2'
import * as CodeWhispererConstants from '../models/constants'
import { DefaultCodeWhispererClient } from '../client/codewhisperer'
import { startSecurityScanWithProgress, confirmStopSecurityScan } from './startSecurityScan'
import { SecurityPanelViewProvider } from '../views/securityPanelViewProvider'
import { CodeScanIssueCommandArgs, codeScanState } from '../models/model'
import { CodeScanIssue, codeScanState } from '../models/model'
import { connectToEnterpriseSso, getStartUrl } from '../util/getStartUrl'
import { showConnectionPrompt } from '../util/showSsoPrompt'
import { ReferenceLogViewProvider } from '../service/referenceLogViewProvider'
@@ -33,6 +33,7 @@ import { localize } from '../../shared/utilities/vsCodeUtils'
import { applyPatch } from 'diff'
import { closeSecurityIssueWebview, showSecurityIssueWebview } from '../views/securityIssue/securityIssueWebview'
import { FileSystemCommon } from '../../srcShared/fs'
import { Mutable } from '../../shared/utilities/tsUtils'

export const toggleCodeSuggestions = Commands.declare(
'aws.codeWhisperer.toggleCodeSuggestion',
@@ -203,8 +204,8 @@ export const refreshStatusBar = Commands.declare(

export const openSecurityIssuePanel = Commands.declare(
'aws.codeWhisperer.openSecurityIssuePanel',
(context: ExtContext) => (issue: CodeScanIssueCommandArgs) => {
showSecurityIssueWebview(context.extensionContext, issue)
(context: ExtContext) => (issue: CodeScanIssue, filePath: string) => {
showSecurityIssueWebview(context.extensionContext, issue, filePath)
}
)

@@ -217,48 +218,54 @@ export const notifyNewCustomizationsCmd = Commands.declare(

export const applySecurityFix = Commands.declare(
'aws.codeWhisperer.applySecurityFix',
() =>
async ({ filePath, ...issue }: CodeScanIssueCommandArgs) => {
const [suggestedFix] = issue?.suggestedFixes ?? []
() => async (issue: CodeScanIssue, filePath: string, source: ApplyFixSource) => {
const [suggestedFix] = issue.suggestedFixes
if (!suggestedFix || !filePath) {
return
}

if (!suggestedFix || !filePath) {
return
}
const applyFixTelemetryEntry: Mutable<CodewhispererCodeScanIssueApplyFix> = {
detectorId: issue.detectorId,
findingId: issue.findingId,
applyFixSource: source,
result: 'Succeeded',
}

try {
const patch = suggestedFix.code
const document = await vscode.workspace.openTextDocument(filePath)
const fileContent = document.getText()

const updatedContent = applyPatch(fileContent, patch)
if (!updatedContent) {
vscode.window.showErrorMessage(CodeWhispererConstants.codeFixAppliedFailedMessage)
throw Error('Failed to get updated content from applying diff patch')
}

if (updatedContent) {
// saving the document text if not save
document.save().then(isSaved => {
if (isSaved) {
// writing the patch applied version of document into the file
FileSystemCommon.instance
.writeFile(filePath, updatedContent)
.then(() => {
vscode.window
.showInformationMessage(CodeWhispererConstants.codeFixAppliedSuccessMessage, {
title: CodeWhispererConstants.runSecurityScanButtonTitle,
})
.then(res => {
if (res?.title === CodeWhispererConstants.runSecurityScanButtonTitle) {
vscode.commands.executeCommand('aws.codeWhisperer.security.scan')
}
})
closeSecurityIssueWebview(issue.findingId)
})
.catch(err => {
getLogger().error(`Unable to write updated text content into the file: ${err}`)
})
} else {
getLogger().error('Failed to save editor text changes into the file.')
// saving the document text if not save
const isSaved = await document.save()
if (!isSaved) {
throw Error('Failed to save editor text changes into the file.')
}

// writing the patch applied version of document into the file
await FileSystemCommon.instance.writeFile(filePath, updatedContent)
vscode.window
.showInformationMessage(CodeWhispererConstants.codeFixAppliedSuccessMessage, {
title: CodeWhispererConstants.runSecurityScanButtonTitle,
})
.then(res => {
if (res?.title === CodeWhispererConstants.runSecurityScanButtonTitle) {
vscode.commands.executeCommand('aws.codeWhisperer.security.scan')
}
})
} else {
vscode.window.showErrorMessage(CodeWhispererConstants.codeFixAppliedFailedMessage)
}
closeSecurityIssueWebview(issue.findingId)
} catch (err) {
getLogger().error(`Apply fix command failed. ${err}`)
applyFixTelemetryEntry.result = 'Failed'
applyFixTelemetryEntry.reason = err as string
} finally {
telemetry.codewhisperer_codeScanIssueApplyFix.emit(applyFixTelemetryEntry)
}
}
)
4 changes: 0 additions & 4 deletions src/codewhisperer/models/model.ts
Original file line number Diff line number Diff line change
@@ -218,10 +218,6 @@ export interface CodeScanIssue {
suggestedFixes: SuggestedFix[]
}

export interface CodeScanIssueCommandArgs extends CodeScanIssue {
filePath: string
}

export interface AggregatedCodeScanIssue {
filePath: string
issues: CodeScanIssue[]
4 changes: 2 additions & 2 deletions src/codewhisperer/service/securityIssueCodeActionProvider.ts
Original file line number Diff line number Diff line change
@@ -38,15 +38,15 @@ export class SecurityIssueCodeActionProvider extends SecurityIssueProvider imple
fixIssue.command = {
title: 'Apply suggested fix',
command: 'aws.codeWhisperer.applySecurityFix',
arguments: [{ ...issue, filePath: group.filePath }],
arguments: [issue, group.filePath, 'quickfix'],
}
codeActions.push(fixIssue)
}
const openIssue = new vscode.CodeAction(`View details for "${issue.title}"`)
openIssue.command = {
title: 'Open "CodeWhisperer Security Issue"',
command: 'aws.codeWhisperer.openSecurityIssuePanel',
arguments: [{ ...issue, filePath: group.filePath }],
arguments: [issue, group.filePath],
}
codeActions.push(openIssue)
}
12 changes: 5 additions & 7 deletions src/codewhisperer/service/securityIssueHoverProvider.ts
Original file line number Diff line number Diff line change
@@ -6,7 +6,7 @@ import * as vscode from 'vscode'
import { CodeScanIssue } from '../models/model'
import globals from '../../shared/extensionGlobals'
import { SecurityIssueProvider } from './securityIssueProvider'
import { telemetry } from '../../shared/telemetry/telemetry'
import { ApplyFixSource, telemetry } from '../../shared/telemetry/telemetry'
import path from 'path'

export class SecurityIssueHoverProvider extends SecurityIssueProvider implements vscode.HoverProvider {
@@ -64,21 +64,19 @@ export class SecurityIssueHoverProvider extends SecurityIssueProvider implements

markdownString.appendMarkdown(`${issue.description.markdown}\n\n`)

const args = [issue, filePath]
const viewDetailsCommand = vscode.Uri.parse(
`command:aws.codeWhisperer.openSecurityIssuePanel?${encodeURIComponent(
JSON.stringify({ ...issue, filePath })
)}`
`command:aws.codeWhisperer.openSecurityIssuePanel?${encodeURIComponent(JSON.stringify(args))}`
)

markdownString.appendMarkdown(
`[$(eye) View Details](${viewDetailsCommand} 'Open "CodeWhisperer Security Issue"')\n`
)

if (suggestedFix) {
const args: [CodeScanIssue, string, ApplyFixSource] = [issue, filePath, 'hover']
const applyFixCommand = vscode.Uri.parse(
`command:aws.codeWhisperer.applySecurityFix?${encodeURIComponent(
JSON.stringify({ ...issue, filePath })
)}`
`command:aws.codeWhisperer.applySecurityFix?${encodeURIComponent(JSON.stringify(args))}`
)
markdownString.appendMarkdown(` | [$(wrench) Apply Fix](${applyFixCommand} "Apply suggested fix")\n`)
markdownString.appendMarkdown(
16 changes: 11 additions & 5 deletions src/codewhisperer/views/securityIssue/securityIssueWebview.ts
Original file line number Diff line number Diff line change
@@ -5,12 +5,13 @@

import * as vscode from 'vscode'
import { VueWebview } from '../../../webviews/main'
import { CodeScanIssueCommandArgs } from '../../models/model'
import { CodeScanIssue } from '../../models/model'

export class SecurityIssueWebview extends VueWebview {
public readonly id = 'aws.codeWhisperer.securityIssue'
public readonly source = 'src/codewhisperer/views/securityIssue/vue/index.js'
private issue: CodeScanIssueCommandArgs | undefined
private issue: CodeScanIssue | undefined
private filePath: string | undefined

public constructor() {
super()
@@ -20,12 +21,16 @@ export class SecurityIssueWebview extends VueWebview {
return this.issue
}

public setIssue(issue: CodeScanIssueCommandArgs) {
public setIssue(issue: CodeScanIssue) {
this.issue = issue
}

public setFilePath(filePath: string) {
this.filePath = filePath
}

public applyFix() {
vscode.commands.executeCommand('aws.codeWhisperer.applySecurityFix', this.issue)
vscode.commands.executeCommand('aws.codeWhisperer.applySecurityFix', this.issue, this.filePath, 'webview')
}

public closeWebview(findingId: string) {
@@ -38,9 +43,10 @@ export class SecurityIssueWebview extends VueWebview {
const Panel = VueWebview.compilePanel(SecurityIssueWebview)
let activePanel: InstanceType<typeof Panel> | undefined

export async function showSecurityIssueWebview(ctx: vscode.ExtensionContext, issue: CodeScanIssueCommandArgs) {
export async function showSecurityIssueWebview(ctx: vscode.ExtensionContext, issue: CodeScanIssue, filePath: string) {
activePanel ??= new Panel(ctx)
activePanel.server.setIssue(issue)
activePanel.server.setFilePath(filePath)

const webviewPanel = await activePanel.show({
title: 'CodeWhisperer Security Issue',
11 changes: 11 additions & 0 deletions src/shared/telemetry/vscodeTelemetry.json
Original file line number Diff line number Diff line change
@@ -47,6 +47,12 @@
"name": "detectorId",
"type": "string",
"description": "The id of the detector which produced the code scan issue"
},
{
"name": "applyFixSource",
"type": "string",
"allowedValues": ["hover", "webview", "quickfix"],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could add these values to the existing component type: https://github.com/aws/aws-toolkit-common/blob/bbc34c2f5b5f1f0a24befa5c42f4193e3e12032c/telemetry/definitions/commonDefinitions.json#L57

then just use component in codewhisperer_codeScanIssueApplyFix

"description": "The location which triggered the apply fix command for a code scan issue"
}
],
"metrics": [
@@ -266,6 +272,11 @@
"name": "codewhisperer_codeScanIssueHover",
"description": "Called when a code scan issue is hovered over",
"metadata": [{ "type": "findingId" }, { "type": "detectorId" }]
},
{
"name": "codewhisperer_codeScanIssueApplyFix",
"description": "Called when a code scan issue suggested fix is applied",
"metadata": [{ "type": "findingId" }, { "type": "detectorId" }, { "type": "applyFixSource" }]
}
]
}
183 changes: 102 additions & 81 deletions src/test/codewhisperer/commands/basicCommands.test.ts
Original file line number Diff line number Diff line change
@@ -8,7 +8,7 @@ import assert from 'assert'
import * as sinon from 'sinon'
import * as CodeWhispererConstants from '../../../codewhisperer/models/constants'
import { createCodeScanIssue, createMockDocument, resetCodeWhispererGlobalVariables } from '../testUtil'
import { assertTelemetryCurried } from '../../testUtil'
import { assertTelemetry, assertTelemetryCurried } from '../../testUtil'
import {
toggleCodeSuggestions,
showSecurityScan,
@@ -138,139 +138,160 @@ describe('CodeWhisperer-basicCommands', function () {
await targetCommand.execute()
assert.strictEqual(getTestWindow().shownMessages[0].message, 'Open a valid file to scan.')
})
})

describe('applySecurityFix', function () {
let sandbox: sinon.SinonSandbox
let saveStub: sinon.SinonStub
let openTextDocumentMock: sinon.SinonStub
let writeFileMock: sinon.SinonStub

beforeEach(function () {
sandbox = sinon.createSandbox()
saveStub = sinon.stub()
openTextDocumentMock = sinon.stub()
writeFileMock = sinon.stub()
})

afterEach(function () {
sandbox.restore()
})

it('should call applySecurityFix command successfully', async function () {
const fileName = 'sample.py'
const saveStub = sinon.stub()
saveStub.resolves(true)
const textDocumentMock = new MockDocument('first line\n second line\n fourth line', fileName, saveStub)

const openTextDocumentMock = sinon.stub()
openTextDocumentMock.resolves(textDocumentMock)
const sandbox = sinon.createSandbox()
sandbox.stub(vscode.workspace, 'openTextDocument').value(openTextDocumentMock)

const writeFileMock = sinon.stub()
writeFileMock.resolves(true)
sinon.stub(FileSystemCommon.prototype, 'writeFile').value(writeFileMock)

targetCommand = testCommand(applySecurityFix)
await targetCommand.execute({
filePath: fileName,
...createCodeScanIssue({
suggestedFixes: [
{
description: 'fix',
code: '@@ -1,3 +1,3 @@\n first line\n- second line\n+ third line\n fourth line',
},
],
}),
const codeScanIssue = createCodeScanIssue({
suggestedFixes: [
{
description: 'fix',
code: '@@ -1,3 +1,3 @@\n first line\n- second line\n+ third line\n fourth line',
},
],
})
await targetCommand.execute(codeScanIssue, fileName, 'hover')
assert.ok(saveStub.calledOnce)
assert.ok(writeFileMock.calledOnceWith(fileName, 'first line\n third line\n fourth line'))

assert.strictEqual(
getTestWindow().shownMessages[0].message,
'Code fix was applied. Run a security scan to validate the fix.'
)

sandbox.restore()
assertTelemetry('codewhisperer_codeScanIssueApplyFix', {
detectorId: codeScanIssue.detectorId,
findingId: codeScanIssue.findingId,
applyFixSource: 'hover',
result: 'Succeeded',
})
})

it('handles patch failure', async function () {
const textDocumentMock = createMockDocument()

const openTextDocumentMock = sinon.stub()
openTextDocumentMock.resolves(textDocumentMock)

const sandbox = sinon.createSandbox()
sandbox.stub(vscode.workspace, 'openTextDocument').value(openTextDocumentMock)

targetCommand = testCommand(applySecurityFix)
await targetCommand.execute({
filePath: 'test.py',
...createCodeScanIssue({
suggestedFixes: [
{
code: '@@ -1,1 -1,1 @@\n-mock\n+line5',
description: 'dummy',
},
],
}),
const codeScanIssue = createCodeScanIssue({
suggestedFixes: [
{
code: '@@ -1,1 -1,1 @@\n-mock\n+line5',
description: 'dummy',
},
],
})
await targetCommand.execute(codeScanIssue, 'test.py', 'webview')

assert.strictEqual(getTestWindow().shownMessages[0].message, 'Failed to apply suggested code fix.')

sandbox.restore()
assertTelemetry('codewhisperer_codeScanIssueApplyFix', {
detectorId: codeScanIssue.detectorId,
findingId: codeScanIssue.findingId,
applyFixSource: 'webview',
result: 'Failed',
reason: 'Error: Failed to get updated content from applying diff patch',
})
})
})

it('handles document save failure', async function () {
const fileName = 'sample.py'
const saveStub = sinon.stub()
saveStub.resolves(false)
const textDocumentMock = new MockDocument('first line\n second line\n fourth line', fileName, saveStub)
it('handles document save failure', async function () {
const fileName = 'sample.py'
saveStub.resolves(false)
const textDocumentMock = new MockDocument('first line\n second line\n fourth line', fileName, saveStub)

const openTextDocumentMock = sinon.stub()
openTextDocumentMock.resolves(textDocumentMock)
openTextDocumentMock.resolves(textDocumentMock)

const sandbox = sinon.createSandbox()
sandbox.stub(vscode.workspace, 'openTextDocument').value(openTextDocumentMock)
const loggerStub = sinon.stub(getLogger(), 'error')
sandbox.stub(vscode.workspace, 'openTextDocument').value(openTextDocumentMock)
const loggerStub = sinon.stub(getLogger(), 'error')

targetCommand = testCommand(applySecurityFix)
await targetCommand.execute({
filePath: fileName,
...createCodeScanIssue({
targetCommand = testCommand(applySecurityFix)
const codeScanIssue = createCodeScanIssue({
suggestedFixes: [
{
description: 'fix',
code: '@@ -1,3 +1,3 @@\n first line\n- second line\n+ third line\n fourth line',
},
],
}),
})
await targetCommand.execute(codeScanIssue, fileName, 'quickfix')

assert.ok(saveStub.calledOnce)
assert.ok(loggerStub.calledOnce)
const actual = loggerStub.getCall(0).args[0]
assert.strictEqual(
actual,
'Apply fix command failed. Error: Failed to save editor text changes into the file.'
)
assertTelemetry('codewhisperer_codeScanIssueApplyFix', {
detectorId: codeScanIssue.detectorId,
findingId: codeScanIssue.findingId,
applyFixSource: 'quickfix',
result: 'Failed',
reason: 'Error: Failed to save editor text changes into the file.',
})
})

assert.ok(saveStub.calledOnce)
assert.ok(loggerStub.calledOnce)
const actual = loggerStub.getCall(0).args[0]
assert.strictEqual(actual, 'Failed to save editor text changes into the file.')
it('handles document write failure', async function () {
const fileName = 'sample.py'
saveStub.resolves(true)
const textDocumentMock = new MockDocument('first line\n second line\n fourth line', fileName, saveStub)

sandbox.restore()
})
it('handles document write failure', async function () {
const fileName = 'sample.py'
const saveStub = sinon.stub()
saveStub.resolves(true)
const textDocumentMock = new MockDocument('first line\n second line\n fourth line', fileName, saveStub)

const openTextDocumentMock = sinon.stub()
openTextDocumentMock.resolves(textDocumentMock)
const writeFileMock = sinon.stub()
writeFileMock.rejects('writing to file failed.')

const sandbox = sinon.createSandbox()
sandbox.stub(vscode.workspace, 'openTextDocument').value(openTextDocumentMock)
sinon.stub(FileSystemCommon.prototype, 'writeFile').value(writeFileMock)
const loggerStub = sinon.stub(getLogger(), 'error')

targetCommand = testCommand(applySecurityFix)
await targetCommand.execute({
filePath: fileName,
...createCodeScanIssue({
openTextDocumentMock.resolves(textDocumentMock)
writeFileMock.rejects('Error: Writing to file failed.')

sandbox.stub(vscode.workspace, 'openTextDocument').value(openTextDocumentMock)
sinon.stub(FileSystemCommon.prototype, 'writeFile').value(writeFileMock)
const loggerStub = sinon.stub(getLogger(), 'error')

targetCommand = testCommand(applySecurityFix)
const codeScanIssue = createCodeScanIssue({
suggestedFixes: [
{
description: 'fix',
code: '@@ -1,3 +1,3 @@\n first line\n- second line\n+ third line\n fourth line',
},
],
}),
})

assert.ok(saveStub.calledOnce)
assert.ok(loggerStub.calledOnce)
const actual = loggerStub.getCall(0).args[0]
assert.strictEqual(actual, 'Unable to write updated text content into the file: writing to file failed.')
})
await targetCommand.execute(codeScanIssue, fileName, 'hover')

sandbox.restore()
assert.ok(saveStub.calledOnce)
assert.ok(loggerStub.calledOnce)
const actual = loggerStub.getCall(0).args[0]
assert.strictEqual(actual, 'Apply fix command failed. Error: Writing to file failed.')
assertTelemetry('codewhisperer_codeScanIssueApplyFix', {
detectorId: codeScanIssue.detectorId,
findingId: codeScanIssue.findingId,
applyFixSource: 'hover',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can we use different source values (hover, webview, or quickfix) to verify that applyFixSource value is not hardcoded by any chance.

result: 'Failed',
reason: 'Error: Writing to file failed.',
})
})
})
})
Original file line number Diff line number Diff line change
@@ -45,10 +45,10 @@ describe('securityIssueHoverProvider', () => {
'## Suggested Fix for title ![High](severity-high.svg)\n' +
'description\n\n' +
`[$(eye) View Details](command:aws.codeWhisperer.openSecurityIssuePanel?${encodeURIComponent(
JSON.stringify({ ...issues[0], filePath: mockDocument.fileName })
JSON.stringify([issues[0], mockDocument.fileName])
)} 'Open "CodeWhisperer Security Issue"')\n` +
` | [$(wrench) Apply Fix](command:aws.codeWhisperer.applySecurityFix?${encodeURIComponent(
JSON.stringify({ ...issues[0], filePath: mockDocument.fileName })
JSON.stringify([issues[0], mockDocument.fileName, 'hover'])
)} "Apply suggested fix")\n\n` +
'<span class="codicon codicon-none" style="background-color:var(--vscode-textCodeBlock-background);">\n\n' +
'```language\n' +
@@ -79,7 +79,7 @@ describe('securityIssueHoverProvider', () => {
'## title ![High](severity-high.svg)\n' +
'description\n\n' +
`[$(eye) View Details](command:aws.codeWhisperer.openSecurityIssuePanel?${encodeURIComponent(
JSON.stringify({ ...issues[1], filePath: mockDocument.fileName })
JSON.stringify([issues[1], mockDocument.fileName])
)} 'Open "CodeWhisperer Security Issue"')\n`
)
assertTelemetry('codewhisperer_codeScanIssueHover', [