fix(crash): Add more telemetry info to crash reporting #5850
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is in preparation for a more simple implementation in the next commit Signed-off-by: nkomonen-amazon <[email protected]>
Signed-off-by: nkomonen-amazon <[email protected]>
- Increase check interval from 10 to 30 minutes, so 15 minute heartbeats. - Emit failure if heartbeat abort itself fails. This will let us know that if heartbeats fail and we try to clean them up, that the cleanup failed. We need to know this otherwise there will be incorrectly reported crashes. - Emit a metric when we detect a computer restart and handle it - Validate that when we write a new heartbeat we can immediately read it from disk. Signed-off-by: nkomonen-amazon <[email protected]>
- Now we emit an event for every heartbeat (every 10 minutes) - Include a timestamp field in the heartbeat and session_end so we can see the diff if any Signed-off-by: nkomonen-amazon <[email protected]>
Signed-off-by: nkomonen-amazon <[email protected]>
We do not need the PID anymore since we fixed the sessionId so that a new one is generated even after the ext host crashes and restarts. The PID previously allowed us to differentiate between ide instances with the same sessionId but different PIDs Signed-off-by: nkomonen-amazon <[email protected]>
simplify the code that deletes the heartbeat file Signed-off-by: nkomonen-amazon <[email protected]>
Signed-off-by: nkomonen-amazon <[email protected]>
Signed-off-by: nkomonen-amazon <[email protected]>
Signed-off-by: nkomonen-amazon <[email protected]>
|
This pull request implements a feature or fix, so it must include a changelog entry. See CONTRIBUTING.md#changelog for instructions. |
hayemaxi
reviewed
Oct 24, 2024
Comment on lines
49
to
59
| it('should return the first 4 and last 4 characters of a valid UUID', function () { | ||
| const fullUUID = 'aaaabbbb-cccc-dddd-eeee-ffffhhhhiiii' | ||
| const result = truncateUuid(fullUUID) | ||
| assert.strictEqual(result, 'aaaa...iiii') | ||
| }) | ||
|
|
||
| it('should handle a different valid UUID correctly', function () { | ||
| const fullUUID = '12340000-0000-0000-0000-000000005678' | ||
| const result = truncateUuid(fullUUID) | ||
| assert.strictEqual(result, '1234...5678') | ||
| }) |
Contributor
There was a problem hiding this comment.
These tests are the same?
Contributor
Author
There was a problem hiding this comment.
good point, merged with the other
hayemaxi
reviewed
Oct 24, 2024
Comment on lines
+86
to
+89
| // - We do not support cloud desktop due to false crash reports due to sleep+vpn+ssh timeouts with the IDE | ||
| if (isWeb() || (await getComputeEnvType()) === 'cloudDesktop-amzn') { | ||
| return undefined | ||
| } |
Contributor
There was a problem hiding this comment.
Why not local only? Wouldn't any remote environment potentially have this issue?
Contributor
Author
There was a problem hiding this comment.
There is a lot of noise caused specifically by cloud desktop, compared to the other ssh'd instances it is not much. This is probably since we have lots of Amazon devs using it.
The goal is to get rid of the noise while still collecting data on the others so that if we need it in the future it is there.
Contributor
There was a problem hiding this comment.
would that apply to many/most "remote" situations?
Signed-off-by: nkomonen-amazon <[email protected]>
Handles a possible edge case when sending a heartbeat Signed-off-by: nkomonen-amazon <[email protected]>
hayemaxi
approved these changes
Oct 24, 2024
Contributor
Author
|
/runIntegrationTests |
justinmk3
reviewed
Oct 24, 2024
| ] | ||
| }, | ||
| { | ||
| "name": "ide_heartbeat", |
Contributor
There was a problem hiding this comment.
toolkit_heartbeat may fit better. ide_ is for non-extension evens (from the ide)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
We are seeing weird issues with some users when crash reporting due to crashes for a specific session id being reported multiple times, even though it should report one.
Solution
Add more telemetry data so that we can see when the heartbeat is successfully sent, and what the crash checker sees when it runs.
Additional
There are other small commits that make small changes, each has a description for what it is doing
License: I confirm that my contribution is made under the terms of the Apache 2.0 license.