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

[Request][ESS] Document advanced setting that allows users to disable ES|QL in ESS #5181

Merged
merged 16 commits into from
May 20, 2024

Conversation

nastasha-solomon
Copy link
Contributor

@nastasha-solomon nastasha-solomon commented May 7, 2024

Contributes to #5163 and #5149

Previews: A note about the enableESQL advanced setting was added to the following locations:

No Serverless PR was created. The advanced setting is only available in ESS right now.

@nastasha-solomon nastasha-solomon added API Feature: Rules Feature: Timeline Priority: High Issues that are time-sensitive and/or are of high customer importance Effort: Small Issues that can be resolved quickly v8.14.0 Feature: ES|QL labels May 7, 2024
@nastasha-solomon nastasha-solomon self-assigned this May 7, 2024
Copy link

github-actions bot commented May 7, 2024

A documentation preview will be available soon.

Request a new doc build by commenting
  • Rebuild this PR: run docs-build
  • Rebuild this PR and all Elastic docs: run docs-build rebuild

run docs-build is much faster than run docs-build rebuild. A rebuild should only be needed in rare situations.

If your PR continues to fail for an unknown reason, the doc build pipeline may be broken. Elastic employees can check the pipeline status here.

@nastasha-solomon nastasha-solomon requested a review from paulewing May 8, 2024 01:27
@nastasha-solomon nastasha-solomon marked this pull request as ready for review May 10, 2024 13:53
@nastasha-solomon nastasha-solomon requested a review from a team as a code owner May 10, 2024 13:53
Copy link
Contributor

@joepeeples joepeeples left a comment

Choose a reason for hiding this comment

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

The text itself LGTM, but I'm not sure why we need to remind users in all these places that they can turn off ES|QL. If a user goes to a section like "Create an ES|QL rule" or "Use ES|QL to investigate events," they're not trying to turn off the entire language — they want to use it. Maybe there's a different context where it'd be more natural to tell users how to disable ES|QL if they don't want to use it?

joepeeples
joepeeples previously approved these changes May 10, 2024
@nastasha-solomon
Copy link
Contributor Author

nastasha-solomon commented May 10, 2024

Looking at this again, I don't think we need to put this notice in the places where users are creating a new ES|QL rule from the UI. I agree that if they've landed on those instructions, they've already decided to create an ES|QL rule. But I do think it's important to keep the note in the places that provide background on the ES|QL features in Security to ensure it's visible in the places where users go to learn about the feature. Here's where I think it'd be appropriate:

I also think we can change the focus of the message to let users know that there's a single advanced setting that controls all ES|QL features in ESS. Here's what I'm thinking:

ES|QL features are turned on by default and are controlled by the enableESQL advanced setting.

@joepeeples and @paulewing let me know your thoughts on what I've laid out above.

@joepeeples
Copy link
Contributor

joepeeples commented May 10, 2024

Thanks @nastasha-solomon, here are some direct responses:

I do think it's important to keep the note in the places that provide background on the ES|QL features in Security to ensure it's visible in the places where users go to learn about the feature.

Definitely agree! Ideally there'd be a single location where we can document this, instead of having the same message repeated across several topics. But repeating might be unavoidable.

This section appears to have two duplicate notes about the same thing — or maybe it's two different things that, at first (and second) glance look like the same thing? Either way it's confusing.

image

Here's what I'm thinking:

ES|QL features are turned on by default and are controlled by the enableESQL advanced setting.

LGTM!

@nastasha-solomon
Copy link
Contributor Author

@joepeeples yeah, within the Security docs, there's no one single place to add the information about the advanced setting. There is this page over on the Elasticsearch side, but it's in the ES docset, so it's a bit out of the way for Security users. Plus, I feel like Using ES|QL page would be the better place to put the advanced setting information. I'll ping Liam about this next week to get his thoughts on adding the advanced setting information to the ES|QL docs.

RE the second note that you pointed out under the Timeline docs:
That note, and the one above it that says to not use ES|QL in production envs will be removed once I merge #5139.

@nastasha-solomon nastasha-solomon requested review from joepeeples and a team May 11, 2024 14:06
Copy link
Contributor

mergify bot commented May 13, 2024

This pull request is now in conflicts. Could you fix it @nastasha-solomon? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b issue-5163-esql-adv-set upstream/issue-5163-esql-adv-set
git merge upstream/main
git push upstream issue-5163-esql-adv-set

@nastasha-solomon
Copy link
Contributor Author

I pinged the ES|QL writer to check whether they're mentioning the advanced setting in the core ES|QL docs. Will report back here once I know more.

@nastasha-solomon nastasha-solomon merged commit 84344d6 into main May 20, 2024
3 checks passed
mergify bot pushed a commit that referenced this pull request May 20, 2024
… ES|QL in ESS (#5181)

* First draft

* Minor edits

* Updated note

* Removing outdated content

* Update docs/assistant/security-assistant.asciidoc

* Update docs/detections/about-rules.asciidoc

* Update docs/events/timeline-ui-overview.asciidoc

* Fixed formatting

(cherry picked from commit 84344d6)
@nastasha-solomon nastasha-solomon deleted the issue-5163-esql-adv-set branch May 20, 2024 18:02
nastasha-solomon added a commit that referenced this pull request May 20, 2024
…disable ES|QL in ESS (backport #5181) (#5234)

* First draft

* Minor edits

* Updated note

* Removing outdated content

* Update docs/assistant/security-assistant.asciidoc

* Update docs/detections/about-rules.asciidoc

* Update docs/events/timeline-ui-overview.asciidoc

* Fixed formatting

(cherry picked from commit 84344d6)

Co-authored-by: Nastasha Solomon <79124755+nastasha-solomon@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Effort: Small Issues that can be resolved quickly Feature: ES|QL Feature: Rules Feature: Timeline Priority: High Issues that are time-sensitive and/or are of high customer importance v8.14.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants