Skip to content

Conversation

@keeganirby
Copy link
Contributor

Problem

Addresses feedback on #916.

Solution

  • Uses hasTextFilter instead of defining a new hasLogEventFilterPattern property
  • Defines a more generic filterPattern type, as an enum that supports logStream filter type strings
  • renames metrics to cloudwatchlogs_startLiveTail and cloudwatchlogs_stopLiveTail

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@keeganirby keeganirby requested a review from a team as a code owner December 5, 2024 18:04
"type": "result"
},
{
"type": "sessionAlreadyStarted",
Copy link
Contributor

Choose a reason for hiding this comment

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

Just FYI: if "sessionAlreadyStarted" is a kind of failure, it's strongly recommended to simply throw an exception and let telemetry.xx.run(...) automatically set result=Failed and reason=<code> where <code> is the error code. https://github.com/aws/aws-toolkit-vscode/blob/016e478399f4e4e050e5e644c98c96297930e8ad/packages/core/src/shared/errors.ts#L144

That has other benefits: all failures can be captured in dashboards/queries easily, without needing to know specific, special fields such as this one.

However, if this is only an informational field and doesn't represent a failure or cancellation, then the above doesn't apply.

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 don't consider this to be a failure case. This would be an informational field. The telemetry.xx.run(...) will not raise an exception.

@justinmk3 justinmk3 merged commit b047c14 into aws:main Dec 5, 2024
6 of 7 checks passed
justinmk3 pushed a commit to aws/aws-toolkit-vscode that referenced this pull request Dec 7, 2024
## Problem
Updating telemetry to accommodate changes in
aws/aws-toolkit-common#932

## Solution
Update package version for telemetry. 
Add validation on the LogStreamFilter submenu's response for filter
type. This is needed to allow the type returned from the
LogStreamFilterSubmenu to be convertible to the type in the metric
definition for filterPattern. In any case, this is a good validation to
have since the 'menu' placeholder value isn't valid for starting a
LiveTail session anyways.
karanA-aws pushed a commit to karanA-aws/aws-toolkit-vscode that referenced this pull request Jan 17, 2025
Updating telemetry to accommodate changes in
aws/aws-toolkit-common#932

Update package version for telemetry.
Add validation on the LogStreamFilter submenu's response for filter
type. This is needed to allow the type returned from the
LogStreamFilterSubmenu to be convertible to the type in the metric
definition for filterPattern. In any case, this is a good validation to
have since the 'menu' placeholder value isn't valid for starting a
LiveTail session anyways.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants