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.

Sign up for GitHub

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

telemetry: Adding metrics for amazonq unit test generation #915

Closed
wants to merge 6 commits into from
Closed

telemetry: Adding metrics for amazonq unit test generation #915

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e69d511
Adding metrics to UTG
laileni-aws Nov 15, 2024
797fb4a
Fixing lint issues
laileni-aws Nov 15, 2024
8cdfd45
Fixing lint issues
laileni-aws Nov 15, 2024
bd8b9d2
Adding jobGroup and jobId
laileni-aws Nov 15, 2024
da93589
Minor edits
laileni-aws Nov 23, 2024
fac1daa
Minor edits
laileni-aws Nov 23, 2024
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
243 changes: 243 additions & 0 deletions telemetry/definitions/commonDefinitions.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,20 @@
{
"types": [
{
"name": "acceptedCharactersCount",
"type": "int",
"description": "The number of accepted characters"
},
{
"name": "acceptedCount",
"type": "int",
"description": "The number of accepted cases"
},
{
"name": "acceptedLinesCount",
"type": "int",
"description": "The number of accepted lines of code"
},
{
"name": "action",
"type": "string",
Expand Down Expand Up @@ -178,11 +193,21 @@
"type": "int",
"description": "The amount of time required for the build to complete (in seconds)."
},
{
"name": "buildPayloadBytes",
"type": "int",
"description": "The uncompressed payload size in bytes of the source files in customer project context"
},
{
"name": "buildSystemVersion",
"type": "string",
"description": "The build system version on the user's machine"
},
{
"name": "buildZipFileBytes",
Copy link

Choose a reason for hiding this comment

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

what about number of files?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We does not have a field for Number of files and I do not think it adds a value as file can be of any size and number does not define our latency or test generation experience.
Correct me If I am missing something.

"type": "int",
"description": "The compressed payload size of source files in bytes of customer project context sent"
},
{
"name": "causedBy",
"type": "string",
Expand Down Expand Up @@ -1218,6 +1243,11 @@
"type": "string",
"description": "The name of the EventBridge Schema used in the operation"
},
{
"name": "executedCount",
"type": "int",
"description": "The number of executed operations"
},
{
"name": "experimentId",
"type": "string",
Expand Down Expand Up @@ -1284,6 +1314,21 @@
"type": "string",
"description": "Application framework being used"
},
{
"name": "generatedCharactersCount",
"type": "int",
"description": "Number of characters of code generated"
},
{
"name": "generatedCount",
Copy link

Choose a reason for hiding this comment

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

this name is unhelpful. why not generatedTestCasesCount?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please refer this PR
Long story short Toolkit want to reuse the fields so the ask is to keep this as generic as possible.

Copy link
Contributor

@justinmk3 justinmk3 Nov 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Each field is "namespaced" in a metric, so there is no need to name each field in a globally-unique way, and it's actually counterproductive (because it results in 10x or 100x the number of field names, which reduces discoverability, re-use, and leads to less-helpful dashboards and queries).

So for example, this field would live on a metric named amazon_foo and so amazonq_foo.generatedCount would clearly be understood based on the metric it is part of.

Copy link
Contributor

Choose a reason for hiding this comment

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

You probably don't need this field, just use the count field:

aws-toolkit-common/telemetry/definitions/commonDefinitions.json

Line 937 in fac1daa

"name": "count",

"type": "int",
"description": "The number of generated cases"
},
{
"name": "generatedLinesCount",
"type": "int",
"description": "The number of generated lines of code"
},
{
"name": "generateFailure",
"type": "string",
Expand All @@ -1307,6 +1352,11 @@
"type": "boolean",
"description": "A time based filter was used"
},
{
"name": "hasUserPromptSupplied",
"type": "boolean",
"description": "True if user supplied prompt message as input else false"
},
{
"name": "httpMethod",
"type": "string",
Expand Down Expand Up @@ -1405,6 +1455,21 @@
"type": "boolean",
"description": "Whether or not the operation was a retry"
},
{
"name": "isSupportedLanguage",
"type": "boolean",
"description": "Indicate if the language is supported"
},
{
"name": "jobGroup",
"type": "string",
"description": "Job group name used in the operation"
},
{
"name": "jobId",
"type": "string",
"description": "Job id used in the operation"
},
{
"name": "lambdaArchitecture",
"type": "string",
Expand Down Expand Up @@ -1696,6 +1761,11 @@
"type": "string",
"description": "Date/time that an SSO client registration expires."
},
{
"name": "step",
"type": "string",
"description": "Indicates the stage at which a user interface click action was performed."
},
{
"name": "successCount",
"type": "int",
Expand Down Expand Up @@ -2240,6 +2310,175 @@
}
]
},
{
"name": "amazonq_unitTestGeneration",
"description": "Client side metrics of AmazonQ Unit Test Generation",
"metadata": [
{
"type": "acceptedCharactersCount",
"required": false
},
{
"type": "acceptedCount",
"required": false
},
{
"type": "acceptedLinesCount",
"required": false
},
{
"type": "credentialStartUrl",
"required": false
},
{
"type": "cwsprChatProgrammingLanguage"
Copy link

Choose a reason for hiding this comment

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

why is this a chat programming language?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We planned to reuse existing fields as much as possible. Please refer this PR

},
{
"type": "executedCount",
"required": false
},
{
"type": "failedCount",
"required": false
},
{
"type": "generatedCharactersCount",
"required": false
},
{
"type": "generatedCount",
Copy link

Choose a reason for hiding this comment

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

same comment as above.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is on a metric named amazonq_unitTestGeneration. Can just use the count field:

aws-toolkit-common/telemetry/definitions/commonDefinitions.json

Line 937 in fac1daa

"name": "count",

each field does not need its own namespace; it's already namespaced in a metric.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Follow this suggestion.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, but count already exists... when a metric's main topic needs a count, just use the existing count field.

"required": false
},
{
"type": "generatedLinesCount",
"required": false
},
{
"type": "hasUserPromptSupplied"
},
{
"type": "isSupportedLanguage"
},
{
"type": "jobGroup",
"required": false
},
{
"type": "successCount",
"required": false
}
]
},
{
"name": "amazonq_utg_buildLoop",
"description": "Client side invocation of the AmazonQ Unit Test Generation build loop",
"metadata": [
{
"type": "credentialStartUrl",
"required": false
},
{
"type": "cwsprChatProgrammingLanguage"
},
{
"type": "hasUserPromptSupplied"
},
{
"type": "isSupportedLanguage"
},
{
"type": "jobGroup",
"required": false
},
{
"type": "jobId",
"required": false
},
{
"type": "perfClientLatency",
"required": false
},
{
"type": "result",
"required": false
},
{
"type": "source",
"required": false
}
]
},
{
"name": "amazonq_utg_generateTests",
"description": "Client side invocation of the AmazonQ Unit Test Generation",
Comment on lines +2412 to +2413
Copy link
Contributor

Choose a reason for hiding this comment

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

how is this different than amazonq_unitTestGeneration ?

        "name": "amazonq_unitTestGeneration",
       "description": "Client side metrics of AmazonQ Unit Test Generation",

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will be the E2E metric after Build execution loop

Copy link
Contributor

Choose a reason for hiding this comment

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

That isn't indicated in the description, and still doesn't explain how it's different

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not being added as part of RIV.

"metadata": [
{
"type": "acceptedCharactersCount",
"required": false
},
{
"type": "acceptedCount",
"required": false
},
{
"type": "acceptedLinesCount",
"required": false
},
{
"type": "artifactsUploadDuration",
"required": false
},
{
"type": "buildPayloadBytes",
"required": false
},
{
"type": "buildZipFileBytes",
"required": false
},
{
"type": "credentialStartUrl",
"required": false
},
{
"type": "cwsprChatProgrammingLanguage"
},
{
"type": "generatedCharactersCount",
"required": false
},
{
"type": "generatedCount",
Copy link
Contributor

Choose a reason for hiding this comment

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

This is on a metric named amazonq_unitTestGeneration. Can just use the count field:

aws-toolkit-common/telemetry/definitions/commonDefinitions.json

Line 937 in fac1daa

"name": "count",

each field does not need its own namespace; it's already namespaced in a metric.

"required": false
},
{
"type": "generatedLinesCount",
"required": false
},
{
"type": "hasUserPromptSupplied"
},
{
"type": "isSupportedLanguage"
},
{
"type": "jobGroup",
"required": false
},
{
"type": "jobId",
"required": false
},
{
"type": "perfClientLatency",
"required": false
},
{
"type": "source",
"required": false
}
]
},
{
"name": "amazonq_viewChatPanel",
"description": "Captures if Q chat panel is successfully viewed or not",
Expand Down Expand Up @@ -7035,6 +7274,10 @@
"metadata": [
{
"type": "elementId"
},
{
"type": "step",
"required": false
}
]
},
Expand Down