-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[SLO] [Alerting] deployment agnostic slo burn rate rule tests #187924
[SLO] [Alerting] deployment agnostic slo burn rate rule tests #187924
Conversation
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
dcb77d1
to
f495936
Compare
FetchHistoricalSummaryResponse, | ||
} from '@kbn/slo-schema'; | ||
import * as t from 'io-ts'; | ||
import { FtrProviderContext } from '../../functional/ftr_provider_context'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pheyos I was trying to recall why I load the ftr provider context from this location, and I think I got inspired by this change of yours. Does this feel right? Before it was like that import { FtrProviderContext } from '../ftr_provider_context';
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI: I already moved this service to https://github.com/elastic/kibana/blob/main/x-pack/test/api_integration/deployment_agnostic/services/slo_api.ts
} from '@kbn/slo-schema'; | ||
import * as t from 'io-ts'; | ||
import { FtrProviderContext } from '../../functional/ftr_provider_context'; | ||
import { RoleCredentials } from '../../../test_serverless/shared/services'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Role based testing was introduced in this PR. As part of that PR an svlUserManager
service was used within the test itselft to create an api role for admin for serverless.
With current PR, where the goal is to have only one test used by both ess and serverless, current approach doesn't work. We would need to check the environment within the test. What I tried instead was to move this role-based logic within the slo_api
service itself. The slo api will check the service
and will create the api key on serverless accordingly.
This worked out well. As you can see below in case of ess it creates all the predefined roles and users, where as on serverless it only creates an api key.
serverless
Let's explore if this approach feels right, and then I will proceed applying the changes to the alerting_api
service as well. My concern with this approach is that slo_api
service is added within the deployment agnostic services
, but internally it uses the config service to check if it is serverless or stateful. What are your thoughts on this? Any other alternatives?
- Test suites are prefixed with specific tags to determine their execution in particular environments or to exclude them from specific environments. | ||
|
||
- We are using the following tags: | ||
* `@ess`: Runs in an ESS environment (on-prem installation) as part of the CI validation on PRs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pheyos Let's agree on the tags we want to use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to apply tags, at least at the moment. We have 2 config files:
- x-pack/test/api_integration/deployment_agnostic/oblt.stateful.config.ts
- x-pack/test/api_integration/deployment_agnostic/oblt.serverless.config.ts
and index file to load all Oblt tests:
- x-pack/test/api_integration/deployment_agnostic/oblt.index.ts
If the test is added to deployment_agnostic/
folder, it is assumed by default to be run both in serverless and stateful without extra filtering
x-pack/test_serverless/api_integration/test_suites/observability/index.ts
Show resolved
Hide resolved
x-pack/test/api_integration/deployment_agnostic/oblt.stateful.config.ts
Outdated
Show resolved
Hide resolved
…mgiota/kibana into slo_burn_rate_deployment_agnostic_tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks @dominiqueclarke and @dmlemeshko for taking care of this! I am testing now locally the PR and doing a code review |
@dominiqueclarke @dmlemeshko I noticed something weird. When I ran the tests for stateful using above mentioned commands:
Screen.Recording.2024-08-20.at.18.28.25.mov |
That is weird, I can see the override for the junit name here https://github.com/elastic/kibana/pull/187924/files#diff-3e77dd5588b23a46609fd3451860d5c3cc53b682f5d33b60430cf0d1aab76bd2R13 |
@dmlemeshko Here's the commit, where I re-organized things in order to have seperate index files for loading tests in serverless and stateful. Similar to what you try to achieve in this PR. |
## Summary While reviewing #187924 I realised that Solution Teams will have their own stateful FTR configs and the overall number of configs in `api_integration/deployment_agnostic` folder might grow up soon and it is better to re-organise it in advance. With new structure we have `configs` folder with 2 sub-folders: `stateful` and `serverless` <img width="494" alt="image" src="https://github.com/user-attachments/assets/c844ce2c-51b6-498c-b51f-0cd9203d84ea"> When _Platform_ teams add deployment-agnostic tests, it is expected to be loaded in `configs/stateful/platform.index.ts` and _at least one_ of `<project>.serverless.config` under `configs/serverless` When _Solution_ team adds deployment-agnostic tests (e.g. one of Oblt teams), it is expected to be loaded _both_ in `configs/stateful/oblt.index.ts` and `configs/serverless/oblt.index.ts`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall!
Also gave it a test run against an MKI project and it passed ✔️
Left two small nits.
x-pack/test/api_integration/deployment_agnostic/services/alerting_api.ts
Outdated
Show resolved
Hide resolved
x-pack/test/api_integration/deployment_agnostic/services/alerting_api.ts
Outdated
Show resolved
Hide resolved
…ing_api.ts Co-authored-by: Robert Oskamp <[email protected]>
@elasticmachine merge upstream |
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]
History
To update your PR or re-run it, just comment with: cc @mgiota |
Addresses #179549
Relates to #183113
Update
Since the Appex QA team has taken on deployment agnostic tests, a lot of the original implementation of this PR has changed. Now that the Appex QA team has provided a current directly to write deployment agnostic tests, the burn rate rule tests have been moved here.
To finish onboarding the burn rate rule test to this new framework, the following was done.
oblt.stateful.config.ts
file to complement the existingoblt.serverless.config.ts
file to ensure the tests are run in CIdeployment_agnostic/services
directory.deployment_agnostic
directoryTo run serverless
To run stateful
For context, I've kept the history from the original PR description below.
🍒 History
A new type of config file will be allowed for API integration and functional tests within the
x-pack/test
folder, using a pattern of*.serverless.config.ts
— these config files will specify configuration needed to run a set of tests in a serverless deployment context.FTR tests already make use of the
it.tags(['my-tag-1', 'my-tag-2'])
pattern, and so we would like to stay with that pattern rather than introducing a new type of mocha tag in the it block's "description" as it was introduced in this POC. The difference with the previous PR in terms of tagging is that we usesuiteTags
instead ofmochaOps
Adding following in the config files:
serverless config
ess config
and then adding
this.tags(['serverless', 'ess'])
in the test suite instructs the test runner to run the same test suite in both environments.In order to keep things simple, we stay with the current skip approach, which means that flaky tests will be skipped for all environments by appending .skip() to the suite or to specific test cases.
Description
suiteTags
for tagging the tests appropriately. We decide through following labels in which environment the tests are going to be executed:x-pack/test/observability_solution_api_integration
which will serve as a centralized location for all tests by obs-ux-management team that must be run in Serverless and ESS environments. A list of all tests can be found in the R&D issuex-pack/test/observability_solution_api_integration/test_suites/alerting/burn_rate/burn_rate_rule.ts
is functional in both Serverless and ESSx-pack/test_serverless/api_integration/test_suites/observability/alerting/burn_rate/burn_rate_rule.ts
alertingApi
andsloApi
services are moved totest/api_integration
serversIn the screenshot below you can see the
test_suites
folder structure, after having migrated the current slo burn rate rule. We recommend having analerting
andslo
subfolders. Rest observability apps could be added as another subfolder under test_suites. As part of this PR, thealerting > burn_rate
subfolders are created.How to run locally
You can navigate into the new
observability_solution_api_integration
folder and use following commands to run the tests in serverless and ess environments accordingly. You can find more information in the README file of the observability_solution_api_integration folder.CI
ftr_configs.yml
to execute the newly added tests in the pipeline.suiteTags
in both serverless/config.base.ts and ess/config.base.ts. In the case of serverless, it includes @serverless while excluding @skipInServerless. Similarly, for ess, it includes @ess and excludes @skipInEss.Quality Gates and MKI pipeline
The Platform team will support config files within
x-pack/test
folder with a pattern of*.serverless.config.ts
, so these tests will be included in Kibana's Quality gates and will be run against a real MKI environment.