[OBS-UX-MNGMT] Move the Alerting comparators from TriggersActionsUI plugin to the alerting-types package

[fkanout]

Summary

It fixes #179633

Observability created a Comparator type/enum, when ResponseOps is already exporting one and other rules using it.
The only difference is the wording of not in between [I put the two types side by side to compare]
Currently, we import the one in triggers-actions-ui-plugin , and then update the not in between to match our Comparator.

Comparing the two enums:

For reviewers 🧪

• Everything should work as expected: Alert flyout, Alert reason message, Rule creation flyout, etc.
• I kept the outside comparator (replaced by NOT BETWEEN) for backward compatibility

[apmmachine]

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

• /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
• run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

[elasticmachine]

Pinging @elastic/obs-ux-management-team (Team:obs-ux-management)

[cauemarcondes]

Infra LGTM

[cnasikas]

ResponesOps changes LGTM

[maryam-saeidi]

Thanks for the update, looks much better now! 🎉 (Waiting for the discussion on slack before approval)

Tested the following scenarios locally, creating the rule in the main and checking it on the current PR.

Rule	Comparator	Result	Preview chart	Flyout	Execution
Custom threshold	Not between	Saved successfully	✅	✅	✅
Metric threshold	Not between	Cannot create a rule, shows params invalid: [criteria.0]: types that failed validation: - [criteria.0.0.comparator] error
Metric threshold	Is not between	Saved successfully	✅	✅	✅
Metric threshold	Not between (warning)	Cannot create a rule, shows params invalid: [criteria.0]: types that failed validation: - [criteria.0.0.warningComparator] error
Metric threshold	Is not between	Saved successfully	✅	✅	✅
Inventory	Not between	Cannot create a rule, shows params invalid: [criteria.0.comparator]: must be one of ... between | outside error
Inventory	Not between (warning)	Cannot create a rule, shows params invalid: [criteria.0.warningComparator]: must be one of ... between | outside error

Btw, when I edited the rule and saved it again, the comparator was still outside, is that expected?

[fkanout]

Not used ❌

[maryam-saeidi]

Thanks for updating the rule param types to COMPARATORS | LEGACY_COMPARATORS. :)
There are some type check failures that I assume we can fix without casting by using the helper function, so I will approve the PR as it works as expected.

I just added one comment about checking outside for Inventory rule, as I assume it should have been working at some point from UI, and from API, it should be possible to create rules even when UI is failing. But up to you to decide as you have more context about the impact of this.

[maryam-saeidi]

Can you please use convertToBuiltInComparators at the upper level to avoid this type change?

i.e.

createConditionScript(condition.threshold, convertToBuiltInComparators(condition.comparator))

[maryam-saeidi]

You can avoid this type change by adjusting formatAlertResult and returning build in comparator:

export type FormattedEvaluation = Omit<Evaluation, 'currentValue' | 'threshold' | 'comparator'> & {
  currentValue: string;
  threshold: string[];
  comparator: COMPARATOR;
};

export const formatAlertResult = (evaluationResult: Evaluation): FormattedEvaluation => {
  return {
    ...
    comparator: convertToBuiltInComparators(comparator),
  };
};

[maryam-saeidi]

We don't check the outside here for the inventory rule, IIRC, since it was not working in the UI. From what I see, it should work fine if someone provides outside in the API. So, I think it would be better to handle this case like other rules.

[fkanout]

When the inventory rule was introduced, it didn't have "not in between". It was introduced in v8.10 -I believe by accident due to a types refactoring, and since the 8.10 version, we can't create the rule with "not in between"

From what I see, it should work fine if someone provides outside in the API

The error we see in the UI is coming from the API. The API will use the validator function provided in the rule registry, so we can't create the rule from the API for the same reason.

[maryam-saeidi]

The error we see in the UI is coming from the API. The API will use the validator function provided in the rule registry, so we can't create the rule from the API for the same reason.

UI is providing notBetween, but in the API validation, it accepts outside. When I tried creating a rule directly via API, providing outside, the rule was created successfully and alerts were generated as you see below:

But now the execution is failing:

[fkanout]

Update:

1. No more COMPARATORS | LEGACY_COMPARATORS all over the place. Only used in the main types definitions.
2. Wherever the comparator is used and before sending it to a function that uses it, I convert it with convertToBuiltInComparators, so the function keeps COMPARATORS as the only type.
3. I was converting the comparator at the entry point of the rule executor, this is gone 🧹 , which is cleaner and better. Thanks to the previous step, I don't need it anymore.
4. Also covered creating the inventory rule from the API using outside use case

[kibana-ci]

💛 Build succeeded, but was flaky

• Buildkite Build
• Commit: e9d6541
• Storybooks Preview
• Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-181584-e9d654181656
• Observability Deployment

Failed CI Steps

• FTR Configs #60

Test Failures

• [job] [logs] FTR Configs #60 / Rule execution logic API Detection Engine - Execution logic @ess @serverless @skipInServerlessMKI ES|QL rule type, alert suppression should suppress alerts by custom field, created in ES|QL query, when do not suppress missing fields configured

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
infra	1560	1562	+2
observability	530	533	+3
stackAlerts	151	153	+2
triggersActionsUi	734	735	+1
total			+8

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
@kbn/alerting-comparators	-	5	+5
observability	686	688	+2
triggersActionsUi	567	562	-5
total			+2

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.5MB	1.5MB	-5.0KB
observability	289.6KB	290.8KB	+1.1KB
stackAlerts	86.5KB	86.5KB	-2.0B
total			-3.9KB

Canvas Sharable Runtime

The Canvas "shareable runtime" is an bundle produced to enable running Canvas workpads outside of Kibana. This bundle is included in third-party webpages that embed canvas and therefor should be as slim as possible.

id	before	after	diff
module count	-	5412	+5412
total size	-	8.8MB	+8.8MB

Page load bundle

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

id	before	after	diff
infra	105.7KB	105.6KB	-106.0B
observability	151.4KB	152.1KB	+685.0B
stackAlerts	24.7KB	24.9KB	+223.0B
triggersActionsUi	121.2KB	121.1KB	-159.0B
total			+643.0B

Unknown metric groups

API count

id	before	after	diff
@kbn/alerting-comparators	-	5	+5
observability	695	697	+2
triggersActionsUi	593	588	-5
total			+2

History

• 💔 Build #212510 failed f3a6b9f
• 💔 Build #212453 failed 0ef4825
• 💔 Build #212399 failed 346f75e
• 💔 Build #212382 failed 2379906

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

cc @fkanout