Skip to content

LG-6668 safeguard failed message#6647

Merged
peggles2 merged 12 commits intomainfrom
LG-6668-safeguard_failed_message
Aug 3, 2022
Merged

LG-6668 safeguard failed message#6647
peggles2 merged 12 commits intomainfrom
LG-6668-safeguard_failed_message

Conversation

@peggles2
Copy link
Contributor

This pull request adds a safe guard so that if the alert had failed and passed, that the failed alert is logged and not written over. The chances currently of this happening is zero because the 'Passed alerts are processed first' but as a safeguard, I added a check to ensure failed is always logged and updated the test.

Currently a work in progress and not review for review just yet.

peggles2 added 4 commits July 28, 2022 11:12
changelog: Improvements, logging, safeguard duplicate success, failed alerts to show failed, during document authentication
@kbighorse
Copy link
Contributor

It looks like perhaps the identical #log_alerts methods can be DRYed up somehow? Or perhaps that can wait for a dedicated issue if there are other such duplicated methods.

@zachmargolis
Copy link
Contributor

It looks like perhaps the identical #log_alerts methods can be DRYed up somehow? Or perhaps that can wait for a dedicated issue if there are other such duplicated methods.

+1 I think it would be nice, could have a Helper class with a static method or a mixin shared across both places

@peggles2
Copy link
Contributor Author

peggles2 commented Jul 29, 2022

Ok so now the code has been changed so that the values are comma delimited, this means passed and failed can be stored in the same alert as well.
For example
{ visible_pattern: { no_side: 'Passed, Failed, Skipped, Caution' }, document_classification: { no_side: 'Failed' } },
or
visible_pattern: { no_side: 'Passed, Failed' }

Nothing is written over. CW query will use a like to wildcard the searches for the result values.

@peggles2 peggles2 requested review from solipet and zachmargolis July 29, 2022 21:14
@zachmargolis
Copy link
Contributor

Ok so now the code has been changed so that the values are comma delimited, this means passed and failed can be stored in the same alert as well. For example { visible_pattern: { no_side: 'Passed, Failed, Skipped, Caution' }, document_classification: { no_side: 'Failed' } }, or visible_pattern: { no_side: 'Passed, Failed' }

Nothing is written over. CW query will use a like to wildcard the searches for the result values.

I think adding commas like that makes things hard to aggregate. Yes, we coud use a "LIKE" query in CW, but that still harms the | stats count(*) by ... approach

Can we do the monitor-only version of this for a week or so, and the re-evaluate based on how often multiple errors occur?

@peggles2 peggles2 requested a review from jmhooper August 1, 2022 21:00
@peggles2
Copy link
Contributor Author

peggles2 commented Aug 2, 2022

Refactored duplicate methods into one service class method. Changed the code so that if there is a duplicate for now to just output to the log file so we can see how frequently this occurs first.

Copy link
Contributor

@jmhooper jmhooper left a comment

Choose a reason for hiding this comment

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

LGTM! Let's throw a changelog commit on here. Looks like CI is upset about that

changelog: Improvements, docuauth Logs, log warning when duplicate messages from same alert occures
@peggles2 peggles2 force-pushed the LG-6668-safeguard_failed_message branch from 1797f18 to 860077a Compare August 3, 2022 14:18
@peggles2 peggles2 merged commit ea46a53 into main Aug 3, 2022
@peggles2 peggles2 deleted the LG-6668-safeguard_failed_message branch August 3, 2022 15:39
@mitchellhenke mitchellhenke mentioned this pull request Aug 3, 2022
@solipet solipet mentioned this pull request Aug 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants