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

[Docs] Update "Alert and action settings" docs to be generated from YAML source #191787

Open
wants to merge 50 commits into
base: main
Choose a base branch
from

Conversation

kilfoyle
Copy link
Contributor

@kilfoyle kilfoyle commented Aug 29, 2024

This PR:

  • Updates the Kibana Alert and action settings page to be based off of a YAML source file (/docs/settings-gen/source/kibana-alert-action-settings.yml) that is manually converted to Asciidoc format (kibana-alert-action-settings.asciidoc) by means of a Perl script (docs/settings-gen/parse-settings.pl). A preview of the new, generated page is here.
  • Adds the docs/settings-gen/parse-settings.pl script which does the YAML → Asciidoc conversion.

All new files are added to the /docs/source-gen folder.

This is a trial run updating only one page of settings in the docs. Later, in separate PRs, we plan to convert other pages. After all Kibana settings pages have been converted, we would ask that the Perl script be run automatically as part of the CI whenever the YAML files in /docs/source-gen are added or updated.

Notes:

  • The Docs team is happy to own and maintain the Perl script (sorry to use Perl - it's the only scripting language that I know).
  • In time we also plan to convert all of these files from Asciidoc to Markdown.
  • When we eventually/hopefully get the rest of the Kibana settings files converted, we will announce the settings doc process to the Kibana team by email and/or in the Kibana newsletter.

Big thanks to the amazing @lukeelmers and @KOTungseth for guiding this!


Why are we doing this? We aim to:

  • Create a more consistent appearance for settings across all of the docs.
  • Make it easier for people to contribute, since all Asciidoc/Markdown formatting is handled by a script.
  • Make it more apparent which settings may be missing info, such as the default values, available options, etc.

P.S. I haven't worked in the Kibana repo very much and would appreciate any help navigating the CI checks.

Rel: https://github.com/elastic/docs-projects/issues/239

Copy link
Contributor

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.

@obltmachine
Copy link

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

@kilfoyle kilfoyle removed the request for review from bmorelli25 August 30, 2024 17:28
@kilfoyle
Copy link
Contributor Author

kilfoyle commented Nov 4, 2024

@elastic/kibana-operations, @kobelb Apologies for the ping but I'd really appreciate if someone can help with this PR and Luke's questions above.

@jbudz
Copy link
Member

jbudz commented Nov 8, 2024

Most important thing to review: are we generally comfortable with the yml schema design here? Because this will dictate the authoring experience for yml config docs in the future

Seems fine, it covers all the needs of existing settings and we'll be in a position to adjust if needed

Is the current directory the right location for the conversion script or does it belong in a separate package? (Currently the script relies on living in the directory with the files it is running against).

We don't run CI on changes only in the docs folder but if validation is left to Kibana CI we'll want to make an adjustment to https://github.com/elastic/kibana/blob/main/.buildkite/pull_requests.json#L24.

If it remains static I don't see issues. If not, it could be difficult to maintain across all repositories with docs and all branches. It does need to be accessible locally by contributors in some form and this is easiest in that regard.

Is the current directory the right location for the generated docs? Due to the way docs are released, we need the generated docs to be committed to the repo

👍

What would the level of effort be to, as a follow-up task, automate the running of this script in CI and auto-commiting the output on PRs?

Low, except we shut off Kibana CI on old branches. We would have to a create a docs only check, similar to
buildkite/docs-build-pr. Preferably the check could be baked into that one.

Copy link
Contributor

@kobelb kobelb left a comment

Choose a reason for hiding this comment

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

Two small corrections.

The following is just my opinion, and can be ignored:

  1. Having the example in a separate file is cumbersome. Is it possible for these to be inline and use the multi-line YAML strings?
  2. Rendering each setting in a collapsible section made consuming these docs hard. However, I'm quite adjusted to the old format, so take this with a grain of salt.

@kilfoyle
Copy link
Contributor Author

kilfoyle commented Nov 21, 2024

@kobelb, thanks very much for the excellent suggestions!

Two small corrections.

Both are fixed.

Having the example in a separate file is cumbersome. Is it possible for these to be inline and use the multi-line YAML strings?

Indeed, it's a lot less cumbersome having the examples included in the YAML rather than in a separate file, so I've made that change.

Rendering each setting in a collapsible section made consuming these docs hard. However, I'm quite adjusted to the old format, so take this with a grain of salt.

I was thinking that the collapsible sections looked tidier, but on further thought I agree that the usability is much better without them. I've removed them.

If you have other suggestions to improve the formatting I'm happy to add them in. Preview page is here.

@kilfoyle
Copy link
Contributor Author

kilfoyle commented Dec 4, 2024

@jbudz and @lukeelmers thanks a lot for your feedback on this PR!

I've addressed the suggestions so far. Are there any changes you'd like or do you think this is ready to merge?

@lukeelmers
Copy link
Member

No concerns on my end as long as we get a ✅ from @jbudz or someone on the ops team.

@kilfoyle
Copy link
Contributor Author

kilfoyle commented Dec 17, 2024

Based on feedback I've made a few changes:

  • Each setting description is now a whitespace-preserving multiline string, rather than an array of strings (it's much cleaner this way).
  • Each setting now has a required "datatype" field that can be one of string/bool/int/float/enum. When enum is selected, the list of supported options should be defined using the options keys.
  • Tidied up the readme.

@kilfoyle
Copy link
Contributor Author

kilfoyle commented Dec 18, 2024

There's an error to do with the codeowners file that I'm not sure how to fix:

Unknown owner on line 3325: make sure the team @elastic/obs-ux-onboarding-team exists, is publicly visible, and has write access to the repository

That @elastic/obs-ux-onboarding-team team is also included in the codeowners file on main. So maybe the error can be ignored?

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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:Docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants