telemetry(amazonq): modifySourceFolder event #4986
Conversation
| @@ -0,0 +1,4 @@ | |||
| { | |||
| "type": "Feature", | |||
| "description": "add modifyDefaultSource folder event when customer uses /dev source folder modification feature" | |||
There was a problem hiding this comment.
please review https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#changelog
this is not a customer-impacting change
There was a problem hiding this comment.
Thanks, I will remove this.
justinmk3
changed the title
sannicm
force-pushed
the
sannicm/sendMetric
branch
2 times, most recently
from
4277e0d
to
17c71ae
Compare
| @@ -587,6 +587,12 @@ export class FeatureDevController { | |||
| private async modifyDefaultSourceFolder(message: any) { | |||
| const session = await this.sessionStorage.getSession(message.tabID) | |||
|
|
|||
| telemetry.amazonq_modifySourceFolder.emit({ | |||
| amazonqConversationId: session.conversationId, | |||
| result: 'Succeeded', | |||
There was a problem hiding this comment.
We can use the result field of this metric to show that customers might be cancelling the file picker or selecting folders that are not in the current workspace, as we discussed the other day.
How about we change this modifyDefaultSourceFolder function to return boolean (returning false when we see an error with the selection, and only returning true after we're able to call session.updateWorkspaceRoot without problems), and then on the calling function we emit the amazonq_modifySourceFolder metric with a proper result field of Succeeded if the selection worked, or Failed if it didn't?
I see the caller does this
case FollowUpTypes.ModifyDefaultSourceFolder:
return this.modifyDefaultSourceFolder(data)
so we can just switch that to
case FollowUpTypes.ModifyDefaultSourceFolder:
const result = this.modifyDefaultSourceFolder(data)
telemetry.amazonq_modifySourceFolder.emit({
result
// ...
})
break
17c71ae
to
1eb61b9
Compare
| @@ -102,7 +102,7 @@ export class FeatureDevController { | |||
| getLogger().error('processChatItemVotedMessage failed: %s', (e as Error).message) | |||
| }) | |||
| }) | |||
| this.chatControllerMessageListeners.followUpClicked.event(data => { | |||
| this.chatControllerMessageListeners.followUpClicked.event(async data => { | |||
There was a problem hiding this comment.
Can't see an await in the block. Any reason this is explicitly declared async?
There was a problem hiding this comment.
leftover, I need to remove this.
| @@ -592,6 +592,8 @@ export class FeatureDevController { | |||
| canSelectFiles: false, | |||
| }).prompt() | |||
|
|
|||
| let metricData: MetricData | |||
There was a problem hiding this comment.
Since this MetricData type won't be used anywhere else, it's ok to use it inline here:
let metricData: { result: 'Succeeded' } | { result: 'Failed'; reason: string } | undefined;
36c78de
to
ed42329
Compare
sannicm
force-pushed
the
sannicm/sendMetric
branch
from
ed42329
to
c343227
Compare
sannicm
force-pushed
the
sannicm/sendMetric
branch
from
c343227
to
a4b7387
Compare
Yep, I was taking a look at this, I just pushed a fix, hopefully it passes now |
The fix is to skip tests? Will you follow up with unskipping or removing the tests? For now, can you add a comment in code as to why the test is being skipped? |
Oh no, I forgot to remove the skipping. Those are testing leftovers, I will update |
…eafult source folderwhile using /dev
sannicm
force-pushed
the
sannicm/sendMetric
branch
from
a4b7387
to
98676af
Compare
Problem
There is no current metric tracking the usage of this UI feature form /dev which allows customer to modify root source folder. This will hopefully help us calculate any impact if this feature gets broken in the future or also have an insight of the usage.
Solution
send the modifySourceFolder when customer clicks the button to do so on the chat.
License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.