telemetry: Metrics for amazonq unit test generation #905
Conversation
| { | ||
| "name": "numberOfBuildExecuted", | ||
| "type": "int", | ||
| "description": "Number of build executed in unit test generation build loop" | ||
| }, | ||
| { | ||
| "name": "numberOfBuildFailed", | ||
| "type": "int", | ||
| "description": "Number of build failures in unit test generation build loop" | ||
| }, | ||
| { | ||
| "name": "numberOfBuildPassed", | ||
| "type": "int", | ||
| "description": "Number of build succeeded in unit test generation build loop" | ||
| }, |
There was a problem hiding this comment.
there are already some generic "successCount" and similar fields. use those. if you need to add a executedCount or something like that, that is preferred.
do not add over-specific fields for every metric.
| "name": "numberOfUnitTestCasesAccepted", | ||
| "type": "int", | ||
| "description": "Number of unit test cases accepted" | ||
| }, | ||
| { | ||
| "name": "numberOfUnitTestCasesGenerated", | ||
| "type": "int", |
There was a problem hiding this comment.
use or create generic fooCount metrics.
please always do this in the future. I have mentiond this in previous reviews.
There was a problem hiding this comment.
Named these two fields to commonacceptedCount and generatedCount.
| "name": "supportedLanguage", | ||
| "type": "boolean", | ||
| "description": "supported language" | ||
| }, |
There was a problem hiding this comment.
there are multiple such fields already. do not add redundant fields.
There was a problem hiding this comment.
I didn't see a field which checks the supported language(boolean).
LMK If I am missing something.
There was a problem hiding this comment.
This field should be isSupportedLanguage to indicate it is boolean field. SupportedLanguage seems like we are going to pass the actual value of language which could be string/enum type.
Also the comment should be "Indicate if the language is supported"
There was a problem hiding this comment.
I did not see convention naming isXXXX for boolean values in the json file so for keeping consistent went with supportedLanguage
Let me change this accordingly
| "name": "linesOfCodeAccepted", | ||
| "type": "int", | ||
| "description": "Number of lines of code accepted" | ||
| }, | ||
| { | ||
| "name": "linesOfCodeGenerated", |
There was a problem hiding this comment.
do you really need to track chars and lines? how is that the existing codewhisperer metrics don't already have fields for this?
| "name": "linesOfCodeAccepted", | |
| "type": "int", | |
| "description": "Number of lines of code accepted" | |
| }, | |
| { | |
| "name": "linesOfCodeGenerated", | |
| "name": "acceptedLinesCount", | |
| "type": "int", | |
| "description": "Number of lines of code accepted" | |
| }, | |
| { | |
| "name": "generatedLinesCount", |
There was a problem hiding this comment.
They are using cwsprChatAcceptedNumberOfLines which is specific name not generic but trying to use the same.
| "name": "testGenerationJobGroupName", | ||
| "type": "string", | ||
| "description": "Unit test generation job group name" | ||
| }, | ||
| { | ||
| "name": "testGenerationJobId", |
There was a problem hiding this comment.
a group name and job id could be used in many metrics. please do not add hyper-specific fields for every metric.
I already see fields like codeTransformJobId, and that's another case where we're paying the price for those fields being over-specific.
| "name": "testGenerationJobGroupName", | |
| "type": "string", | |
| "description": "Unit test generation job group name" | |
| }, | |
| { | |
| "name": "testGenerationJobId", | |
| "name": "jobGroup", | |
| "type": "string", | |
| "description": "Job group name associated with the metric" | |
| }, | |
| { | |
| "name": "jobId", |
There was a problem hiding this comment.
Will update with this
laileni-aws
changed the title
Problem
Solution
Planning to add 4 metrics for Unit test generation.
License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.