[Metrics UI] Increase metric threshold test coverage

[neptunian]

We need to improve test coverage around the Metric Threshold rule and alerting. Here is our current state and an incomplete list of some scenarios we are missing.

• Unit tests

  • metric_threshold_executor.test.ts Tests the correct actions are being created
  • metric_query.test.ts. Tests that getElasticsearchMetricQuery creates a query we expect in a few scenarios

• Integration tests

  • metric_threshold_alert.ts . Using archive data to test whether an alert should fire. We should add more scenarios and edge cases that are possible to create
  • metrics_alerting.ts. This file currently only contains tests that test the result of the elasticsearch query we use for the alert. I think it'd be useful to add more scenarios here in case an alert is firing for a different reason we might assume it is in metric_threshold_alert.ts and so we understand why its firing or not in more complex scenarios. We could possibly combine these two files to keep the scenarios in one test.

Improvements (feel free to edit):

• Unit tests to add
  • evaluate_alert.ts. There are several functions in here where we should add unit tests for
• Integration tests to add
  • "Alert me if there's no data" is checked in combination with other scenarios such as querying for when document count is <= 0
  • groupBy is used with or without "Alert me if there's no data" and document count is <= 0
  • multiple conditions
  • adding various warning thresholds
  • KQL filter query in combination with other scenarios
  • various time units
  • various comparators
  • various aggregators

[elasticmachine]

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

[Zacqary]

• "Alert me if there's no data" is checked in combination with other scenarios such as querying for when document count is <= 0
• groupBy is used with or without "Alert me if there's no data" and document count is <= 0

#111465 adds at least some of these, testing specifically with a groupBy enabled. We can expand that for sure.

[elasticmachine]

Pinging @elastic/actionable-observability (Team: Actionable Observability)

[roshan-elastic]

Hey @neptunian / @katrin-freihofner - do you think this work makes sense anymore if we move to a single 'threshold' type rule (as I believe is planned)?

Checking this work would still be valuable if we do.

[katrin-freihofner]

Yes, we are working on end-to-end testing.

[roshan-elastic]

Thanks @katrin-freihofner.

@neptunian / @smith - Shall we wait until the new threshold types are done before we tackle this?

[maryam-saeidi]

@roshan-elastic We are working on a new rule to replace metric threshold, inventory and log threshold. So I am not even sure if it makes sense to put the effort into increasing the test coverage.

Also, I added an integration test for metric threshold in this PR, so we already improved the coverage.

My suggestion is to close this ticket in favor of the new rule. We already have a test plan for the new rule.

[roshan-elastic]

Thanks a lot @maryam-saeidi - that's really helpful.

@neptunian - I'll let you decide on whether we close this or not.

[neptunian]

I created this when we owned the rule but now it makes sense for AO to handle it as @maryam-saeidi describes. Thanks!