[ObsUX] Fix timestamp for anomaly alert test #169255

MiriamAparicio · 2023-10-18T14:25:47Z

What was done

The anomaly alert test was failing with timeout because was not generating alerts, the data and spikes were added to far away in time, so when the job is created doesn't take into account data that far in the past
We fixed the timerange of the data generated and the spikes.

BEFORE:

AFTER:

Flaky test runner:

https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/3662

elasticmachine · 2023-10-18T14:25:55Z

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

apmmachine · 2023-10-18T14:26:07Z

🤖 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-10-18T14:59:31Z

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

+      const start = Date.now() - 1000 * 60 * 60 * 24 * 2; // day ago
+      const end = Date.now();


nit: moment with subtract() and add() is easier to read.

kibana/x-pack/test/apm_api_integration/tests/mobile/mobile_detailed_statistics_by_field.spec.ts

Line 47 in be85fef

start: moment(end).subtract(7, 'minutes').toISOString(),

Changed here bf3d743

sorenlouv · 2023-10-18T15:00:57Z

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

-      const spikeStart = new Date('2021-01-07T23:15:00.000Z').getTime();
-      const spikeEnd = new Date('2021-01-08T00:15:00.000Z').getTime();
+      const spikeStart = new Date(Date.now() - 1000 * 60 * 15).getTime(); // 15 minutes ago
+      const spikeEnd = new Date(Date.now()).getTime();


Suggested change

const spikeEnd = new Date(Date.now()).getTime();

const spikeEnd = Date.now();

Updated here bf3d743

sorenlouv · 2023-10-18T19:34:53Z

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

+      const start = moment().subtract(1, 'days').toISOString();
+      const end = moment().toISOString();


Shouldn't this also be timestamps? Then you don't have to convert them later

sorenlouv · 2023-10-18T19:35:39Z

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

      const NORMAL_DURATION = 100;
      const NORMAL_RATE = 1;


Inline these. Having them extracted makes the code harder to read.

sorenlouv

just some nits

…t-skipped-test

kpatticha · 2023-10-19T08:55:51Z

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


      let ruleId: string;

      before(async () => {
+        await cleanup();


Is this needed since you're running the cleanup on line 72 ?https://github.com/elastic/kibana/pull/169255/files#diff-12f5dc28099b892098fa1f396eeb9618adbef84c776e77559c5634dbe0f49da2R72?

We want to clean before and after the test

During development it's much easier if cleanup happens before running the test. On CI it is better to have cleanup after to avoid contamination between tests. I don't have strong opinions whether we need to cleanup both before and after though. Perhaps you can comment out the cleanup in the before step. It's still easy to enable for debug purposes.

MiriamAparicio · 2023-10-20T08:59:11Z

x-pack/plugins/observability/public/pages/slo_edit/slo_edit.test.tsx

-        getAddRuleFlyout: jest
-          .fn()
-          .mockReturnValue(<div data-test-subj="add-rule-flyout">Add Rule Flyout</div>),
+        getAddRuleFlyout: jest.fn().mockReturnValue(


@elastic/actionable-observability
I'm not sure how this end up in my PR, and is actually creating a conflict

Fixed merging with main

…t-skipped-test

sorenlouv · 2023-10-20T09:33:24Z

x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/register_anomaly_rule_type.ts

@@ -174,7 +174,6 @@ export function registerAnomalyRuleType({
                    range: {
                      timestamp: {
                        gte: dateStart,
-                        format: 'epoch_millis',


Are you sure about this change? Don't we pass an epoch timestamp anymore?

I checked the other endpoints, and we don't pass it anywhere else, also dateStart is typed as string, I had a conversation about it on slack

kibana-ci · 2023-10-20T10:39:35Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 9bb3eff

Failed CI Steps

Metrics [docs]

✅ unchanged

History

💔 Build #169580 failed 6137417644c8c91539e7bc9e348958f0c86ca217
💔 Build #169537 failed e4ace46c0ede5be1855252c1567c83bf5f4490ab
💔 Build #169462 failed b80e59652cc8188dd557c528bb6efcf276fedc64
💔 Build #169309 failed 99f1599
💔 Build #169118 failed 773d7f52e4ba49fa35e009b813df8d07c52931fd

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

fix time range for anomaly alert test

61a2f04

MiriamAparicio requested a review from a team as a code owner October 18, 2023 14:25

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

MiriamAparicio added the release_note:skip Skip the PR/issue when compiling release notes label Oct 18, 2023

sorenlouv reviewed Oct 18, 2023

View reviewed changes

MiriamAparicio force-pushed the 160769-anomaly-alert-skipped-test branch from 29c268f to 9c1b116 Compare October 18, 2023 15:46

use moment

bf3d743

MiriamAparicio force-pushed the 160769-anomaly-alert-skipped-test branch from 9c1b116 to bf3d743 Compare October 18, 2023 15:48

sorenlouv reviewed Oct 18, 2023

View reviewed changes

sorenlouv approved these changes Oct 18, 2023

View reviewed changes

MiriamAparicio added 2 commits October 19, 2023 07:59

pr review comments

f0610be

Merge remote-tracking branch 'upstream/main' into 160766-anomaly-aler…

99f1599

…t-skipped-test

MiriamAparicio force-pushed the 160769-anomaly-alert-skipped-test branch from 773d7f5 to 99f1599 Compare October 19, 2023 07:04

kpatticha reviewed Oct 19, 2023

View reviewed changes

remove format: 'epoch_millis' from registerAnomalyRuleType

05805c9

MiriamAparicio force-pushed the 160769-anomaly-alert-skipped-test branch from 94a8adc to 05805c9 Compare October 19, 2023 15:25

kibanamachine requested a review from a team as a code owner October 19, 2023 15:32

MiriamAparicio commented Oct 20, 2023

View reviewed changes

Merge remote-tracking branch 'upstream/main' into 160769-anomaly-aler…

9bb3eff

…t-skipped-test

MiriamAparicio force-pushed the 160769-anomaly-alert-skipped-test branch from 6137417 to 9bb3eff Compare October 20, 2023 09:02

sorenlouv reviewed Oct 20, 2023

View reviewed changes

sorenlouv approved these changes Oct 23, 2023

View reviewed changes

sorenlouv added the v8.12.0 label Oct 23, 2023

MiriamAparicio merged commit 17c78db into elastic:main Oct 23, 2023
2 checks passed

kibanamachine added the backport:skip This commit does not require backporting label Oct 23, 2023

MiriamAparicio deleted the 160769-anomaly-alert-skipped-test branch October 23, 2023 07:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ObsUX] Fix timestamp for anomaly alert test #169255

[ObsUX] Fix timestamp for anomaly alert test #169255

MiriamAparicio commented Oct 18, 2023 •

edited by sorenlouv

Loading

elasticmachine commented Oct 18, 2023

apmmachine commented Oct 18, 2023

sorenlouv Oct 18, 2023 •

edited

Loading

MiriamAparicio Oct 18, 2023

sorenlouv Oct 18, 2023

MiriamAparicio Oct 18, 2023

sorenlouv Oct 18, 2023

MiriamAparicio Oct 19, 2023

sorenlouv Oct 18, 2023

sorenlouv left a comment

kpatticha Oct 19, 2023 •

edited

Loading

MiriamAparicio Oct 19, 2023

sorenlouv Oct 20, 2023

MiriamAparicio Oct 20, 2023

MiriamAparicio Oct 20, 2023

sorenlouv Oct 20, 2023

MiriamAparicio Oct 20, 2023

kibana-ci commented Oct 20, 2023

		const start = Date.now() - 1000 * 60 * 60 * 24 * 2; // day ago
		const end = Date.now();

	const spikeEnd = new Date(Date.now()).getTime();
	const spikeEnd = Date.now();

		const start = moment().subtract(1, 'days').toISOString();
		const end = moment().toISOString();

[ObsUX] Fix timestamp for anomaly alert test #169255

[ObsUX] Fix timestamp for anomaly alert test #169255

Conversation

MiriamAparicio commented Oct 18, 2023 • edited by sorenlouv Loading

What was done

elasticmachine commented Oct 18, 2023

apmmachine commented Oct 18, 2023

🤖 GitHub comments

sorenlouv Oct 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sorenlouv left a comment

Choose a reason for hiding this comment

kpatticha Oct 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibana-ci commented Oct 20, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

History

MiriamAparicio commented Oct 18, 2023 •

edited by sorenlouv

Loading

sorenlouv Oct 18, 2023 •

edited

Loading

kpatticha Oct 19, 2023 •

edited

Loading