[performance] support triggering subset of journeys against KIbana PR in CI #193175
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dmlemeshko
changed the title
dmlemeshko
added
release_note:skip
wayneseymour
approved these changes
Sep 18, 2024
| @@ -39,8 +39,13 @@ if [ "$BUILDKITE_PIPELINE_SLUG" == "kibana-performance-data-set-extraction" ]; t | |||
| node scripts/run_performance.js --kibana-install-dir "$KIBANA_BUILD_LOCATION" --skip-warmup | |||
| else | |||
| # pipeline should use bare metal static worker | |||
| echo "--- Running performance tests" | |||
| node scripts/run_performance.js --kibana-install-dir "$KIBANA_BUILD_LOCATION" | |||
| if [[ -z "${JOURNEYS_GROUP+x}" ]]; then | |||
There was a problem hiding this comment.
The +x is checking whether a variable is set at all, regardless of weather it has a specific value or empty string.
Now I wonder if we want to pass empty string to the script: it will handle it and return clear error message, but I also can do it directly in bash. Thoughts?
There was a problem hiding this comment.
🤔
Ahh, ok so it's handled in nodejs here right: https://github.com/dmlemeshko/kibana/blob/kbn-journeys/run-subset-of-journeys/src/dev/performance/run_performance_cli.ts#L77
But to handle it in bash....
So, -z string in a bash conditional expression means True if the length of string is zero
...
There was a problem hiding this comment.
Yeah so currently, the env var can be an empty string, just like you said.
So, the conditional expression could be: if [[ "${JOURNEYS_GROUP}" = "" ]]; then
There was a problem hiding this comment.
In the truth block, you'd return the clear error message
There was a problem hiding this comment.
I can go with the current solution.
if [[ "${JOURNEYS_GROUP}" = "" ]] could break if the variable is missing
There was a problem hiding this comment.
Break? It would just be an empty string right?
JOURNEYS_GROUP="" .buildkite/scripts/steps/functional/performance_playwright.sh
-- same as ---
.buildkite/scripts/steps/functional/performance_playwright.sh
Am I wrong here? I'd like to know to be honest lol
There was a problem hiding this comment.
@wayneseymour - when we run our scripts in strict mode, it will fail with an error on undefined variables. See:
➜ elastic-kibana git:(main) ✗ cat test.sh
#!/usr/bin/env bash
set -euo pipefail
echo "This is a missing variable: $TOMATO"
➜ elastic-kibana git:(main) ✗ bash test.sh
test.sh: line 5: TOMATO: unbound variable
➜ elastic-kibana git:(main) ✗ TOMATO= bash test.sh
This is a missing variable:
This is why you see in many places this notion: ${TOMATO:-} - this means $TOMATO or default to (notice the empty string :) )
Thus the expression I'm used to is:
if [[ "${VARIABLE:-}" == "" ]]; then
# VARIABLE was empty or missing
fi
But I think Dima's +x is just as good
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]
To update your PR or re-run it, just comment with: cc @dmlemeshko |
| node scripts/run_performance.js --kibana-install-dir "$KIBANA_BUILD_LOCATION" | ||
| if [[ -z "${JOURNEYS_GROUP+x}" ]]; then | ||
| echo "--- Running performance tests" | ||
| node scripts/run_performance.js --kibana-install-dir "$KIBANA_BUILD_LOCATION" |
There was a problem hiding this comment.
@delanni it looks like empty is ok, based on this line right?
There was a problem hiding this comment.
I flipped the condition in my head, it is in fact OK like it is
There was a problem hiding this comment.
Meaning, an empty or missing group would end up in the first branch, where we run everything
There was a problem hiding this comment.
yes, exactly. Thank you both for double check
There was a problem hiding this comment.
Yeah this was a good nerd-scussion! ❤️ hahahaa
delanni
approved these changes
Sep 18, 2024
dmlemeshko
added
v8.16.0
backport:version
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Sep 23, 2024
… in CI (elastic#193175) ## Summary It’s common request for Dev teams to run specific journeys on a PR to compare performance metrics against the `main` branch. These requests usually focus on a particular area, such as the Dashboard or Discover app. To streamline the process, this PR groups relevant journeys into categories that can be triggered through an environment variable. For example, setting `JOURNEYS_GROUP=dashboard` will execute only the three dashboard-specific journeys, which are (usually) sufficient for evaluating the performance impact of code changes within the Dashboard app. Current Process for Triggering Performance Builds: - Create a new kibana-single-user-performance [build](https://buildkite.com/elastic/kibana-single-user-performance#new) - Provide the following arguments: Branch: `refs/pull/<PR_number>/head` Under Options, set the environment variable: `JOURNEYS_GROUP=<group_name>` Currently supported journey groups: - kibanaStartAndLoad - crud - dashboard - discover - maps - ml [Build example ](https://buildkite.com/elastic/kibana-single-user-performance/builds/14427) Each group focuses on a specific set of journeys tied to its respective area in Kibana, allowing for more targeted performance testing. Since running group takes ~5-10 min on bare metal worker, it should not delay the regular (every 3h) runs against `main` branch test locally with `node scripts/run_performance.js --group <group_name>` (cherry picked from commit f5975d2)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
kibanamachine
added a commit
that referenced
this pull request
Sep 23, 2024
…ana PR in CI (#193175) (#193680) # Backport This will backport the following commits from `main` to `8.x`: - [[performance] support triggering subset of journeys against KIbana PR in CI (#193175)](#193175) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Dzmitry Lemechko","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-09-18T12:53:02Z","message":"[performance] support triggering subset of journeys against KIbana PR in CI (#193175)\n\n## Summary\r\n\r\nIt’s common request for Dev teams to run specific journeys on a PR to\r\ncompare performance metrics against the `main` branch. These requests\r\nusually focus on a particular area, such as the Dashboard or Discover\r\napp.\r\n\r\nTo streamline the process, this PR groups relevant journeys into\r\ncategories that can be triggered through an environment variable. For\r\nexample, setting `JOURNEYS_GROUP=dashboard` will execute only the three\r\ndashboard-specific journeys, which are (usually) sufficient for\r\nevaluating the performance impact of code changes within the Dashboard\r\napp.\r\n\r\nCurrent Process for Triggering Performance Builds:\r\n- Create a new kibana-single-user-performance\r\n[build](https://buildkite.com/elastic/kibana-single-user-performance#new)\r\n- Provide the following arguments:\r\n\r\nBranch: `refs/pull/<PR_number>/head`\r\nUnder Options, set the environment variable:\r\n`JOURNEYS_GROUP=<group_name>`\r\n\r\nCurrently supported journey groups:\r\n- kibanaStartAndLoad\r\n- crud\r\n- dashboard\r\n- discover\r\n- maps\r\n- ml\r\n\r\n[Build example\r\n\r\n](https://buildkite.com/elastic/kibana-single-user-performance/builds/14427)\r\nEach group focuses on a specific set of journeys tied to its respective\r\narea in Kibana, allowing for more targeted performance testing. Since\r\nrunning group takes ~5-10 min on bare metal worker, it should not delay\r\nthe regular (every 3h) runs against `main` branch\r\n\r\n\r\ntest locally with `node scripts/run_performance.js --group <group_name>`","sha":"f5975d28fa06d6de6193d5e518084c45c65b5aed","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","v8.16.0","backport:version"],"title":"[performance] support triggering subset of journeys against KIbana PR in CI","number":193175,"url":"https://github.com/elastic/kibana/pull/193175","mergeCommit":{"message":"[performance] support triggering subset of journeys against KIbana PR in CI (#193175)\n\n## Summary\r\n\r\nIt’s common request for Dev teams to run specific journeys on a PR to\r\ncompare performance metrics against the `main` branch. These requests\r\nusually focus on a particular area, such as the Dashboard or Discover\r\napp.\r\n\r\nTo streamline the process, this PR groups relevant journeys into\r\ncategories that can be triggered through an environment variable. For\r\nexample, setting `JOURNEYS_GROUP=dashboard` will execute only the three\r\ndashboard-specific journeys, which are (usually) sufficient for\r\nevaluating the performance impact of code changes within the Dashboard\r\napp.\r\n\r\nCurrent Process for Triggering Performance Builds:\r\n- Create a new kibana-single-user-performance\r\n[build](https://buildkite.com/elastic/kibana-single-user-performance#new)\r\n- Provide the following arguments:\r\n\r\nBranch: `refs/pull/<PR_number>/head`\r\nUnder Options, set the environment variable:\r\n`JOURNEYS_GROUP=<group_name>`\r\n\r\nCurrently supported journey groups:\r\n- kibanaStartAndLoad\r\n- crud\r\n- dashboard\r\n- discover\r\n- maps\r\n- ml\r\n\r\n[Build example\r\n\r\n](https://buildkite.com/elastic/kibana-single-user-performance/builds/14427)\r\nEach group focuses on a specific set of journeys tied to its respective\r\narea in Kibana, allowing for more targeted performance testing. Since\r\nrunning group takes ~5-10 min on bare metal worker, it should not delay\r\nthe regular (every 3h) runs against `main` branch\r\n\r\n\r\ntest locally with `node scripts/run_performance.js --group <group_name>`","sha":"f5975d28fa06d6de6193d5e518084c45c65b5aed"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/193175","number":193175,"mergeCommit":{"message":"[performance] support triggering subset of journeys against KIbana PR in CI (#193175)\n\n## Summary\r\n\r\nIt’s common request for Dev teams to run specific journeys on a PR to\r\ncompare performance metrics against the `main` branch. These requests\r\nusually focus on a particular area, such as the Dashboard or Discover\r\napp.\r\n\r\nTo streamline the process, this PR groups relevant journeys into\r\ncategories that can be triggered through an environment variable. For\r\nexample, setting `JOURNEYS_GROUP=dashboard` will execute only the three\r\ndashboard-specific journeys, which are (usually) sufficient for\r\nevaluating the performance impact of code changes within the Dashboard\r\napp.\r\n\r\nCurrent Process for Triggering Performance Builds:\r\n- Create a new kibana-single-user-performance\r\n[build](https://buildkite.com/elastic/kibana-single-user-performance#new)\r\n- Provide the following arguments:\r\n\r\nBranch: `refs/pull/<PR_number>/head`\r\nUnder Options, set the environment variable:\r\n`JOURNEYS_GROUP=<group_name>`\r\n\r\nCurrently supported journey groups:\r\n- kibanaStartAndLoad\r\n- crud\r\n- dashboard\r\n- discover\r\n- maps\r\n- ml\r\n\r\n[Build example\r\n\r\n](https://buildkite.com/elastic/kibana-single-user-performance/builds/14427)\r\nEach group focuses on a specific set of journeys tied to its respective\r\narea in Kibana, allowing for more targeted performance testing. Since\r\nrunning group takes ~5-10 min on bare metal worker, it should not delay\r\nthe regular (every 3h) runs against `main` branch\r\n\r\n\r\ntest locally with `node scripts/run_performance.js --group <group_name>`","sha":"f5975d28fa06d6de6193d5e518084c45c65b5aed"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Dzmitry Lemechko <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
It’s common request for Dev teams to run specific journeys on a PR to compare performance metrics against the
mainbranch. These requests usually focus on a particular area, such as the Dashboard or Discover app.To streamline the process, this PR groups relevant journeys into categories that can be triggered through an environment variable. For example, setting
JOURNEYS_GROUP=dashboardwill execute only the three dashboard-specific journeys, which are (usually) sufficient for evaluating the performance impact of code changes within the Dashboard app.Current Process for Triggering Performance Builds:
Branch:
refs/pull/<PR_number>/headUnder Options, set the environment variable:
JOURNEYS_GROUP=<group_name>Currently supported journey groups:
Build example
Each group focuses on a specific set of journeys tied to its respective area in Kibana, allowing for more targeted performance testing. Since running group takes ~5-10 min on bare metal worker, it should not delay the regular (every 3h) runs against
mainbranchtest locally with
node scripts/run_performance.js --group <group_name>