[Logs UI] Log threshold ratio alerts #76867

Kerry350 · 2020-09-07T11:42:21Z

Summary

This primarily closes #72648, it also closes #73274 and closes #72453. It adds the ability to create ratio based alerts for the log threshold alert type.

Implementation notes

I've stuck with the mathematics principle that dividing by 0 isn't possible, as such if either value is 0 then a ratio is undefined / indeterminate. Given an undefined ratio the alert will not fire.
Ratio alerts are, realistically, what we have already but two sets of criteria compared against each other, as such a lot of the executor logic is reused, but with new ratio results processors and context variables.
Threshold annotation rendering is turned off for ratio alerts on chart previews.
I've moved around some folders in preparation for us adding more alert types (e.g. the anomaly alert type). The diff seems to have marked some (not all) of the renames as new files (validation, threshold component etc) so it looks like more code has been added there than in reality.

Testing

Alerting requires SSL (yarn start --ssl and yarn es snapshot --ssl)
Quite a lot has been touched, so it would be good to not only verify the new ratio alerts, but also standard count based alerts. Things like UI validation should try to be broken etc.

elasticmachine · 2020-09-07T11:42:24Z

Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui)

jasonrhodes

I'm taking a look at this PR this week 👍

jasonrhodes

Looks good so far! I'm going to take a look at the code more deeply next, but I'm going to add UX based comments individually so far.

jasonrhodes · 2020-09-09T18:54:10Z

Missing some padding/separation between these sections now, I think.

jasonrhodes · 2020-09-09T18:54:49Z

The loading indicator when opening a new alert is up in the top left corner and not centered (or padded from the edge). Not sure if this is new or not, though.

jasonrhodes · 2020-09-09T18:55:54Z

I assume this means "more than 75%"? We probably need the % indicator to help with that understanding, both in the form and in the display here. Or use 0.75? Something to help me understand what that number is?

jasonrhodes · 2020-09-09T18:58:25Z

At first I was admittedly pretty lost with what is going on here. It took me a while to realize that when I select "ratio" from the "When" dropdown, I get two separate condition "sets" added and that I can now add and remove conditions to each set which are then evaluated together as a ratio. I'm not sure I have a ton of great ideas to fix this, but as far as a small initial tweak maybe indentation would help? I do think this type of SQL UX may be starting to break down a bit, though, more generally.

@katrin-freihofner do you have thoughts here?

Kerry350 · 2020-09-10T11:38:30Z

Missing some padding/separation between these sections now, I think.

Yeah 👍

The loading indicator when opening a new alert is up in the top left corner and not centered (or padded from the edge). Not sure if this is new or not, though.

This is old, I think this was originally due to the fact we don't really control the initial rendering space, the framework runs our expression code and pops it in there. Things may have changed by now, and positioning that spinner might be easier.

I assume this means "more than 75%"? We probably need the % indicator to help with that understanding, both in the form and in the display here. Or use 0.75? Something to help me understand what that number is?

This is the straight up ratio value, as it's the ratio of the count of Case A to the count of Case B. Or CaseA / CaseB. Admittedly 75 isn't a great default, it's what the count type uses, something like 2 would be better for twice as many.

It could be converted to percentages.

I went with the actual ratio values as that's what other ratio alerting products seemed to use. With ratios I think most people are accustomed to the 3:1 (numerator:denominator) format, and therefore plugging in a real value.

We could make it clear that this is CaseA / CaseB, here is an example from elsewhere:

At first I was admittedly pretty lost with what is going on here. It took me a while to realize that when I select "ratio" from the "When" dropdown, I get two separate condition "sets" added and that I can now add and remove conditions to each set which are then evaluated together as a ratio. I'm not sure I have a ton of great ideas to fix this, but as far as a small initial tweak maybe indentation would help? I do think this type of SQL UX may be starting to break down a bit, though, more generally.

I don't have any real ideas here. I did a literal implementation of what was in #72648, which was When ratio of the count of log entries with alert condition1 AND alert condition2 to the count of log entries with alert condition3 AND alert condition4 is threshold within the last 5 minutes then action., when I spoke with @katrin-freihofner it was agreed there'd be no specific design for this, and what was in that ticket was enough.

katrin-freihofner · 2020-09-16T10:20:27Z

@jasonrhodes I'm not sure I fully understand the problem. And @Kerry350 did not get any design help as we thought this can move directly to engineering.
From what I see in the screenshots it is what was asked for but I agree, it looks confusing.
I'm wondering if the new expression layout could help here. This is an example:

Also, I think we need to move the chart to the bottom. That said, this is something I would really like to do across Observability and not application by application. Let me know what you are thinking.

jasonrhodes

From what I see in the screenshots it is what was asked for but I agree, it looks confusing.
I'm wondering if the new expression layout could help here.

@katrin-freihofner Can we talk about this option at Monday's meeting? This is the kind of solution I'm looking for, for right now -- just something to help orient the user to how these conditions are grouped/organized. We can re-think beyond that after this ticket, if needed.

Also, I think we need to move the chart to the bottom. That said, this is something I would really like to do across Observability and not application by application. Let me know what you are thinking.

Yeah, this topic has been coming up a lot, and there are valid reasons to keep it where it is but there is also a lot going on with these condition rows. Can we schedule a session at some point next week to talk about Alerting Design overall? I don't think it's urgent but it would be good to start thinking through whether the SQL-style design is going to continue to work or if we need something else, and I think the per-condition previews are one aspect of many there. I'll put something on the calendar and include some folks.

...k/plugins/infra/public/components/alerting/logs/log_threshold/expression_editor/criteria.tsx

jasonrhodes · 2020-09-18T18:03:40Z

This is old, I think this was originally due to the fact we don't really control the initial rendering space, the framework runs our expression code and pops it in there. Things may have changed by now, and positioning that spinner might be easier.

If it's not super simple, we can address this in another ticket. Not a huge deal.

jasonrhodes · 2020-09-18T18:04:53Z

This is the straight up ratio value, as it's the ratio of the count of Case A to the count of Case B. Or CaseA / CaseB. Admittedly 75 isn't a great default, it's what the count type uses, something like 2 would be better for twice as many.

It could be converted to percentages.

I went with the actual ratio values as that's what other ratio alerting products seemed to use. With ratios I think most people are accustomed to the 3:1 (numerator:denominator) format, and therefore plugging in a real value.

Ahh ok this makes sense. Changing the default should remove the confusion I had. Sounds good.

…ld-ratio-alerts

Kerry350 · 2020-09-24T14:00:25Z

@jasonrhodes This is ready for another look, I've addressed all of the feedback here. Recap on the main bits:

New design is in place

Used a default of 2 vs 75 for ratio alert thresholds
Removed all type casting in favour of user-defined type guards

A note on the loading spinner: this is actually the spinner from the alerting framework, and not something we control. The spinner we control is this one (which is when we're loading the fields information, if needed):

So I haven't changed anything there.

Kerry350 · 2020-09-28T14:32:36Z

@elasticmachine merge upstream

Kerry350 · 2020-09-29T10:09:21Z

@elasticmachine merge upstream

kibanamachine · 2020-09-29T11:36:58Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: e97686f

Metrics [docs]

@kbn/optimizer bundle module count

id	value	diff	baseline
`infra`	1126	+1	1125

async chunks size

id	value	diff	baseline
`infra`	4.2MB	+11.3KB	4.2MB

page load bundle size

id	value	diff	baseline
`infra`	137.7KB	+3.2KB	134.5KB

History

💔 Build #77440 failed e46a9c7
💚 Build #76928 succeeded cdcc4b1
💚 Build #72922 succeeded fbc1283
💔 Build #72914 failed 8290090

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

jasonrhodes

LGTM @Kerry350 ! Thanks so much for working with us on getting this ready to merge!

* Add ratio alerting to log threshold alerts * Fix i18n * Move grouped query must not filtering from outer to inner clause * Use new ratio alerting layout * Use better defaults for ratio alerts * Remove div wrapper * Remove type casting, use user-defined type guards Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Kerry Gallagher <k.gallagher.05@gmail.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Add ratio alerting to log threshold alerts

8290090

Kerry350 added release_note:enhancement v8.0.0 Feature:Logs UI Logs UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v7.10.0 labels Sep 7, 2020

Kerry350 added this to the Logs UI 7.10 milestone Sep 7, 2020

Kerry350 requested a review from a team September 7, 2020 11:42

Kerry350 self-assigned this Sep 7, 2020

Fix i18n

fbc1283

jasonrhodes reviewed Sep 8, 2020

View reviewed changes

jasonrhodes reviewed Sep 9, 2020

View reviewed changes

jasonrhodes reviewed Sep 18, 2020

View reviewed changes

...k/plugins/infra/public/components/alerting/logs/log_threshold/expression_editor/criteria.tsx Outdated Show resolved Hide resolved

...k/plugins/infra/public/components/alerting/logs/log_threshold/expression_editor/criteria.tsx Outdated Show resolved Hide resolved

Kerry350 added 7 commits September 23, 2020 11:57

Merge remote-tracking branch 'upstream/master' into 72648-log-thresho…

20a8eb7

…ld-ratio-alerts

Move grouped query must not filtering from outer to inner clause

e07d469

Use new ratio alerting layout

20dea59

Use better defaults for ratio alerts

df8f422

Remove div wrapper

65f4a7d

Remove type casting, use user-defined type guards

822f5aa

Merge remote-tracking branch 'upstream/master' into 72648-log-thresho…

cdcc4b1

…ld-ratio-alerts

Merge branch 'master' into 72648-log-threshold-ratio-alerts

e46a9c7

Merge branch 'master' into 72648-log-threshold-ratio-alerts

e97686f

jasonrhodes approved these changes Sep 29, 2020

View reviewed changes

jasonrhodes merged commit 33d051b into elastic:master Sep 29, 2020

jasonrhodes mentioned this pull request Sep 29, 2020

[7.x] [Logs UI] Log threshold ratio alerts (#76867) #78852

Merged

This was referenced Sep 30, 2020

[Logs UI] Move alerting components to public/alerting #76028

Closed

[Logs UI] Update log threshold alert docs elastic/observability-docs#155

Merged

This was referenced Jul 5, 2021

[Logs UI][Test Scenario] Log threshold alerts (general) #104297

Closed

[Logs UI] [Test Scenario] Log threshold alerts (count configuration) #104353

Closed

[Logs UI] [Test Scenario] Log threshold alerts (ratio configuration) #104354

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Logs UI] Log threshold ratio alerts #76867

[Logs UI] Log threshold ratio alerts #76867

Kerry350 commented Sep 7, 2020

elasticmachine commented Sep 7, 2020

jasonrhodes left a comment

jasonrhodes left a comment

jasonrhodes commented Sep 9, 2020

jasonrhodes commented Sep 9, 2020

jasonrhodes commented Sep 9, 2020

jasonrhodes commented Sep 9, 2020

Kerry350 commented Sep 10, 2020

katrin-freihofner commented Sep 16, 2020

jasonrhodes left a comment •

edited

Loading

jasonrhodes commented Sep 18, 2020

jasonrhodes commented Sep 18, 2020

Kerry350 commented Sep 24, 2020

Kerry350 commented Sep 28, 2020

Kerry350 commented Sep 29, 2020

kibanamachine commented Sep 29, 2020

jasonrhodes left a comment

[Logs UI] Log threshold ratio alerts #76867

[Logs UI] Log threshold ratio alerts #76867

Conversation

Kerry350 commented Sep 7, 2020

Summary

Implementation notes

Testing

elasticmachine commented Sep 7, 2020

jasonrhodes left a comment

Choose a reason for hiding this comment

jasonrhodes left a comment

Choose a reason for hiding this comment

jasonrhodes commented Sep 9, 2020

jasonrhodes commented Sep 9, 2020

jasonrhodes commented Sep 9, 2020

jasonrhodes commented Sep 9, 2020

Kerry350 commented Sep 10, 2020

katrin-freihofner commented Sep 16, 2020

jasonrhodes left a comment • edited Loading

Choose a reason for hiding this comment

jasonrhodes commented Sep 18, 2020

jasonrhodes commented Sep 18, 2020

Kerry350 commented Sep 24, 2020

Kerry350 commented Sep 28, 2020

Kerry350 commented Sep 29, 2020

kibanamachine commented Sep 29, 2020

💚 Build Succeeded

Metrics [docs]

@kbn/optimizer bundle module count

async chunks size

page load bundle size

History

jasonrhodes left a comment

Choose a reason for hiding this comment

jasonrhodes left a comment •

edited

Loading