This repository has been archived by the owner on Aug 2, 2022. It is now read-only.
Simplify historical detector failure states #368
Merged
Conversation
This file contains 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
ohltyler
force-pushed
the
add-failed-state
branch
from
ac61c0b to
f1f1473
Compare
ohltyler
changed the title
ohltyler
force-pushed
the
add-failed-state
branch
from
f1f1473 to
21c6325
Compare
ohltyler
force-pushed
the
add-failed-state
branch
from
07e8cfd to
5afa2bf
Compare
ohltyler
changed the title
ohltyler
requested review from
yizheliu-amazon,
ylwu-amzn and
saratvemulapalli
ohltyler
added
the
enhancement
yizheliu-amazon
approved these changes
Feb 2, 2021
ylwu-amzn
approved these changes
Feb 3, 2021
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Issue #, if available:
Description of changes:
This PR simplifies and organizes the failure states for historical detectors. Currently, all failures are grouped into the
Unexpected failurestate, except for theNo datastate. Since there are other handled failures (e.g., "Not enough data to initialize"), these should also not be ignored, and we should provide some useful information to the user. However, creating more states similar toNo datastate introduces a lot of extra code, and makes it difficult to maintain compatibility on the frontend with all of the possible backend/ES failures.To simplify this, we group the failures into (1)
Unexpected failureand (2)Failedstates by parsing out the backend error message. If the message is a stack trace: we consider it unexpected, with no readable error message. If the message is anything else, we consider it handled, and propagate that error message to the frontend via a callout on the historical detector details page.Details & examples on the 4 possible types of error messages that need to be handled can be found in the function here and the corresponding unit test here.
Added unit tests & confirmed all are passing.
Screenshots:
Failed state with propagated ES error message:
Unexpected failure state:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.