feat(awsq-telemetery): Update awsq_ metrics #595
Conversation
| "name": "awsq_serviceErrors", | ||
| "description": "Number of errors from engineering and service availability?", | ||
| "unit": "Count", | ||
| "metadata": [{ "type": "awsqServiceErrorsCount" }] |
There was a problem hiding this comment.
This should not be needed. If it is, then there's something wrong with the application logic. Failed metrics in the application (Toolkit) should emit with Result=Failed
aws-toolkit-common/telemetry/definitions/commonDefinitions.json
Lines 51 to 52 in 13cccc3
This is handled automatically if you structure the code to use telemetry.run() as documented in https://github.com/aws/aws-toolkit-vscode/blob/master/docs/telemetry.md
| "unit": "Count", | ||
| "metadata": [{ "type": "awsqFilesChangedCount" }] | ||
| }, | ||
| { | ||
| "name": "awsq_thumbsUp", | ||
| "description": "User clicked on the thumbs up button, to mention that they are satisfied", | ||
| "unit": "Count", | ||
| "metadata": [{ "type": "awsqThumbsUpCount" }] | ||
| }, | ||
| { | ||
| "name": "awsq_thumbsDown", | ||
| "description": "User clicked on the thumbs down button to say that they are unsatisfied", | ||
| "unit": "Count", | ||
| "metadata": [{ "type": "awsqThumbsDownCount" }] | ||
| }, |
There was a problem hiding this comment.
in all of these cases, the unit: count declaration means the value can be set on the metric without the need for a special-purpose field (awsqFilesChangedCount, awsqThumbsUpCount, ...).
Please look at existing examples in this file, as well as usages in the Toolkit, to understand how things can be shaped.
For example, aws_loadCredentials has unit:Count :
and this is how it's used in the Toolkit (
value: 1): https://github.com/aws/aws-toolkit-vscode/blob/61737287d918f777ea2e4a389ffea08fcc950adc/src/auth/providers/credentialsProviderManager.ts#L32
There was a problem hiding this comment.
from the example given it is not clear that the metadata value can be omitted.
telemetry.aws_loadCredentials.emit({ credentialSourceId: telemType, value: 1 })
it seems like it increments on credentialSourceId.
I will remove Count values
justinmk3
changed the title
| { | ||
| "name": "awsq_repo", | ||
| "description": "The size of the input repository", | ||
| "metadata": [{ "type": "awsqRepositorySize" }] |
There was a problem hiding this comment.
this metric could be "unit": "Count", then awsqRepositorySize is not needed.
Alternatively, please consider naming the type as filesize or directorysize so that it can be re-used in other metrics.
| }, | ||
| { | ||
| "name": "awsq_filesChanged", | ||
| "description": "The numbed of files suggested to change", |
There was a problem hiding this comment.
| "description": "The numbed of files suggested to change", | |
| "description": "Number of files suggested to change", |
Problem
need to have awsq_ metrics individually, not under one entity like
awsq_serviceInvocation, as we need to emit those metrics separately not all at onceSolution
License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.