[APM] Cleanup alerting api tests #164438

sorenlouv · 2023-08-22T12:12:13Z

This PR cleans up and refactors the APM API tests for rules and alerting.

introduces some new helper methods like deleteRuleById
test all template variables using the index connector
improve flaky tests and ensure that tests can be run in isolation and in any order

elasticmachine · 2023-08-22T12:12:21Z

Pinging @elastic/apm-ui (Team:APM)

apmmachine · 2023-08-22T12:12:33Z

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

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

sorenlouv · 2023-08-22T12:16:35Z

x-pack/test/apm_api_integration/tests/alerts/transaction_error_rate.spec.ts

-      await supertest.delete(`/api/alerting/rule/${ruleId1}`).set('kbn-xsrf', 'foo');
-      await supertest.delete(`/api/actions/connector/${actionId1}`).set('kbn-xsrf', 'foo');
-      await supertest.delete(`/api/alerting/rule/${ruleId2}`).set('kbn-xsrf', 'foo');
-      await supertest.delete(`/api/actions/connector/${actionId2}`).set('kbn-xsrf', 'foo');


This is problematic: rules and alerts are created per test but not deleted until all tests have run. This means that the rules and alerts created in the first test still exists when running the second test. This means that tests are required to run in a specific order, and it is not possible to run tests in isolation.

This was changed so rules and alerts are created per test, and also removed after each individual test. This ensures that subsequent tests are not polluted with state created in previous tests.

sorenlouv · 2023-08-22T12:34:08Z

x-pack/test/apm_api_integration/tests/alerts/transaction_error_rate.spec.ts

-        actionId1 = await createIndexConnector({
-          supertest,
-          name: 'Transation error rate without filter query',
-          indexName: ALERT_ACTION_INDEX_NAME1,
-        });


I've removed all reference to the index connector action. We don't need it to test whether an alert was produced since we can just query the alerts index directly

There are two things that we are testing:

whether an alert is triggered (and shown in Kibana UI).

alert notification was sent (with index connector in this case) with correct action variables.

I think the tests to check alert notifications are useful.

alert notification was sent (with index connector in this case) with correct action variables.

Are we testing the APM rule in this case, or the connector functionality (and thus the alerting framework). By exercising the indexing connector it sounds like we are testing the latter where we should focus on the former.

.. or put another way: what is currently not covered anymore? What could break in the APM rule that would no longer go noticed?

We are setting context action variables during rule execution https://github.com/elastic/kibana/blob/f7e36a96d035e1cb4feec334a3bab270be9eac4e/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts#L288C69-L288C69.

In the tests, we are checking if the action variables are set correctly in alert notification. Without it, any changes around context action variables would not be checked anymore.

sorenlouv · 2023-08-22T12:35:13Z

x-pack/test/apm_api_integration/tests/alerts/transaction_error_rate.spec.ts

-        alertId = (resp.hits.hits[0]._source as any)['kibana.alert.uuid'];
-        startedAt = (resp.hits.hits[0]._source as any)['kibana.alert.start'];


This was also problematic: a variable was set in one test and consumed in another. This means that tests depend on each other and cannot be run in isolation.

sorenlouv · 2023-08-22T12:41:26Z

x-pack/test/apm_api_integration/tests/alerts/transaction_error_rate.spec.ts

        const serviceInventoryAlertCounts = await fetchServiceInventoryAlertCounts(apmApiClient);
        expect(serviceInventoryAlertCounts).to.eql({
          'opbeans-node': 1,
-          'opbeans-java': 1,
+          'opbeans-java': 0,


This was an incorrect assertion caused by the alert pollution. Now alerts are cleaned up between runs.

benakansara

LGTM. Thank you for addressing the issues and cleaning up the tests.
As discussed offline, tests around context variables will be added in a separate PR.

x-pack/test/apm_api_integration/tests/alerts/helpers/alerting_api_helper.ts

kpatticha

LGTM, thanks for pushing this work forward. I left only 2 comments.

Also, have you run the flaky test runner?

sorenlouv · 2023-08-23T22:11:47Z

Flaky test runner builds:

kibana-ci · 2023-08-24T00:09:38Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 6aeedfc

Failed CI Steps

Serverless Security Cypress Tests #5

Metrics [docs]

✅ unchanged

History

💔 Build #152969 failed 953aa0c39fc32baa903995304c7a4722bd771f39
💔 Build #152892 failed e24caf570e61e8ea26b9e245a33d5339c4f993c7
💔 Build #152776 failed 87dddd3bb2b28373814f0739ceb832cc0aa5db3e
💚 Build #152381 succeeded 981116f87338a6885dc8700ae0d7e34692f68418

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

* main: (3152 commits) [Security Solution][Detection Engine] fixes 410 error on index legacy template call (#164682) [SavedObjects] Create serverless roots for jest integration tests (#164157) Create upselling package and implement EntityAnalytics serverless upselling (#164136) [Fleet] Change 'Out-of-date' to 'Outdated policy' in agent list table (#164673) [IndexManagement] Use internal base path for API (#164665) [Profiling] removing ~ symbol (#164595) [Telemetry] Fetch snapshot: allow specifying the version via querystring (#164670) [Cases] Show warning when all cases table reaches 10k cases message (#164323) [ML] Removing token list from text expansion model testing (#164560) [Fleet] Add secrets package API integration test (#164583) [Fleet] Fix security solution tag id (#164582) [Security Solution] Modal says "duplicating 0 rules" when you duplicate an individual rule (#163908) [api-docs] 2023-08-24 Daily api_docs build (#164658) [APM] Cleanup alerting api tests (#164438) Upgrade EUI to 87.2.0 (#164385) [ML] Fix query bar autocompletion for ML and AIOps embeddables (#164485) [Fleet] Fix flaky unit test for the details page (#164641) [Security Solution] update codeowner for serverless security subdir (#164640) [Security Solution] Fixes Assistant Connector and Actions RBAC Flow (#164382) [Discover] Removing large string truncation from doc viewer (#164236) ...

sorenlouv · 2023-08-26T22:39:34Z

💚 All backports created successfully

Status	Branch	Result
✅	8.10

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

This PR cleans up and refactors the APM API tests for rules and alerting. - introduces some new helper methods like `deleteRuleById` - removes dependency on index actions to test alerts (we can just use the alert index) - improve flaky tests and ensure that tests can be run in isolation and in any order (cherry picked from commit d440288)

sorenlouv · 2023-08-26T22:47:28Z

💚 All backports created successfully

Status	Branch	Result
✅	8.10

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

sorenlouv · 2023-08-26T22:50:46Z

💚 All backports created successfully

Status	Branch	Result
✅	8.10

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

sorenlouv · 2023-08-26T22:51:09Z

💚 All backports created successfully

Status	Branch	Result
✅	8.10

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

sorenlouv · 2023-08-26T22:52:05Z

💚 All backports created successfully

Status	Branch	Result
✅	8.10

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

This PR cleans up and refactors the APM API tests for rules and alerting. - introduces some new helper methods like `deleteRuleById` - removes dependency on index actions to test alerts (we can just use the alert index) - improve flaky tests and ensure that tests can be run in isolation and in any order (cherry picked from commit d440288)

sorenlouv · 2023-08-26T22:53:09Z

💚 All backports created successfully

Status	Branch	Result
✅	8.10

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

sorenlouv · 2023-08-26T23:07:22Z

💚 All backports created successfully

Status	Branch	Result
✅	8.10

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

This PR cleans up and refactors the APM API tests for rules and alerting. - introduces some new helper methods like `deleteRuleById` - removes dependency on index actions to test alerts (we can just use the alert index) - improve flaky tests and ensure that tests can be run in isolation and in any order (cherry picked from commit d440288)

kibanamachine · 2023-08-26T23:50:48Z

💚 All backports created successfully

Status	Branch	Result
✅	8.10

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

# Backport This will backport the following commits from `main` to `8.10`: - [[APM] Cleanup alerting api tests (#164438)](#164438)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Søren Louv-Jansen <[email protected]>

sorenlouv requested a review from a team as a code owner August 22, 2023 12:12

botelastic bot added the Team:APM All issues that need APM UI Team support label Aug 22, 2023

sorenlouv commented Aug 22, 2023

View reviewed changes

sorenlouv force-pushed the apm-alerting-improvements branch from daa0e75 to 3ca87f3 Compare August 22, 2023 13:12

sorenlouv added v8.11.0 release_note:skip Skip the PR/issue when compiling release notes labels Aug 22, 2023

benakansara approved these changes Aug 23, 2023

View reviewed changes

kpatticha reviewed Aug 23, 2023

View reviewed changes

x-pack/test/apm_api_integration/tests/alerts/helpers/alerting_api_helper.ts Outdated Show resolved Hide resolved

kpatticha reviewed Aug 23, 2023

View reviewed changes

x-pack/test/apm_api_integration/tests/alerts/helpers/alerting_api_helper.ts Outdated Show resolved Hide resolved

kpatticha approved these changes Aug 23, 2023

View reviewed changes

sorenlouv added 9 commits August 24, 2023 00:57

[APM] Cleanup alerting api tests

f3bfa45

Improve spec titles

c07f46f

Ensure that alerts have been produced before running expectations

ec64b51

Use ruleId instead of id

e783c36

Rename esClient to es

15fd88b

Add index connector tests

a5a47f8

Remove initial clean

1187202

Catch errors when deleting

0adfcc6

Address PR comments

0927bcd

sorenlouv force-pushed the apm-alerting-improvements branch from 953aa0c to 0927bcd Compare August 23, 2023 22:57

Make tests more resilient

6aeedfc

sorenlouv merged commit d440288 into elastic:main Aug 24, 2023

kibanamachine added the backport:skip This commit does not require backporting label Aug 24, 2023

sorenlouv deleted the apm-alerting-improvements branch August 24, 2023 00:22

sorenlouv added auto-backport Deprecated - use backport:version if exact versions are needed v8.10.0 labels Aug 26, 2023

kibanamachine mentioned this pull request Aug 26, 2023

[8.10] [APM] Cleanup alerting api tests (#164438) #164928

Merged

This was referenced Oct 1, 2023

[8.10] fix(slo): Handle partial indicator url state (#167247) #167722

Merged

[8.10] backport SLO fixes #168231

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[APM] Cleanup alerting api tests #164438

[APM] Cleanup alerting api tests #164438

sorenlouv commented Aug 22, 2023 •

edited

Loading

elasticmachine commented Aug 22, 2023

apmmachine commented Aug 22, 2023

sorenlouv Aug 22, 2023 •

edited

Loading

sorenlouv Aug 22, 2023

benakansara Aug 22, 2023

sorenlouv Aug 22, 2023 •

edited

Loading

sorenlouv Aug 22, 2023

benakansara Aug 23, 2023

sorenlouv Aug 22, 2023

sorenlouv Aug 22, 2023

benakansara left a comment

kpatticha left a comment

sorenlouv commented Aug 23, 2023 •

edited

Loading

kibana-ci commented Aug 24, 2023

sorenlouv commented Aug 26, 2023

sorenlouv commented Aug 26, 2023

sorenlouv commented Aug 26, 2023

sorenlouv commented Aug 26, 2023

sorenlouv commented Aug 26, 2023

sorenlouv commented Aug 26, 2023

sorenlouv commented Aug 26, 2023

kibanamachine commented Aug 26, 2023

		alertId = (resp.hits.hits[0]._source as any)['kibana.alert.uuid'];
		startedAt = (resp.hits.hits[0]._source as any)['kibana.alert.start'];

[APM] Cleanup alerting api tests #164438

[APM] Cleanup alerting api tests #164438

Conversation

sorenlouv commented Aug 22, 2023 • edited Loading

elasticmachine commented Aug 22, 2023

apmmachine commented Aug 22, 2023

🤖 GitHub comments

sorenlouv Aug 22, 2023 • edited Loading

Choose a reason for hiding this comment

sorenlouv Aug 22, 2023

Choose a reason for hiding this comment

benakansara Aug 22, 2023

Choose a reason for hiding this comment

sorenlouv Aug 22, 2023 • edited Loading

Choose a reason for hiding this comment

sorenlouv Aug 22, 2023

Choose a reason for hiding this comment

benakansara Aug 23, 2023

Choose a reason for hiding this comment

sorenlouv Aug 22, 2023

Choose a reason for hiding this comment

sorenlouv Aug 22, 2023

Choose a reason for hiding this comment

benakansara left a comment

Choose a reason for hiding this comment

kpatticha left a comment

Choose a reason for hiding this comment

sorenlouv commented Aug 23, 2023 • edited Loading

kibana-ci commented Aug 24, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

History

sorenlouv commented Aug 26, 2023

💚 All backports created successfully

Questions ?

sorenlouv commented Aug 26, 2023

💚 All backports created successfully

Questions ?

sorenlouv commented Aug 26, 2023

💚 All backports created successfully

Questions ?

sorenlouv commented Aug 26, 2023

💚 All backports created successfully

Questions ?

sorenlouv commented Aug 26, 2023

💚 All backports created successfully

Questions ?

sorenlouv commented Aug 26, 2023

💚 All backports created successfully

Questions ?

sorenlouv commented Aug 26, 2023

💚 All backports created successfully

Questions ?

kibanamachine commented Aug 26, 2023

💚 All backports created successfully

Questions ?

sorenlouv commented Aug 22, 2023 •

edited

Loading

sorenlouv Aug 22, 2023 •

edited

Loading

sorenlouv Aug 22, 2023 •

edited

Loading

sorenlouv commented Aug 23, 2023 •

edited

Loading