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

[Serverless] Disable Search Sessions #158356

Merged

Conversation

ElenaStoeva
Copy link
Contributor

Partially addresses #157756

Summary

This PR disables the Search Sessions plugin for serverless.

How to test:

  1. Start Elasticsearch with yarn es snapshot and Kibana with yarn serverless-{mode} where {mode} can be es, security, or oblt.
  2. Verify that the Search Sessions app is not accessible and its path (app/management/kibana/search_sessions) leads to the Stack Management landing page.

@ElenaStoeva ElenaStoeva added Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more release_note:skip Skip the PR/issue when compiling release notes labels May 24, 2023
@ElenaStoeva ElenaStoeva requested a review from a team May 24, 2023 09:59
@ElenaStoeva ElenaStoeva self-assigned this May 24, 2023
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
securitySolution 399 403 +4
total +6

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
securitySolution 479 483 +4
total +6

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

cc @ElenaStoeva

@ElenaStoeva ElenaStoeva marked this pull request as ready for review May 24, 2023 11:04
@elasticmachine
Copy link
Contributor

Pinging @elastic/platform-deployment-management (Team:Deployment Management)

@kertal
Copy link
Member

kertal commented May 26, 2023

@davismcphee one thing to check, I think we're using searchSessions in the unified histogram layout:

const searchSessionId = useObservable(stateContainer.searchSessionManager.searchSessionId$);

so the question if this still works, but anyway, it might be better to search for an alternative since I don't think we need the searchSessionManager service anywhere else in the code

@davismcphee
Copy link
Contributor

@davismcphee one thing to check, I think we're using searchSessions in the unified histogram layout:

const searchSessionId = useObservable(stateContainer.searchSessionManager.searchSessionId$);

so the question if this still works, but anyway, it might be better to search for an alternative since I don't think we need the searchSessionManager service anywhere else in the code

@kertal The main thing we're using it for here is to determine if the first search has run yet when navigating to Discover. If we can find another way to check for this, I think we can likely remove searchSessionId from this check:

if (!searchSessionId && !isPlainRecord) {
return null;
}

Otherwise we may have to check if the search session plugin is enabled and add that to the check as well.

Although in either case we'll still need to be able to retrieve the searchSessionId (if one exists) to pass down to Unified Histogram.

Copy link
Contributor

@davismcphee davismcphee left a comment

Choose a reason for hiding this comment

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

Pulled and tested locally, and I was unable to access app/management/kibana/search_sessions as expected. I also did some testing in Discover and everything seems to working fine there as well. LGTM 👍

@kertal It looks like this change just disables the use of search sessions in searches, but the search session service is still available, and it still returns IDs so it doesn't seem to have any impact on Discover from what I can tell.

@kertal
Copy link
Member

kertal commented May 30, 2023

Thx for checking!

@ElenaStoeva
Copy link
Contributor Author

Thanks a lot for the review @davismcphee and @kertal!

@ElenaStoeva ElenaStoeva merged commit dd4e88e into elastic:main May 30, 2023
@ElenaStoeva ElenaStoeva deleted the serverless/disable-search-sessions branch May 30, 2023 09:56
@kibanamachine kibanamachine added v8.9.0 backport:skip This commit does not require backporting labels May 30, 2023
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 release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v8.9.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants