Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate Custom threshold > AVG - PCT - FIRED test to the deployment agnostic framework #195902

Conversation

maryam-saeidi
Copy link
Member

@maryam-saeidi maryam-saeidi commented Oct 11, 2024

Part of #183378

Summary

This PR moves the first Custom threshold rule test to the deployment agnostic test. The rest will follow in a follow-up PR.

How to run

To run serverless

node scripts/functional_tests_server --config x-pack/test/api_integration/deployment_agnostic/configs/serverless/oblt.serverless.config.ts
node scripts/functional_test_runner --config x-pack/test/api_integration/deployment_agnostic/configs/serverless/oblt.serverless.config.ts --grep="Custom Threshold rule"

To run stateful

node scripts/functional_tests_server --config x-pack/test/api_integration/deployment_agnostic/configs/stateful/oblt.stateful.config.ts
node scripts/functional_test_runner --config x-pack/test/api_integration/deployment_agnostic/configs/stateful/oblt.stateful.config.ts --grep="Custom Threshold rule"

TODO

How to run tests on MKI

According to this discussion, we should test in MKI environment before merging. For details on how to run in MKI, see this section of the document and this readme.

@maryam-saeidi maryam-saeidi added release_note:skip Skip the PR/issue when compiling release notes backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Oct 11, 2024
@maryam-saeidi maryam-saeidi self-assigned this Oct 11, 2024
@maryam-saeidi maryam-saeidi marked this pull request as ready for review October 11, 2024 12:08
@maryam-saeidi maryam-saeidi requested review from a team as code owners October 11, 2024 12:08
@pheyos
Copy link
Member

pheyos commented Oct 11, 2024

We should consider this PR to be blocked by #195890, because right now, the serverless part of deployment agnostic tests is not enabled, so we wouldn't see any problems with the newly added tests.

@maryam-saeidi maryam-saeidi added the Team:obs-ux-management Observability Management User Experience Team label Oct 11, 2024
@elasticmachine
Copy link
Contributor

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

@pheyos
Copy link
Member

pheyos commented Oct 11, 2024

@maryam-saeidi if you add the label ci:build-serverless-image to this PR, we can use the MKI pipeline to kick off a build using your PR code.

@maryam-saeidi
Copy link
Member Author

@pheyos Thanks for the heads-up; I added the PR that you shared in the TODO section of the PR description.

Regarding ci:build-serverless-image, I should wait for your PR to be merged, and after that, add the label for testing, right?

@pheyos
Copy link
Member

pheyos commented Oct 11, 2024

@maryam-saeidi

I should wait for your PR to be merged, and after that, add the label for testing, right?

You can add the label now, it will be picked up by the next CI run. And yes, you should wait for the other PR to be merged and then merge main into your PR (which will trigger another CI run and produce the serverless container if the label has been added before).

@maryam-saeidi
Copy link
Member Author

Funnily enough, before creating this PR, I checked @mgiota's PR and I saw both configs were executed in CI, but it seems it was before skipping this config.

Serverless config Stateful config
image image

@mgiota
Copy link
Contributor

mgiota commented Oct 11, 2024

@pheyos I added the ci:build-serverless-image label to my PR as well. Is that right?

.post(`/api/content_management/rpc/create`)
.post(`/api/content_management/rpc/delete`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thank you

@dmlemeshko dmlemeshko self-requested a review October 11, 2024 12:39
Copy link
Member

@dmlemeshko dmlemeshko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM, thanks for migrating the tests!

@pheyos
Copy link
Member

pheyos commented Oct 11, 2024

@mgiota

I added the ci:build-serverless-image label to my #195715 as well. Is that right?

Yes, that's right. However, your PR should also wait for #195890 to be merged and then merge main into your PR.

@maryam-saeidi
Copy link
Member Author

@pheyos How can I ensure your PR changes are included in the main merge? I might have been a bit too eager when I saw your PR was merged :D

@pheyos
Copy link
Member

pheyos commented Oct 11, 2024

@maryam-saeidi yes, it was a bit too early. When checking the details of your merge, you can see that you merged commit level 5131215 from main, which is earlier than my commit.

@maryam-saeidi
Copy link
Member Author

yes, it was a bit too early. When checking the details of your merge, you can see that you merged commit level 5131215 from main, which is earlier than my commit.

@pheyos Where did you check the exact commit? Should I see it somewhere in this view:

image

@pheyos
Copy link
Member

pheyos commented Oct 11, 2024

@maryam-saeidi yes, when you click view the diff, then you'll see the two parents in the top right corner (see screenshot) - one of them being the commit level that your PR had before, the other one being the commit level from main that was merged:
image

@pheyos
Copy link
Member

pheyos commented Oct 14, 2024

The MKI job failed:

Serverless Observability - Deployment-agnostic api integration tests Slo - Burn rate rule Custom Threshold rule AVG - PCT - FIRED Rule creation should set correct action variables

Error: expected 'https://bk-serverless-ftr-3067-e697d43e3ad9-e0ac80.kb.eu-west-1.aws.qa.elastic.cloud/app/observability/alerts/1e0c2d3e-e5c2-4bfe-9df0-d46681253b9f'
to sort of equal 'https://bk-serverless-ftr-3067-e697d43e3ad9-e0ac80.kb.eu-west-1.aws.qa.elastic.cloud:443/app/observability/alerts/1e0c2d3e-e5c2-4bfe-9df0-d46681253b9f'

Note that the actual value doesnt have the :443 port.

@maryam-saeidi
Copy link
Member Author

@pheyos Thanks for the update; how can I find the related MKI job in the future?

To recap, adding ci:build-serverless-image to a PR will run the MKI tests for that PR, right? I thought we needed to take more steps, and this label would only give us an image to work with.

@pheyos
Copy link
Member

pheyos commented Oct 15, 2024

@maryam-saeidi there's no automation attached and the ci:build-serverless-image only builds the image. I've kicked off the testing as a one-off, providing overrides based on the created image. This is not the standard workflow and due to capcity limitations, we can't enable this broadly at this point. We have a discussion going on about what we can do there in the future to automate this more. Until then, the standard process would be to run these tests from your local against an MKI project.

@maryam-saeidi
Copy link
Member Author

Regarding this MKI failure, I've run the test on MKI locally, and it worked as expected:

image

We are getting the port from this function, and I am not sure why kibana configuration has "443" as port in this MKI job.

@pheyos how can I access debug logs for this MKI job, locally I got a debug log for requesting URL.

@pheyos
Copy link
Member

pheyos commented Oct 23, 2024

@maryam-saeidi the problem with that sort of assertion is that the browser is hiding the default port. At the same time, it's not an error if the port is part of the Kibana configuration. In order to make that sort of assertions more robust, I'm introducing a new method to get the URL parts but with stripped port if it's the default one. Once #197418 is merged you should be able to use that new method in your test and get rid of the "port flakiness".

Copy link
Contributor

@mgiota mgiota left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maryam-saeidi Great work migrating the custom threshold rules to the deployment agnostic solution!

@maryam-saeidi
Copy link
Member Author

maryam-saeidi commented Oct 24, 2024

Thanks @pheyos for adding getUrlPartsWithStrippedDefaultPort. I used this function in ba30240 and testing on MKI locally works now 🎉

@elasticmachine
Copy link
Contributor

elasticmachine commented Oct 24, 2024

💚 Build Succeeded

  • Buildkite Build
  • Commit: 372509e
  • Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-195902-372509ed6cfc

Metrics [docs]

Unknown metric groups

ESLint disabled line counts

id before after diff
@kbn/test-suites-xpack 732 731 -1

Total ESLint disabled count

id before after diff
@kbn/test-suites-xpack 757 756 -1

History

cc @maryam-saeidi

@maryam-saeidi maryam-saeidi merged commit 30f81ce into elastic:main Oct 24, 2024
24 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11497372332

@maryam-saeidi maryam-saeidi deleted the 183378-migrate-ct-to-deployment-agnostic branch October 24, 2024 10:23
@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.x Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 195902

Questions ?

Please refer to the Backport tool documentation

@maryam-saeidi
Copy link
Member Author

💚 All backports created successfully

Status Branch Result
8.x

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

Questions ?

Please refer to the Backport tool documentation

maryam-saeidi added a commit to maryam-saeidi/kibana that referenced this pull request Oct 24, 2024
…gnostic framework (elastic#195902)

Part of elastic#183378

## Summary
This PR moves the first Custom threshold rule test to the deployment
agnostic test. The rest will follow in a follow-up PR.

## How to run

To run serverless
```
node scripts/functional_tests_server --config x-pack/test/api_integration/deployment_agnostic/configs/serverless/oblt.serverless.config.ts
node scripts/functional_test_runner --config x-pack/test/api_integration/deployment_agnostic/configs/serverless/oblt.serverless.config.ts --grep="Custom Threshold rule"
```

To run stateful
```
node scripts/functional_tests_server --config x-pack/test/api_integration/deployment_agnostic/configs/stateful/oblt.stateful.config.ts
node scripts/functional_test_runner --config x-pack/test/api_integration/deployment_agnostic/configs/stateful/oblt.stateful.config.ts --grep="Custom Threshold rule"
```

### TODO

- [x] elastic#195890
- [x] Test in MKI before merging

#### How to run tests on MKI

According to this
[discussion](elastic/observability-dev#3519 (comment)),
we should test in MKI environment before merging. For details on how to
run in MKI, see [this section of the
document](https://docs.google.com/document/d/1tiax7xoDYwFXYZjRTgVKkVMjN-SQzBWk4yn1JY6Z5UY/edit#heading=h.ece2z8p74izh)
and [this
readme](https://github.com/elastic/kibana/blob/main/x-pack/test_serverless/README.md#run-tests-on-mki).

(cherry picked from commit 30f81ce)

# Conflicts:
#	x-pack/test/api_integration/deployment_agnostic/apis/observability/alerting/index.ts
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Oct 25, 2024
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

maryam-saeidi added a commit that referenced this pull request Oct 25, 2024
…ment agnostic framework (#195902) (#197632)

# Backport

This will backport the following commits from `main` to `8.x`:
- [Migrate Custom threshold > AVG - PCT - FIRED test to the deployment
agnostic framework
(#195902)](#195902)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Maryam
Saeidi","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-24T10:22:59Z","message":"Migrate
Custom threshold > AVG - PCT - FIRED test to the deployment agnostic
framework (#195902)\n\nPart of #183378\r\n\r\n## Summary\r\nThis PR
moves the first Custom threshold rule test to the deployment\r\nagnostic
test. The rest will follow in a follow-up PR.\r\n\r\n## How to
run\r\n\r\nTo run serverless\r\n```\r\nnode
scripts/functional_tests_server --config
x-pack/test/api_integration/deployment_agnostic/configs/serverless/oblt.serverless.config.ts\r\nnode
scripts/functional_test_runner --config
x-pack/test/api_integration/deployment_agnostic/configs/serverless/oblt.serverless.config.ts
--grep=\"Custom Threshold rule\"\r\n```\r\n\r\nTo run
stateful\r\n```\r\nnode scripts/functional_tests_server --config
x-pack/test/api_integration/deployment_agnostic/configs/stateful/oblt.stateful.config.ts\r\nnode
scripts/functional_test_runner --config
x-pack/test/api_integration/deployment_agnostic/configs/stateful/oblt.stateful.config.ts
--grep=\"Custom Threshold rule\"\r\n```\r\n\r\n### TODO\r\n\r\n- [x]
https://github.com/elastic/kibana/pull/195890\r\n- [x] Test in MKI
before merging\r\n\r\n\r\n#### How to run tests on MKI\r\n\r\nAccording
to
this\r\n[discussion](https://github.com/elastic/observability-dev/issues/3519#issuecomment-2379914274),\r\nwe
should test in MKI environment before merging. For details on how
to\r\nrun in MKI, see [this section of
the\r\ndocument](https://docs.google.com/document/d/1tiax7xoDYwFXYZjRTgVKkVMjN-SQzBWk4yn1JY6Z5UY/edit#heading=h.ece2z8p74izh)\r\nand
[this\r\nreadme](https://github.com/elastic/kibana/blob/main/x-pack/test_serverless/README.md#run-tests-on-mki).","sha":"30f81ce4e932622e4d284b1e6af8c015c22836f5","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","backport:prev-minor","ci:build-serverless-image","Team:obs-ux-management"],"number":195902,"url":"https://github.com/elastic/kibana/pull/195902","mergeCommit":{"message":"Migrate
Custom threshold > AVG - PCT - FIRED test to the deployment agnostic
framework (#195902)\n\nPart of #183378\r\n\r\n## Summary\r\nThis PR
moves the first Custom threshold rule test to the deployment\r\nagnostic
test. The rest will follow in a follow-up PR.\r\n\r\n## How to
run\r\n\r\nTo run serverless\r\n```\r\nnode
scripts/functional_tests_server --config
x-pack/test/api_integration/deployment_agnostic/configs/serverless/oblt.serverless.config.ts\r\nnode
scripts/functional_test_runner --config
x-pack/test/api_integration/deployment_agnostic/configs/serverless/oblt.serverless.config.ts
--grep=\"Custom Threshold rule\"\r\n```\r\n\r\nTo run
stateful\r\n```\r\nnode scripts/functional_tests_server --config
x-pack/test/api_integration/deployment_agnostic/configs/stateful/oblt.stateful.config.ts\r\nnode
scripts/functional_test_runner --config
x-pack/test/api_integration/deployment_agnostic/configs/stateful/oblt.stateful.config.ts
--grep=\"Custom Threshold rule\"\r\n```\r\n\r\n### TODO\r\n\r\n- [x]
https://github.com/elastic/kibana/pull/195890\r\n- [x] Test in MKI
before merging\r\n\r\n\r\n#### How to run tests on MKI\r\n\r\nAccording
to
this\r\n[discussion](https://github.com/elastic/observability-dev/issues/3519#issuecomment-2379914274),\r\nwe
should test in MKI environment before merging. For details on how
to\r\nrun in MKI, see [this section of
the\r\ndocument](https://docs.google.com/document/d/1tiax7xoDYwFXYZjRTgVKkVMjN-SQzBWk4yn1JY6Z5UY/edit#heading=h.ece2z8p74izh)\r\nand
[this\r\nreadme](https://github.com/elastic/kibana/blob/main/x-pack/test_serverless/README.md#run-tests-on-mki).","sha":"30f81ce4e932622e4d284b1e6af8c015c22836f5"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/195902","number":195902,"mergeCommit":{"message":"Migrate
Custom threshold > AVG - PCT - FIRED test to the deployment agnostic
framework (#195902)\n\nPart of #183378\r\n\r\n## Summary\r\nThis PR
moves the first Custom threshold rule test to the deployment\r\nagnostic
test. The rest will follow in a follow-up PR.\r\n\r\n## How to
run\r\n\r\nTo run serverless\r\n```\r\nnode
scripts/functional_tests_server --config
x-pack/test/api_integration/deployment_agnostic/configs/serverless/oblt.serverless.config.ts\r\nnode
scripts/functional_test_runner --config
x-pack/test/api_integration/deployment_agnostic/configs/serverless/oblt.serverless.config.ts
--grep=\"Custom Threshold rule\"\r\n```\r\n\r\nTo run
stateful\r\n```\r\nnode scripts/functional_tests_server --config
x-pack/test/api_integration/deployment_agnostic/configs/stateful/oblt.stateful.config.ts\r\nnode
scripts/functional_test_runner --config
x-pack/test/api_integration/deployment_agnostic/configs/stateful/oblt.stateful.config.ts
--grep=\"Custom Threshold rule\"\r\n```\r\n\r\n### TODO\r\n\r\n- [x]
https://github.com/elastic/kibana/pull/195890\r\n- [x] Test in MKI
before merging\r\n\r\n\r\n#### How to run tests on MKI\r\n\r\nAccording
to
this\r\n[discussion](https://github.com/elastic/observability-dev/issues/3519#issuecomment-2379914274),\r\nwe
should test in MKI environment before merging. For details on how
to\r\nrun in MKI, see [this section of
the\r\ndocument](https://docs.google.com/document/d/1tiax7xoDYwFXYZjRTgVKkVMjN-SQzBWk4yn1JY6Z5UY/edit#heading=h.ece2z8p74izh)\r\nand
[this\r\nreadme](https://github.com/elastic/kibana/blob/main/x-pack/test_serverless/README.md#run-tests-on-mki).","sha":"30f81ce4e932622e4d284b1e6af8c015c22836f5"}}]}]
BACKPORT-->
@kibanamachine kibanamachine added v8.17.0 and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Oct 25, 2024
maryam-saeidi added a commit that referenced this pull request Nov 7, 2024
## Summary

Use getUrlPartsWithStrippedDefaultPort to avoid this
[issue](#195902 (comment))
on MKI:

```
Serverless Observability - Deployment-agnostic api integration - Custom Threshold rule AVG - PCT - FIRED Rule creation should set correct action variables

Error: expected 'https://bk-serverless-ftr-3067-e697d43e3ad9-e0ac80.kb.eu-west-1.aws.qa.elastic.cloud/app/observability/alerts/1e0c2d3e-e5c2-4bfe-9df0-d46681253b9f'
to sort of equal 'https://bk-serverless-ftr-3067-e697d43e3ad9-e0ac80.kb.eu-west-1.aws.qa.elastic.cloud:443/app/observability/alerts/1e0c2d3e-e5c2-4bfe-9df0-d46681253b9f'
```
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 7, 2024
…c#199264)

## Summary

Use getUrlPartsWithStrippedDefaultPort to avoid this
[issue](elastic#195902 (comment))
on MKI:

```
Serverless Observability - Deployment-agnostic api integration - Custom Threshold rule AVG - PCT - FIRED Rule creation should set correct action variables

Error: expected 'https://bk-serverless-ftr-3067-e697d43e3ad9-e0ac80.kb.eu-west-1.aws.qa.elastic.cloud/app/observability/alerts/1e0c2d3e-e5c2-4bfe-9df0-d46681253b9f'
to sort of equal 'https://bk-serverless-ftr-3067-e697d43e3ad9-e0ac80.kb.eu-west-1.aws.qa.elastic.cloud:443/app/observability/alerts/1e0c2d3e-e5c2-4bfe-9df0-d46681253b9f'
```

(cherry picked from commit 996104f)
mbondyra pushed a commit to mbondyra/kibana that referenced this pull request Nov 8, 2024
…c#199264)

## Summary

Use getUrlPartsWithStrippedDefaultPort to avoid this
[issue](elastic#195902 (comment))
on MKI:

```
Serverless Observability - Deployment-agnostic api integration - Custom Threshold rule AVG - PCT - FIRED Rule creation should set correct action variables

Error: expected 'https://bk-serverless-ftr-3067-e697d43e3ad9-e0ac80.kb.eu-west-1.aws.qa.elastic.cloud/app/observability/alerts/1e0c2d3e-e5c2-4bfe-9df0-d46681253b9f'
to sort of equal 'https://bk-serverless-ftr-3067-e697d43e3ad9-e0ac80.kb.eu-west-1.aws.qa.elastic.cloud:443/app/observability/alerts/1e0c2d3e-e5c2-4bfe-9df0-d46681253b9f'
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) ci:build-serverless-image release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-management Observability Management User Experience Team v8.17.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants