[AO][SERVERLESS] Fix Custom Threshold rule tests for Serverless by fkanout · Pull Request #166942 · elastic/kibana

fkanout · 2023-09-21T13:50:32Z

Summary

Fixes #165569
Fixes #166617
Fixes #166618
Fixes #166619
Fixes #166620

ghost · 2023-09-21T13:50:52Z

🤖 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!)

elasticmachine · 2023-09-21T13:51:47Z

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

kibana-ci · 2023-09-22T13:05:35Z

💚 Build Succeeded

Buildkite Build
Commit: 555cc28
Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-166942-555cc28c2791

Metrics [docs]

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`	-	5615	+5615
`total size`	-	6.0MB	+6.0MB

History

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

cc @fkanout

fkanout · 2023-09-22T13:20:20Z

🧪 Flaky test runner for both Serverless and Statefull ENVs (200 runs for each)
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/3212

fkanout · 2023-09-22T15:42:53Z

~~Waiting for @XavierM inputs as on serverless, we could create a Custom threshold rule with apm consumer, where we should not~~

fkanout · 2023-09-25T08:40:35Z

flakiness-free

maryam-saeidi

Nice job! 👏🏻

Overall LGTM, just added some clarification questions.

maryam-saeidi · 2023-09-27T14:35:33Z

x-pack/test/alerting_api_integration/observability/custom_threshold_rule/avg_pct_fired.ts

  describe('Custom Threshold rule - AVG - PCT - FIRED', () => {
    const CUSTOM_THRESHOLD_RULE_ALERT_INDEX = '.alerts-observability.threshold.alerts-default';
    const ALERT_ACTION_INDEX = 'alert-action-threshold';
+    // DATE_VIEW should match the index template:


👍🏻

This is about index_patterns field, right?

maryam-saeidi · 2023-09-27T14:37:47Z

..._serverless/api_integration/test_suites/observability/custom_threshold_rule/avg_pct_fired.ts

    const CUSTOM_THRESHOLD_RULE_ALERT_INDEX = '.alerts-observability.threshold.alerts-default';
+    // DATE_VIEW should match the index template:
+    // x-pack/packages/kbn-infra-forge/src/data_sources/composable/template.json
+    const DATE_VIEW = 'kbn-data-forge-fake_hosts';


Can we expose this const from the package and use it both inside the package and in the integration tests instead of defining them multiple times?

@maryam-saeidi, this value should not be part of the package as it depends on the test we are performing. e.g., for this rule and this test, we are using the template of fake_host.
But later, we could use a different type of data, we need to define another template and another DATA_VIEW.

Do you mean we will add other metrics besides fake_host in the future so the DATA_VIEW will be different? Can we have multiple DATA_VIEWs exposed when we add more scenarios?

Would you have any suggestions about improving the current case and not relying on the comment? It can be tricky to debug when the data view in the package is changed, as in the package, we don't have a connection between the logic and what is used in tests.

Do you mean we will add other metrics besides fake_host in the future so the DATA_VIEW will be different? Can we have multiple DATA_VIEWs exposed when we add more scenarios?

We could.

There is always room for improvement, but for the current tests and data, it's totally okay.

maryam-saeidi · 2023-09-27T14:43:11Z

...serverless/api_integration/test_suites/observability/custom_threshold_rule/group_by_fired.ts

        expect(resp.hits.hits[0]._source?.ruleType).eql('observability.rules.custom_threshold');
        expect(resp.hits.hits[0]._source?.alertDetailsUrl).eql(
-          `${protocol}://${hostname}:${port}/app/observability/alerts?_a=(kuery:%27kibana.alert.uuid:%20%22${alertId}%22%27%2CrangeFrom:%27${rangeFrom}%27%2CrangeTo:now%2Cstatus:all)`
+          // Added the S to protocol.getUrlParts as not returning the correct value.


Does it mean our code is not working correctly, or is the test environment setup incorrect?

If I recall correctly, the protocol is part of the base path config. Do you think we can update the config then, or will it create an issue?
I want to understand why we have this mismatch in the first place.

@maryam-saeidi it's a package that we don't own. I checked the code and discussed it with the team on Slack. It returns HTTP by default when KIBANA_HOST (if not mistaken) is not provided, which is the case for serverless.

I see, where can I learn more about the discussion you mentioned?

As I mentioned, it was on Slack, I need to search the thread.

maryam-saeidi

I added a comment here, it would be nice if there is an idea about how to avoid issue in case someone changes data view in the package.

Also, if you found the thread, please share.

…ss (#166942)" This reverts commit 4c1ca7e.

jbudz · 2023-09-29T12:19:24Z

Hey all, this PR introduced a few type errors and was reverted with 706f6fa

https://buildkite.com/elastic/kibana-on-merge/builds/36016

) ## Summary Revert the revert for #166942 💪🏻 Fixes #165569 Fixes #166617 Fixes #166618 Fixes #166619 Fixes #166620

Fix serverless tests

3316118

fkanout requested a review from a team September 21, 2023 13:50

fkanout added release_note:skip Skip the PR/issue when compiling release notes Team: Actionable Observability - DEPRECATED For Observability Alerting and SLOs use "Team:obs-ux-management", for AIops "Team:obs-knowledge" v8.11.0 labels Sep 21, 2023

fkanout self-assigned this Sep 21, 2023

Update file name

c42f17c

fkanout added the ci:build-serverless-image label Sep 21, 2023

fkanout and others added 6 commits September 21, 2023 17:45

Fix metric threshold rule test

516ae30

Fix metric threshold rule test

dc4ec0a

Merge branch 'main' into fix-custom-threshold-rule-serverless-tests

5681a69

Update serverless tests after ResponseOps PR about consumer

6b6bbcd

Merge branch 'main' into fix-custom-threshold-rule-serverless-tests

94a0a8a

Merge branch 'main' into fix-custom-threshold-rule-serverless-tests

555cc28

maryam-saeidi self-requested a review September 27, 2023 14:20

fkanout mentioned this pull request Sep 27, 2023

[AO] Save group information in AAD for the new threshold rule #164087

Merged

maryam-saeidi reviewed Sep 27, 2023

View reviewed changes

maryam-saeidi approved these changes Sep 28, 2023

View reviewed changes

fkanout merged commit 4c1ca7e into elastic:main Sep 29, 2023

kibanamachine added the backport:skip This PR does not require backporting label Sep 29, 2023

jbudz added a commit that referenced this pull request Sep 29, 2023

Revert "[AO][SERVERLESS] Fix Custom Threshold rule tests for Serverle…

706f6fa

…ss (#166942)" This reverts commit 4c1ca7e.

jbudz added the reverted label Sep 29, 2023

fkanout mentioned this pull request Sep 29, 2023

[AO][SERVERLESS] Fix Custom Threshold rule tests for Serverless #167644

Merged

fkanout added a commit that referenced this pull request Oct 2, 2023

[AO][SERVERLESS] Fix Custom Threshold rule tests for Serverless (#167644

b41dd04

) ## Summary Revert the revert for #166942 💪🏻 Fixes #165569 Fixes #166617 Fixes #166618 Fixes #166619 Fixes #166620

elasticmachine unassigned fkanout Oct 29, 2024

Conversation

fkanout commented Sep 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

ghost commented Sep 21, 2023

🤖 GitHub comments

Uh oh!

elasticmachine commented Sep 21, 2023

Uh oh!

kibana-ci commented Sep 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💚 Build Succeeded

Metrics [docs]

Canvas Sharable Runtime

History

Uh oh!

fkanout commented Sep 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fkanout commented Sep 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fkanout commented Sep 25, 2023

Uh oh!

maryam-saeidi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maryam-saeidi Sep 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maryam-saeidi left a comment

Choose a reason for hiding this comment

Uh oh!

jbudz commented Sep 29, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

fkanout commented Sep 21, 2023 •

edited

Loading

kibana-ci commented Sep 22, 2023 •

edited

Loading

fkanout commented Sep 22, 2023 •

edited

Loading

fkanout commented Sep 22, 2023 •

edited

Loading

maryam-saeidi Sep 28, 2023 •

edited

Loading