-
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
[ResponseOps] Bring back EuiThemeProvider to fix o11y and stack monitoring rules in stack management #188724
[ResponseOps] Bring back EuiThemeProvider to fix o11y and stack monitoring rules in stack management #188724
Conversation
… stack management
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
@@ -321,6 +371,19 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => { | |||
await discardNewRuleCreation(); | |||
}); | |||
|
|||
// Related issue that this test is trying to prevent: | |||
// https://github.com/elastic/kibana/issues/186969 | |||
it('should successfully show the APM error count rule flyout', async () => { |
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.
Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services) |
💚 Build Succeeded
Metrics [docs]Async chunks
HistoryTo update your PR or re-run it, just comment with: |
@@ -14,10 +14,11 @@ interface Props { | |||
value: React.ReactNode; | |||
children?: React.ReactNode; | |||
color?: ExpressionColor; | |||
dataTestSubj?: string; |
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.
I wonder if it would be better to name it data-test-subj
instead?
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.
I used dataTestSubj
since this is our component not Eui, but I can change it if that's preferred. I also saw dataTestSubj
used in other places in APM plugin such as this.
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.
Does it mean when I want to access this prop, I should do something like:
const { title, value, children, color, 'data-test-subj': dataTestSubj } = props;
or
data-test-subj={props['data-test-subj']}
The current implementation seems simpler, IMO.
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.
IMO this attribute should preferably be all in either kebab-case or cameCase, but both conventions are used all over Kibana. My comment was a nit though. Anything you decide will be fine with me.
@cnasikas I checked all the observability rules in stack management, and they seemed fine (In case of an issue, we still have the option of creating them via observability.) Regarding stack management rules, most of them had the same issue, and this fix will solve the problem. Would you mind checking the rest of the rules to make sure they work with this fix? |
Thanks @maryam-saeidi! Yes, we will test the rest of them to be 100% sure that the fix works and does not break the rest of the rules. |
Flaky Test Runner Stats🎉 All tests passed! - kibana-flaky-test-suite-runner#6603[✅] x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/config.ts: 100/100 tests passed. |
Pinging @elastic/response-ops (Team:ResponseOps) |
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.
I tested all rule types and for each rule type, I interacted with all inputs, charts, and KQL bars. It is working as expected. LGTM!
…oring rules in stack management (elastic#188724) Related to elastic#186969 and elastic/response-ops-team#218 ## Summary This PR brings back EuiThemeProvider to fix o11y and stack monitoring rules in stack management. ## To check/do - [x] Add an APM test that fails without this fix - [x] Check if this solves the related SDH for [CPU Usage](elastic/sdh-kibana#4829) - Yes, it will solve that issue ([comment](elastic/sdh-kibana#4829 (comment))) - [x] Smoke test **ALL** rule types in stack management - Can we load the rule form? - Can we adjust all the input fields without error? - Does it work both with and without data in the preview chart? (cherry picked from commit ed32c98)
…oring rules in stack management (elastic#188724) Related to elastic#186969 and elastic/response-ops-team#218 ## Summary This PR brings back EuiThemeProvider to fix o11y and stack monitoring rules in stack management. ## To check/do - [x] Add an APM test that fails without this fix - [x] Check if this solves the related SDH for [CPU Usage](elastic/sdh-kibana#4829) - Yes, it will solve that issue ([comment](elastic/sdh-kibana#4829 (comment))) - [x] Smoke test **ALL** rule types in stack management - Can we load the rule form? - Can we adjust all the input fields without error? - Does it work both with and without data in the preview chart? (cherry picked from commit ed32c98)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…k monitoring rules in stack management (#188724) (#188905) # Backport This will backport the following commits from `main` to `8.15`: - [[ResponseOps] Bring back EuiThemeProvider to fix o11y and stack monitoring rules in stack management (#188724)](#188724) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Maryam Saeidi","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-07-23T08:42:42Z","message":"[ResponseOps] Bring back EuiThemeProvider to fix o11y and stack monitoring rules in stack management (#188724)\n\nRelated to #186969 and\r\nelastic/response-ops-team#218\r\n\r\n## Summary\r\n\r\nThis PR brings back EuiThemeProvider to fix o11y and stack monitoring\r\nrules in stack management.\r\n\r\n## To check/do\r\n\r\n- [x] Add an APM test that fails without this fix\r\n- [x] Check if this solves the related SDH for [CPU\r\nUsage](https://github.com/elastic/sdh-kibana/issues/4829)\r\n- Yes, it will solve that issue\r\n([comment](https://github.com/elastic/sdh-kibana/issues/4829#issuecomment-2242509680))\r\n- [x] Smoke test **ALL** rule types in stack management\r\n - Can we load the rule form?\r\n - Can we adjust all the input fields without error?\r\n - Does it work both with and without data in the preview chart?","sha":"ed32c980722efa113e70f0f409ee95e36a9f7a15","branchLabelMapping":{"^v8.16.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:ResponseOps","ci:project-deploy-observability","Team:obs-ux-infra_services","apm:review","v8.15.0","v8.16.0","v8.14.4"],"title":"[ResponseOps] Bring back EuiThemeProvider to fix o11y and stack monitoring rules in stack management","number":188724,"url":"https://github.com/elastic/kibana/pull/188724","mergeCommit":{"message":"[ResponseOps] Bring back EuiThemeProvider to fix o11y and stack monitoring rules in stack management (#188724)\n\nRelated to #186969 and\r\nelastic/response-ops-team#218\r\n\r\n## Summary\r\n\r\nThis PR brings back EuiThemeProvider to fix o11y and stack monitoring\r\nrules in stack management.\r\n\r\n## To check/do\r\n\r\n- [x] Add an APM test that fails without this fix\r\n- [x] Check if this solves the related SDH for [CPU\r\nUsage](https://github.com/elastic/sdh-kibana/issues/4829)\r\n- Yes, it will solve that issue\r\n([comment](https://github.com/elastic/sdh-kibana/issues/4829#issuecomment-2242509680))\r\n- [x] Smoke test **ALL** rule types in stack management\r\n - Can we load the rule form?\r\n - Can we adjust all the input fields without error?\r\n - Does it work both with and without data in the preview chart?","sha":"ed32c980722efa113e70f0f409ee95e36a9f7a15"}},"sourceBranch":"main","suggestedTargetBranches":["8.15","8.14"],"targetPullRequestStates":[{"branch":"8.15","label":"v8.15.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/188724","number":188724,"mergeCommit":{"message":"[ResponseOps] Bring back EuiThemeProvider to fix o11y and stack monitoring rules in stack management (#188724)\n\nRelated to #186969 and\r\nelastic/response-ops-team#218\r\n\r\n## Summary\r\n\r\nThis PR brings back EuiThemeProvider to fix o11y and stack monitoring\r\nrules in stack management.\r\n\r\n## To check/do\r\n\r\n- [x] Add an APM test that fails without this fix\r\n- [x] Check if this solves the related SDH for [CPU\r\nUsage](https://github.com/elastic/sdh-kibana/issues/4829)\r\n- Yes, it will solve that issue\r\n([comment](https://github.com/elastic/sdh-kibana/issues/4829#issuecomment-2242509680))\r\n- [x] Smoke test **ALL** rule types in stack management\r\n - Can we load the rule form?\r\n - Can we adjust all the input fields without error?\r\n - Does it work both with and without data in the preview chart?","sha":"ed32c980722efa113e70f0f409ee95e36a9f7a15"}},{"branch":"8.14","label":"v8.14.4","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Maryam Saeidi <[email protected]>
…k monitoring rules in stack management (#188724) (#188904) # Backport This will backport the following commits from `main` to `8.14`: - [[ResponseOps] Bring back EuiThemeProvider to fix o11y and stack monitoring rules in stack management (#188724)](#188724) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Maryam Saeidi","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-07-23T08:42:42Z","message":"[ResponseOps] Bring back EuiThemeProvider to fix o11y and stack monitoring rules in stack management (#188724)\n\nRelated to #186969 and\r\nelastic/response-ops-team#218\r\n\r\n## Summary\r\n\r\nThis PR brings back EuiThemeProvider to fix o11y and stack monitoring\r\nrules in stack management.\r\n\r\n## To check/do\r\n\r\n- [x] Add an APM test that fails without this fix\r\n- [x] Check if this solves the related SDH for [CPU\r\nUsage](https://github.com/elastic/sdh-kibana/issues/4829)\r\n- Yes, it will solve that issue\r\n([comment](https://github.com/elastic/sdh-kibana/issues/4829#issuecomment-2242509680))\r\n- [x] Smoke test **ALL** rule types in stack management\r\n - Can we load the rule form?\r\n - Can we adjust all the input fields without error?\r\n - Does it work both with and without data in the preview chart?","sha":"ed32c980722efa113e70f0f409ee95e36a9f7a15","branchLabelMapping":{"^v8.16.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:ResponseOps","ci:project-deploy-observability","Team:obs-ux-infra_services","apm:review","v8.15.0","v8.16.0","v8.14.4"],"title":"[ResponseOps] Bring back EuiThemeProvider to fix o11y and stack monitoring rules in stack management","number":188724,"url":"https://github.com/elastic/kibana/pull/188724","mergeCommit":{"message":"[ResponseOps] Bring back EuiThemeProvider to fix o11y and stack monitoring rules in stack management (#188724)\n\nRelated to #186969 and\r\nelastic/response-ops-team#218\r\n\r\n## Summary\r\n\r\nThis PR brings back EuiThemeProvider to fix o11y and stack monitoring\r\nrules in stack management.\r\n\r\n## To check/do\r\n\r\n- [x] Add an APM test that fails without this fix\r\n- [x] Check if this solves the related SDH for [CPU\r\nUsage](https://github.com/elastic/sdh-kibana/issues/4829)\r\n- Yes, it will solve that issue\r\n([comment](https://github.com/elastic/sdh-kibana/issues/4829#issuecomment-2242509680))\r\n- [x] Smoke test **ALL** rule types in stack management\r\n - Can we load the rule form?\r\n - Can we adjust all the input fields without error?\r\n - Does it work both with and without data in the preview chart?","sha":"ed32c980722efa113e70f0f409ee95e36a9f7a15"}},"sourceBranch":"main","suggestedTargetBranches":["8.15","8.14"],"targetPullRequestStates":[{"branch":"8.15","label":"v8.15.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/188724","number":188724,"mergeCommit":{"message":"[ResponseOps] Bring back EuiThemeProvider to fix o11y and stack monitoring rules in stack management (#188724)\n\nRelated to #186969 and\r\nelastic/response-ops-team#218\r\n\r\n## Summary\r\n\r\nThis PR brings back EuiThemeProvider to fix o11y and stack monitoring\r\nrules in stack management.\r\n\r\n## To check/do\r\n\r\n- [x] Add an APM test that fails without this fix\r\n- [x] Check if this solves the related SDH for [CPU\r\nUsage](https://github.com/elastic/sdh-kibana/issues/4829)\r\n- Yes, it will solve that issue\r\n([comment](https://github.com/elastic/sdh-kibana/issues/4829#issuecomment-2242509680))\r\n- [x] Smoke test **ALL** rule types in stack management\r\n - Can we load the rule form?\r\n - Can we adjust all the input fields without error?\r\n - Does it work both with and without data in the preview chart?","sha":"ed32c980722efa113e70f0f409ee95e36a9f7a15"}},{"branch":"8.14","label":"v8.14.4","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Maryam Saeidi <[email protected]>
Related to #186969 and elastic/response-ops-team#218
Summary
This PR brings back EuiThemeProvider to fix o11y and stack monitoring rules in stack management.
To check/do