Custom equation editor for Metric Threshold Rule #148732

simianhacker · 2023-01-11T15:04:31Z

Summary

This PR closes #145444 by adding a custom equation editor to the Metric Threshold rule. I also added support for custom metrics to the Metric Explorer API which powers the preview chart on the rule editor. Eventually we could do a follow up PR to the Metrics Explorer UI to expose this new functionality; which is outside the scope of this PR.

Notable changes with this PR

I changed the reason message for Metric Threshold rules which do not have a group by. The original message would say something like system.cpu.user.pct is 82% in the last 1 min for all hosts. Alert when > 81%. I removed the for all hosts portion because the Metric Threshold rule is not limited to just the concept of hosts, our users rely on this rule as their "Swiss Army Knife" rule for all types of data.

I also had to change the format of the currentPeriod bucket for the Metric Threshold aggregation to support the "document count with KQL filter" use case. One of the requirements of a filter aggregation is that it must be a child of a multi-bucket aggregation. This is why I converted it from a 'filter' aggregation to a filters aggregation with an all key for the time range query.

I added basic validation for the equations with a regular expression that just limits the characters to the allowable: A-Z, +, -, /, *, (, ), ?, !, &, :, |, >, <, =. I feel like for now this is good enough. If we want to expose some of the Painless Math.* libraries then we can follow up in a later release with a PegJS parser which would do some syntax validation as well.

Rule with custom equation

Rule with custom ratio equation

Reason message with custom label

StephanErb · 2023-01-12T20:49:07Z

Would you intend that this shall have the full power of Lens Formular's or do yo have a different scope in mind? (https://www.elastic.co/guide/en/kibana/current/lens.html#lens-formulas )

…stom-metric-editor-for-mtr

simianhacker · 2023-01-19T23:26:21Z

@StephanErb We are exploring ideas for an expressive rule type in the future which will be based on something like the Len's expressions or ESQL. If and when we deliver that, it will be a separate rule type.

…stom-metric-editor-for-mtr

simianhacker · 2023-01-30T14:59:36Z

@maryam-saeidi @crespocarlos Yes, it's expected that Document count's filter does not need a value. That way you can defined 2 "Document count" aggregation, one with a filter and one without. This enables the user to do "filter ratios" where they might want to alert on error ratios where the numerator is event.type: error and the denominator is "everything" (blank filter).

…stom-metric-editor-for-mtr

simianhacker · 2023-01-30T18:25:12Z

I had a Zoom with @maryam-saeidi and we figured out how to reproduce the validation error she was seeing:

Create a new rule, give it a title and select "Metric Threshold"
Change the condition to `WHEN average OF system.cpu.user.pct IS ABOVE 80"
Change average to custom equation
Configure a custom equation
Save
You should see a validation error.

…tom equation

crespocarlos

LGTM! thanks for applying the suggestions and fixing the bugs.

maryam-saeidi

The validation when changing condition from average to custom works but it does not work the other way around

I added some comments as well.

x-pack/plugins/infra/public/pages/metrics/metrics_explorer/components/aggregation.tsx

maryam-saeidi · 2023-01-31T12:48:12Z

...blic/alerting/metric_threshold/components/custom_equation/custom_equation_editor.stories.tsx

+import { validateMetricThreshold } from '../validation';
+
+export default {
+  title: 'infra/alerting/CustomEquationEditor',


...blic/alerting/metric_threshold/components/custom_equation/custom_equation_editor.stories.tsx

...infra/public/alerting/metric_threshold/components/custom_equation/custom_equation_editor.tsx

x-pack/plugins/infra/server/lib/metrics/index.ts

…stom-metric-editor-for-mtr

… changing to other aggreation types

Co-authored-by: Maryam Saeidi <maryam.saeidi@elastic.co>

simianhacker · 2023-01-31T17:03:37Z

@maryam-saeidi Thanks for the review! I've addressed all the issue you found. Please give it another review, thanks in advance.

kibana-ci · 2023-01-31T18:04:33Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`infra`	1242	1248	+6

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`infra`	1.3MB	1.3MB	+7.3KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`infra`	85.9KB	87.0KB	+1.0KB

Unknown metric groups

async chunk count

id	before	after	diff
`infra`	18	19	+1

History

💚 Build #104254 succeeded 165c274
💚 Build #103927 succeeded ef640c1
💛 Build #103812 was flaky d11e1d6
💚 Build #103484 succeeded 9035ade
💚 Build #103389 succeeded 14ff4d5
💚 Build #103187 succeeded 7374389

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

maryam-saeidi

Tested locally, and it worked as expected; fantastic job! 🎉

Just to double-check something here, when I test on a fresh instance everything is working fine, but when I use my oblt instance, I get the following error:

Is it because of some recent changes in this PR, so the rule that I created with the earlier version of this PR is not valid anymore? What can cause this issue? (Maybe it is not related to this PR at all ¯_(ツ)_/¯)

simianhacker · 2023-02-01T14:59:20Z

@maryam-saeidi That's an error in a different rule:

kibana/x-pack/plugins/infra/common/alerting/logs/log_threshold/types.ts

Line 13 in 9b85acb

export const LOG_DOCUMENT_COUNT_RULE_TYPE_ID = 'logs.alert.document.count';

## Summary This PR closes elastic#145444 by adding a custom equation editor to the Metric Threshold rule. I also added support for custom metrics to the Metric Explorer API which powers the preview chart on the rule editor. Eventually we could do a follow up PR to the Metrics Explorer UI to expose this new functionality; which is outside the scope of this PR. ### Notable changes with this PR I changed the reason message for Metric Threshold rules which do not have a group by. The original message would say something like `system.cpu.user.pct is 82% in the last 1 min for all hosts. Alert when > 81%.` I removed the `for all hosts` portion because the Metric Threshold rule is not limited to just the concept of hosts, our users rely on this rule as their "Swiss Army Knife" rule for all types of data. I also had to change the format of the `currentPeriod` bucket for the Metric Threshold aggregation to support the "document count with KQL filter" use case. One of the requirements of a `filter` aggregation is that it must be a child of a multi-bucket aggregation. This is why I converted it from a 'filter' aggregation to a `filters` aggregation with an `all` key for the time range query. I added basic validation for the equations with a regular expression that just limits the characters to the allowable: `A-Z, +, -, /, *, (, ), ?, !, &, :, |, >, <, =`. I feel like for now this is good enough. If we want to expose some of the Painless `Math.*` libraries then we can follow up in a later release with a PegJS parser which would do some syntax validation as well. ### Rule with custom equation <img width="538" alt="image" src="https://user-images.githubusercontent.com/41702/213583128-1adbc405-828e-4571-aeb4-9900baeaabee.png"> ### Rule with custom ratio equation <img width="538" alt="image" src="https://user-images.githubusercontent.com/41702/213583239-a39d15d2-7023-4daf-af97-cb25a9965433.png"> ### Reason message with custom label ![image](https://user-images.githubusercontent.com/41702/211936062-4b696f0c-dfec-4e48-b89c-b0462fb5f7f0.png) --------- Co-authored-by: Carlos Crespo <crespocarlos@users.noreply.github.com> Co-authored-by: Maryam Saeidi <maryam.saeidi@elastic.co>

Adding Custom Metrics (with math) to Metric Threshold Rule

4608c5b

simianhacker changed the title ~~Adding Custom Metrics (with math) to Metric Threshold Rule~~ Custom Metrics (with math) for Metric Threshold Rule Jan 11, 2023

Adding label, implimenting backend, fixing bugs

681bc04

Merge branch 'main' of github.com:elastic/kibana into issue-145444-cu…

6ab6909

…stom-metric-editor-for-mtr

simianhacker changed the title ~~Custom Metrics (with math) for Metric Threshold Rule~~ Custom equation editor for Metric Threshold Rule Jan 19, 2023

simianhacker added 21 commits January 23, 2023 07:50

Applying new design

a058a42

Merge branch 'main' of github.com:elastic/kibana into issue-145444-cu…

ecbaf7d

…stom-metric-editor-for-mtr

Add i18n strings

9edd8cd

Adding equation validation

455b8cf

Breaking apart and renaming the custom metric component

7d94384

Merge branch 'main' of github.com:elastic/kibana into issue-145444-cu…

3b3917b

…stom-metric-editor-for-mtr

Add type checking to custom metric formatter

d0127d4

Fixing types and filter custom metric from Metric Exporer aggs

a48828c

Excluding customMetrics in favor of the optional override

7d636aa

Cleaning up types

e2fc47f

Updating test to reflect new reson message

3c1e11d

Removing customMetrics default from expression inti

3395feb

Fixing types in test data

d81704f

Making sure the actual keys match the type

a3ccc83

Create and use a constant for ommited aggregations for custom metrics

403d847

Remove unnecessary check for customMetrics

5400060

Consolidating static labels

d93cda1

Adding integration test for Metric Explorer API

be8a4e5

Adding integration test for custom metric with document count

e86504d

Adding unit test for equation validation

6e0f562

Adding storybook for Custom Equation Editor

006af29

simianhacker marked this pull request as ready for review January 25, 2023 22:44

simianhacker requested a review from a team as a code owner January 25, 2023 22:44

simianhacker added 3 commits January 30, 2023 07:43

Making more clear that aggType is count

8bd1650

Simplifying createCustomMetricsAggregation

5f104df

Merge branch 'main' of github.com:elastic/kibana into issue-145444-cu…

d11e1d6

…stom-metric-editor-for-mtr

Merge branch 'main' of github.com:elastic/kibana into issue-145444-cu…

8186934

…stom-metric-editor-for-mtr

Fixing validation issue when switching from average with field to cus…

ef640c1

…tom equation

simianhacker requested review from crespocarlos and maryam-saeidi January 30, 2023 18:44

crespocarlos approved these changes Jan 31, 2023

View reviewed changes

maryam-saeidi requested changes Jan 31, 2023

View reviewed changes

simianhacker and others added 6 commits January 31, 2023 08:32

Merge branch 'main' of github.com:elastic/kibana into issue-145444-cu…

165c274

…stom-metric-editor-for-mtr

Removing in preference of letting the error surface

1cc2c9e

Removing the custom equation specific fields from the expression when…

384d641

… changing to other aggreation types

Removing extra story

c1ab527

Remove fallback

3c55dba

Co-authored-by: Maryam Saeidi <maryam.saeidi@elastic.co>

Removing extra type casting

949b08d

simianhacker requested a review from maryam-saeidi January 31, 2023 17:03

maryam-saeidi approved these changes Feb 1, 2023

View reviewed changes

simianhacker merged commit 6e70bdb into elastic:main Feb 1, 2023

kibanamachine added the backport:skip This commit does not require backporting label Feb 1, 2023

simianhacker mentioned this pull request Feb 7, 2023

Allow user to override index pattern for Metric Threshold Rule #149989

Closed

2 tasks

simianhacker mentioned this pull request Jun 28, 2023

Create a an alert from a dynamically created percent using two metric fields #68297

Closed

maryam-saeidi mentioned this pull request Jun 28, 2023

[RAC] Using painless script to build any conditions for threshold alerts #101161

Closed

simianhacker deleted the issue-145444-custom-metric-editor-for-mtr branch April 17, 2024 15:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Custom equation editor for Metric Threshold Rule #148732

Custom equation editor for Metric Threshold Rule #148732

simianhacker commented Jan 11, 2023 •

edited

Loading

StephanErb commented Jan 12, 2023

simianhacker commented Jan 19, 2023 •

edited

Loading

simianhacker commented Jan 30, 2023

simianhacker commented Jan 30, 2023

crespocarlos left a comment

maryam-saeidi left a comment •

edited

Loading

maryam-saeidi Jan 31, 2023

simianhacker commented Jan 31, 2023

kibana-ci commented Jan 31, 2023

async chunk count

maryam-saeidi left a comment •

edited

Loading

simianhacker commented Feb 1, 2023

Custom equation editor for Metric Threshold Rule #148732

Custom equation editor for Metric Threshold Rule #148732

Conversation

simianhacker commented Jan 11, 2023 • edited Loading

Summary

Notable changes with this PR

Rule with custom equation

Rule with custom ratio equation

Reason message with custom label

StephanErb commented Jan 12, 2023

simianhacker commented Jan 19, 2023 • edited Loading

simianhacker commented Jan 30, 2023

simianhacker commented Jan 30, 2023

crespocarlos left a comment

Choose a reason for hiding this comment

maryam-saeidi left a comment • edited Loading

Choose a reason for hiding this comment

maryam-saeidi Jan 31, 2023

Choose a reason for hiding this comment

simianhacker commented Jan 31, 2023

kibana-ci commented Jan 31, 2023

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

async chunk count

History

maryam-saeidi left a comment • edited Loading

Choose a reason for hiding this comment

simianhacker commented Feb 1, 2023

simianhacker commented Jan 11, 2023 •

edited

Loading

simianhacker commented Jan 19, 2023 •

edited

Loading

maryam-saeidi left a comment •

edited

Loading

maryam-saeidi left a comment •

edited

Loading