Repository navigation
fix(telemetry): report a sanitized cause for device setting and app control failures - #61
Merged
Merged
Conversation
…ontrol failures Device setting and app control failures reached Sentry as one fixed message with no cause. Each failure now names the command or backend route it came from and how it failed, as low-cardinality tags read from error codes, exit and HTTP statuses, and error types. Messages and output stay out of reports. A device setting request on a device that stopped or went away now counts as device_unavailable instead of a report. The check reads the device list without starting a backend. App control also treats a simulator that is shutting down as unavailable. Closes #49
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes #49.
Device setting and app control failures reach Sentry as
Device setting command failed.orApp <action> command failed.with no cause. MOBILE-DEV-SERVER-8S alone has 275 events from 69 users, almost alldevice_settings.read, and none of them say what broke.What changes
Reports keep their fixed messages, so the existing issues keep grouping. They now carry these tags, which the
server.error.outcomecount also gets:device_platform: from the tool's target. Many events had no platform before.device_command: where the failure came from, such assimctl_launch,adb_shell_am,baguette_interfaceorserve_emu_start. Baguette and serve-emu label their own request failures. The app control drivers labelsimctl,adbandplutilfailures.device_command_failure:exit,signal,deadline,http_status,rejected,invalid_response,connection_failed,unexpected_outputand a few more. It comes from error codes, exit and HTTP statuses, and error types. Messages and output are never read.device_command_statusanddevice_command_signal.When a device setting request fails, the service looks the device up again without starting a backend. If the device stopped or went away, the failure counts as
device_unavailableand isn't reported. If the device list can't be read, the original failure is reported as before. App control now treats "current state: Shutting Down" and a few other device-gone messages as unavailable too.captureServerErrortakes areportAserror and extratags. Diagnostic tags, including serve-emu startup tags, still come from the original error. The replacement error is created where the failure is caught, so Sentry keeps the same code location.Tests
packages/shared/test/device-command-diagnostics.test.tscovers the classification, the labels, and the device-gone patterns.telemetry.test.tschecks the tags on real reports, that nothing private is sent, and that a simulator stopping mid-read creates no report. That test runs the real Baguette client against a local HTTP server.device-settings.test.tschecks the device-gone lookup: stopped, gone, and an unreadable device list.npm testpasses except two tests outside this change. The relocated Baguette test needs the vendored Baguette binary and fails onmainin my checkout too. The TraceSift startup test failed once under parallel load and passed three runs on its own.