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.

Sign up for GitHub

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

Jump to bottom

[Security solution][Alert Details] Enable preview feature flag and cypress tests #188580

Merged
merged 3 commits into from
Jul 23, 2024
Merged

[Security solution][Alert Details] Enable preview feature flag and cypress tests #188580

merged 3 commits into from
Jul 23, 2024

Conversation

christineweng
Copy link
Contributor

@christineweng christineweng commented Jul 17, 2024 •
edited by kibanamachine
Loading

Summary

This PR inverted feature flag entityAlertPreviewEnabled to entityAlertPreviewDisabled and related cypress tests. The flag inversion will allow customers to disable the feature if issues arise.

Checklist

@christineweng christineweng self-assigned this Jul 17, 2024
@christineweng christineweng added release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting Security Solution Threat Hunting Team Team:Threat Hunting:Investigations Security Solution Investigations Team v8.15.0 v8.16.0 labels Jul 17, 2024
@christineweng
Copy link
Contributor Author

/ci

@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

🎉 All tests passed! - kibana-flaky-test-suite-runner#6567

[✅] Security Solution Investigations - Cypress: 25/25 tests passed.
[✅] [Serverless] Security Solution Investigations - Cypress: 25/25 tests passed.

see run history

@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

🎉 All tests passed! - kibana-flaky-test-suite-runner#6570

[✅] Security Solution Investigations - Cypress: 50/50 tests passed.
[✅] [Serverless] Security Solution Investigations - Cypress: 50/50 tests passed.

see run history

@christineweng christineweng marked this pull request as ready for review July 18, 2024 06:14
@christineweng christineweng requested review from a team as code owners July 18, 2024 06:14
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting (Team:Threat Hunting)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting-investigations (Team:Threat Hunting:Investigations)

@PhilippeOberti
Copy link
Contributor

PhilippeOberti commented Jul 18, 2024 •
edited
Loading

@christineweng have you considered switching the feature flag around? Instead of having entityAlertPreviewEnabled: true we could have entityAlertPreviewDisabled: false.

Jatin recently did this in this PR and I had done something similar in this PR a few weeks ago.

That advantage is customers can turn off the feature if something goes wrong in their environment, by updating their kibana.yml file.
With your current solution, the feature cannot be turned off by customers directly...

@christineweng
Copy link
Contributor Author

christineweng commented Jul 18, 2024 •
edited
Loading

@christineweng have you considered switching the feature flag around? Instead of having entityAlertPreviewEnabled: true we could have entityAlertPreviewDisabled: false.
...
That advantage is customers can turn off the feature if something goes wrong in their environment, by updating their kibana.yml file. With your current solution, the feature cannot be turned off by customers directly...

@PhilippeOberti Is this a new process we are promoting? I see the benefit, but it seems counter intuitive to implement technically a new flag versus turning it on and off. my concern is more around timing and the possibility of introducing bugs. @michaelolo24 wdyt?

@christineweng christineweng removed the request for review from a team July 18, 2024 18:21
@PhilippeOberti
Copy link
Contributor

@christineweng have you considered switching the feature flag around? Instead of having entityAlertPreviewEnabled: true we could have entityAlertPreviewDisabled: false.
...
That advantage is customers can turn off the feature if something goes wrong in their environment, by updating their kibana.yml file. With your current solution, the feature cannot be turned off by customers directly...

@PhilippeOberti Is this a new process we are promoting? I see the benefit, but it seems counter intuitive to implement technically a new flag versus turning it on and off. my concern is more around timing and the possibility of introducing bugs. @michaelolo24 wdyt?

I don't know if it's really a new process, but after talking about it with Sergi, Jatin and a few other people on Slack we felt it made sense, until we have a way to enable and disable feature flag via the kiaba.yml file...

I see your point, but keeping the feature flag as you have is very inflexible. If a customer has an issue, they have absolutely no way to turn things off, and would have to wait for a minor release to be available. This is pretty risky (even though in this case the risk is probably very low)

@christineweng christineweng requested a review from a team as a code owner July 18, 2024 23:17
@elasticmachine
Copy link
Contributor

elasticmachine commented Jul 19, 2024 •
edited
Loading

⏳ Build in-progress, with failures

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #3 / should open host preview when click on host when feature flag is on
  • [job] [logs] Jest Tests #3 / should open host preview when click on host when feature flag is on
  • [job] [logs] Jest Tests #3 / should open left panel when clicking on the link within a a link cell when feature flag is off
  • [job] [logs] Jest Tests #3 / should open left panel when clicking on the link within a a link cell when feature flag is off
  • [job] [logs] Jest Tests #3 / should open user preview when click on user when feature flag is on
  • [job] [logs] Jest Tests #3 / should open user preview when click on user when feature flag is on
  • [job] [logs] Jest Tests #3 / Related users should render the related user table with correct dates and indices
  • [job] [logs] Jest Tests #3 / Related users should render the related user table with correct dates and indices
  • [job] [logs] Jest Tests #3 / Related users should render user name as clicable link when feature flag is true
  • [job] [logs] Jest Tests #3 / Related users should render user name as clicable link when feature flag is true
  • [job] [logs] Jest Tests #3 / should render host details correctly
  • [job] [logs] Jest Tests #3 / should render host details correctly
  • [job] [logs] Jest Tests #3 / should render host name as clicable link when feature flag is true
  • [job] [logs] Jest Tests #3 / should render host name as clicable link when feature flag is true
  • [job] [logs] Jest Tests #3 / license is not valid should navigate to left panel entities tab when clicking on title when feature flag is off
  • [job] [logs] Jest Tests #3 / license is not valid should navigate to left panel entities tab when clicking on title when feature flag is off
  • [job] [logs] Jest Tests #3 / Related hosts should render host name as clicable link when feature flag is true
  • [job] [logs] Jest Tests #3 / Related hosts should render host name as clicable link when feature flag is true
  • [job] [logs] Jest Tests #3 / Related hosts should render the related host table with correct dates and indices
  • [job] [logs] Jest Tests #3 / Related hosts should render the related host table with correct dates and indices
  • [job] [logs] Jest Tests #3 / should render user details correctly
  • [job] [logs] Jest Tests #3 / should render user details correctly
  • [job] [logs] Jest Tests #3 / should render user name as clicable link when feature flag is true
  • [job] [logs] Jest Tests #3 / should render user name as clicable link when feature flag is true
  • [job] [logs] Jest Tests #3 / license is not valid should navigate to left panel entities tab when clicking on title when feature flag is off
  • [job] [logs] Jest Tests #3 / license is not valid should navigate to left panel entities tab when clicking on title when feature flag is off
  • [job] [logs] Jest Tests #3 / CorrelationsDetailsAlertsTable renders EuiBasicTable with correct props
  • [job] [logs] Jest Tests #3 / CorrelationsDetailsAlertsTable renders EuiBasicTable with correct props
  • [job] [logs] Jest Tests #3 / CorrelationsDetailsAlertsTable renders open preview button when feature flag is on
  • [job] [logs] Jest Tests #3 / CorrelationsDetailsAlertsTable renders open preview button when feature flag is on
  • [job] [logs] Jest Tests #3 / PrevalenceDetails should render host and user name as clickable link if feature flag is true
  • [job] [logs] Jest Tests #3 / PrevalenceDetails should render host and user name as clickable link if feature flag is true
  • [job] [logs] Jest Tests #3 / PrevalenceDetails should render the table with all data if license is platinum
  • [job] [logs] Jest Tests #3 / PrevalenceDetails should render the table with all data if license is platinum
  • [job] [logs] Jest Tests #8 / RiskInputsTab it does not render alert preview button when feature flag is disable
  • [job] [logs] Jest Tests #8 / RiskInputsTab it does not render alert preview button when feature flag is disable
  • [job] [logs] Jest Tests #8 / RiskInputsTab it renders alert preview button when feature flag is enable
  • [job] [logs] Jest Tests #8 / RiskInputsTab it renders alert preview button when feature flag is enable

History

cc @christineweng

PhilippeOberti
PhilippeOberti approved these changes Jul 19, 2024
View reviewed changes
Copy link
Contributor

@PhilippeOberti PhilippeOberti left a comment

Choose a reason for hiding this comment

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

desk tested and the code LGTM! Thanks for switching the flag around!

@christineweng christineweng added release_note:feature Makes this part of the condensed release notes and removed release_note:skip Skip the PR/issue when compiling release notes labels Jul 19, 2024
@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

🟠 Some tests failed. - kibana-flaky-test-suite-runner#6596

[❌] Security Solution Investigations - Cypress: 24/50 tests passed.
[❌] [Serverless] Security Solution Investigations - Cypress: 0/50 tests passed.

see run history

tiansivive
tiansivive approved these changes Jul 22, 2024
View reviewed changes
Copy link
Contributor

@tiansivive tiansivive left a comment

Choose a reason for hiding this comment

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

LGTM from Entity Analytics

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 17.3MB 17.3MB +82.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
securitySolution 86.7KB 86.7KB +1.0B

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

cc @christineweng

hop-dev
hop-dev approved these changes Jul 23, 2024
View reviewed changes
Copy link
Contributor

@hop-dev hop-dev left a comment

Choose a reason for hiding this comment

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

🚀

@christineweng christineweng merged commit f6a837d into elastic:main Jul 23, 2024
40 checks passed
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jul 23, 2024
@christineweng
[Security solution][Alert Details] Enable preview feature flag and cy…
4d22224 
…press tests (elastic#188580)

## Summary

This PR inverted feature flag `entityAlertPreviewEnabled` to
`entityAlertPreviewDisabled` and related cypress tests. The flag
inversion will allow customers to disable the feature if issues arise.

### Checklist

- [x] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed

(cherry picked from commit f6a837d)
@kibanamachine kibanamachine mentioned this pull request Jul 23, 2024
Merged
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.15

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 Jul 24, 2024
@kibanamachine @christineweng
[8.15] [Security solution][Alert Details] Enable preview feature flag…
42ef1e6 
… and cypress tests (#188580) (#189005)

# Backport

This will backport the following commits from `main` to `8.15`:
- [[Security solution][Alert Details] Enable preview feature flag and
cypress tests (#188580)](#188580)

<!--- Backport version: 9.4.3 -->

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

<!--BACKPORT
[{"author":{"name":"christineweng","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-07-23T20:01:56Z","message":"[Security
solution][Alert Details] Enable preview feature flag and cypress tests
(#188580)\n\n## Summary\r\n\r\nThis PR inverted feature flag
`entityAlertPreviewEnabled` to\r\n`entityAlertPreviewDisabled` and
related cypress tests. The flag\r\ninversion will allow customers to
disable the feature if issues arise.\r\n\r\n\r\n### Checklist\r\n\r\n-
[x] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests
changed","sha":"f6a837dae0a6438b534e934edfd3232effe966b3","branchLabelMapping":{"^v8.16.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Threat
Hunting","release_note:feature","Team:Threat
Hunting:Investigations","v8.15.0","v8.16.0"],"title":"[Security
solution][Alert Details] Enable preview feature flag and cypress
tests","number":188580,"url":"https://github.com/elastic/kibana/pull/188580","mergeCommit":{"message":"[Security
solution][Alert Details] Enable preview feature flag and cypress tests
(#188580)\n\n## Summary\r\n\r\nThis PR inverted feature flag
`entityAlertPreviewEnabled` to\r\n`entityAlertPreviewDisabled` and
related cypress tests. The flag\r\ninversion will allow customers to
disable the feature if issues arise.\r\n\r\n\r\n### Checklist\r\n\r\n-
[x] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests
changed","sha":"f6a837dae0a6438b534e934edfd3232effe966b3"}},"sourceBranch":"main","suggestedTargetBranches":["8.15"],"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/188580","number":188580,"mergeCommit":{"message":"[Security
solution][Alert Details] Enable preview feature flag and cypress tests
(#188580)\n\n## Summary\r\n\r\nThis PR inverted feature flag
`entityAlertPreviewEnabled` to\r\n`entityAlertPreviewDisabled` and
related cypress tests. The flag\r\ninversion will allow customers to
disable the feature if issues arise.\r\n\r\n\r\n### Checklist\r\n\r\n-
[x] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests
changed","sha":"f6a837dae0a6438b534e934edfd3232effe966b3"}}]}]
BACKPORT-->

Co-authored-by: christineweng <[email protected]>
TinLe added a commit to TinLe/kibana that referenced this pull request Jul 30, 2024
@TinLe
Merge branch 'master' into dependabot/npm_and_yarn/express-4.19.2
df656a6 
* master: (3487 commits)
  `BedrockChat` & `GeminiChat` (elastic#186809)
  [ResponseOps] log error when ES Query rules find docs out of time range (elastic#186332)
  skip flaky suite (elastic#188997)
  [Security solution][Alert Details] Enable preview feature flag and cypress tests (elastic#188580)
  [EuiProviders] Warn Developer if EuiProvider is missing (elastic#184608)
  [Security Solution ] Fixes Timeline infinite loading bug (elastic#188943)
  Improve SearchSource SearchRequest type (elastic#186862)
  Deprecate Search Sessions config (elastic#188037)
  [Synthetics] Add missing monitorType and tag info in cards !! (elastic#188824)
  [Console Monaco] Resolve uncaught error from tokenizer (elastic#188746)
  [Data Forge] Add `service.logs` dataset as a  data stream (elastic#188786)
  [Console] Fix failing bulk requests (elastic#188552)
  Update dependency terser to ^5.31.2 (main) (elastic#188528)
  [APM][ECO] Telemetry (elastic#188627)
  [Fleet] Fix uninstall package validation accross space (elastic#188749)
  Update warning on `xpack.fleet.enableExperimental` (elastic#188917)
  [DOCS][Cases] Automate more screenshots for cases (elastic#188697)
  [Fleet] Fix get one agent when feature flag disabled (elastic#188953)
  chore(investigate): Add investigate-app plugin from poc (elastic#188122)
  [Monaco Editor] Add Search functionality (elastic#188337)
  ...
TinLe added a commit to TinLe/kibana that referenced this pull request Jul 30, 2024
@TinLe
Merge branch 'master' into dependabot/npm_and_yarn/pdfjs-dist-4.2.67
8bec40b 
* master: (2400 commits)
  `BedrockChat` & `GeminiChat` (elastic#186809)
  [ResponseOps] log error when ES Query rules find docs out of time range (elastic#186332)
  skip flaky suite (elastic#188997)
  [Security solution][Alert Details] Enable preview feature flag and cypress tests (elastic#188580)
  [EuiProviders] Warn Developer if EuiProvider is missing (elastic#184608)
  [Security Solution ] Fixes Timeline infinite loading bug (elastic#188943)
  Improve SearchSource SearchRequest type (elastic#186862)
  Deprecate Search Sessions config (elastic#188037)
  [Synthetics] Add missing monitorType and tag info in cards !! (elastic#188824)
  [Console Monaco] Resolve uncaught error from tokenizer (elastic#188746)
  [Data Forge] Add `service.logs` dataset as a  data stream (elastic#188786)
  [Console] Fix failing bulk requests (elastic#188552)
  Update dependency terser to ^5.31.2 (main) (elastic#188528)
  [APM][ECO] Telemetry (elastic#188627)
  [Fleet] Fix uninstall package validation accross space (elastic#188749)
  Update warning on `xpack.fleet.enableExperimental` (elastic#188917)
  [DOCS][Cases] Automate more screenshots for cases (elastic#188697)
  [Fleet] Fix get one agent when feature flag disabled (elastic#188953)
  chore(investigate): Add investigate-app plugin from poc (elastic#188122)
  [Monaco Editor] Add Search functionality (elastic#188337)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nastasha-solomon nastasha-solomon nastasha-solomon approved these changes

@PhilippeOberti PhilippeOberti PhilippeOberti approved these changes

@hop-dev hop-dev hop-dev approved these changes

@tiansivive tiansivive tiansivive approved these changes

Assignees

@christineweng christineweng

Labels
release_note:feature Makes this part of the condensed release notes Team:Threat Hunting:Investigations Security Solution Investigations Team Team:Threat Hunting Security Solution Threat Hunting Team v8.15.0 v8.16.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@christineweng @kibanamachine @elasticmachine @PhilippeOberti @kibana-ci @tiansivive @hop-dev @nastasha-solomon