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.

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

update environments telemetry to add hresults #3804

Merged
merged 3 commits into from
Sep 10, 2024

Conversation

bbonaby
Copy link
Contributor

@bbonaby bbonaby commented Sep 9, 2024

Summary of the pull request

Although we capture the error messaging, we don't capture the HResult error codes. Capturing these will provide us with more information that we can use to filter out noisy data a bit easier.

Changes:

  1. Added a TelemetryResult class that extracts data from the ProviderOperationResult so this doesn't have to be done by all the other telemetry classes.
  2. Added an HResult integer property for the EnvironmentOperationEvent, EnvironmentCreationEvent and the EnvironmentLaunchEvent classes. These also now take a TelemetryResult in their constructors to get the error information.

References and relevant issues

Detailed description of the pull request / Additional comments

Validation steps performed

I used the TraceView++ app to confirm that the events are now submitting the HResults, that are received by the operations in their payload.

PR checklist

  • Closes #xxx
  • Tests added/passed
  • Documentation updated

@krschau krschau added this to the Dev Home v0.18 milestone Sep 9, 2024
Copy link
Collaborator

@krschau krschau left a comment

Choose a reason for hiding this comment

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

Hope you don't mind, I pushed a commit with only whitespace changes, so you'll need to pull if you make any other changes.

@bbonaby
Copy link
Contributor Author

bbonaby commented Sep 10, 2024

Hope you don't mind, I pushed a commit with only whitespace changes, so you'll need to pull if you make any other changes.

Nope I don't mind at all, thanks @krschau !

@bbonaby bbonaby merged commit a1d7c69 into main Sep 10, 2024
4 checks passed
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.

3 participants