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

[SLO] [Alerting] deployment agnostic slo burn rate rule tests #187924

Merged
merged 38 commits into from
Aug 21, 2024

Conversation

mgiota
Copy link
Contributor

@mgiota mgiota commented Jul 9, 2024

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.

  1. Add an oblt.stateful.config.ts file to complement the existing oblt.serverless.config.ts file to ensure the tests are run in CI
  2. Ensure our test config is added to the buildkite pipepline
  3. Add the alerting service to the new deployment_agnostic/services directory.
  4. Port the tests over to the new deployment_agnostic directory

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="Burn rate 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="Burn rate rule"

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 use suiteTags instead of mochaOps

Adding following in the config files:

serverless config

suiteTags: {
        include: ['serverless'],
      },

ess config

suiteTags: {
        include: ['ess'],
      },

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

  • This PR uses suiteTags for tagging the tests appropriately. We decide through following labels in which environment the tests are going to be executed:
    • @ess: Runs in an ESS environment (on-prem installation) as part of the CI validation on PRs.
    • @serverless: Runs in a serverless environment.
  • It introduces a new folder 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 issue
  • Within this folder, there is a "config" subdirectory that stores base configurations specific to both the Serverless and ESS environments. These configurations build upon the base configuration provided by test_serverless and api_integration, incorporating additional settings such as environment variables and tagging options.
  • The file x-pack/test/observability_solution_api_integration/test_suites/alerting/burn_rate/burn_rate_rule.ts is functional in both Serverless and ESS
  • It removes the existing burn rate rule from x-pack/test_serverless/api_integration/test_suites/observability/alerting/burn_rate/burn_rate_rule.ts
  • The alertingApi and sloApi services are moved to test/api_integration servers

In the screenshot below you can see the test_suites folder structure, after having migrated the current slo burn rate rule. We recommend having an alerting and slo subfolders. Rest observability apps could be added as another subfolder under test_suites. As part of this PR, the alerting > burn_rate subfolders are created.

Screenshot 2024-05-13 at 09 21 28

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.

cd x-pack/test/observability_solution_api_integration

// SERVERLESS
npm run alerting_burn_rate:server:serverless
npm run alerting_burn_rate:runner:serverless

// ESS
npm run alerting_burn_rate:server:ess
npm run alerting_burn_rate:runner:ess

CI

  • It includes a new entry in the ftr_configs.yml to execute the newly added tests in the pipeline.
  • It involves the addition of 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.

@obltmachine
Copy link

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

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

@mgiota mgiota force-pushed the slo_burn_rate_deployment_agnostic_tests branch from dcb77d1 to f495936 Compare July 10, 2024 07:47
FetchHistoricalSummaryResponse,
} from '@kbn/slo-schema';
import * as t from 'io-ts';
import { FtrProviderContext } from '../../functional/ftr_provider_context';
Copy link
Contributor Author

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';

Copy link
Member

Choose a reason for hiding this comment

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

} from '@kbn/slo-schema';
import * as t from 'io-ts';
import { FtrProviderContext } from '../../functional/ftr_provider_context';
import { RoleCredentials } from '../../../test_serverless/shared/services';
Copy link
Contributor Author

@mgiota mgiota Jul 10, 2024

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.

ess
Screenshot 2024-07-09 at 14 37 54

Screenshot 2024-07-09 at 14 41 37

serverless

Screenshot 2024-07-09 at 14 42 45

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.
Copy link
Contributor Author

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.

Copy link
Member

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

.eslintrc.js Outdated Show resolved Hide resolved
@dominiqueclarke dominiqueclarke changed the title deployment agnostic slo burn rate rule tests [SLO] [Alerting] deployment agnostic slo burn rate rule tests Aug 13, 2024
@dominiqueclarke dominiqueclarke added chore release_note:skip Skip the PR/issue when compiling release notes v8.15.0 v8.16.0 and removed v8.15.0 labels Aug 13, 2024
@dominiqueclarke dominiqueclarke marked this pull request as ready for review August 13, 2024 12:38
…mgiota/kibana into slo_burn_rate_deployment_agnostic_tests
Copy link
Contributor

@dominiqueclarke dominiqueclarke left a comment

Choose a reason for hiding this comment

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

LGTM

@mgiota
Copy link
Contributor Author

mgiota commented Aug 20, 2024

Thanks @dominiqueclarke and @dmlemeshko for taking care of this! I am testing now locally the PR and doing a code review

@mgiota
Copy link
Contributor Author

mgiota commented Aug 20, 2024

@dominiqueclarke @dmlemeshko I noticed something weird. When I ran the tests for stateful using above mentioned commands:

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

Serverless Observability - Deployment-agnostic api integration tests is being printed.

Screen.Recording.2024-08-20.at.18.28.25.mov

@dominiqueclarke
Copy link
Contributor

@dominiqueclarke @dmlemeshko I noticed something weird. When I ran the tests for stateful using above mentioned commands:

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

Serverless Observability - Deployment-agnostic api integration tests is being printed.

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

@mgiota
Copy link
Contributor Author

mgiota commented Aug 20, 2024

@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.

@mgiota mgiota requested a review from dmlemeshko August 21, 2024 08:28
dmlemeshko added a commit that referenced this pull request Aug 21, 2024
## 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`
Copy link
Member

@pheyos pheyos left a 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.

@mgiota
Copy link
Contributor Author

mgiota commented Aug 21, 2024

@elasticmachine merge upstream

@mgiota mgiota enabled auto-merge (squash) August 21, 2024 14:31
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #90 / Rules Management - Prebuilt Rules - Prebuilt Rules Management @ess @serverless @skipInServerlessMKI Prebuilt Rules status get_prebuilt_rules_status should return empty structure when no prebuilt rule assets

Metrics [docs]

✅ unchanged

History

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

cc @mgiota

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting chore release_note:skip Skip the PR/issue when compiling release notes v8.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants