-
Notifications
You must be signed in to change notification settings - Fork 167
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
Adding SLOs troubleshooting document (attempt 2) #4486
Conversation
A documentation preview will be available soon. Request a new doc build by commenting
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. |
@dedemorton : This is the PR to focus for your review, I have closed the other one as it got corrupted after the major changes we have done with serverless docs :) |
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.
This PR adds a nice level of technical depth that's often missing from the docs we write, so kudos to that! I've made some editorial suggestions (hope that's what you were looking for) plus pointed out some things I thought might be confusing to users. I Hope this is helpful.
Co-authored-by: DeDe Morton <[email protected]>
Co-authored-by: DeDe Morton <[email protected]>
Co-authored-by: DeDe Morton <[email protected]>
Co-authored-by: DeDe Morton <[email protected]>
Co-authored-by: DeDe Morton <[email protected]>
Co-authored-by: DeDe Morton <[email protected]>
@lucabelluccini : would you please comment out on the current status of the PR and the remaining topics? |
General disclaimers:
For what it's worth, this is the dependency graph of the assets I've deduced from the behavior of SLO Source here Suggestions: Let's add a WARNING/INFO banner at the topWe should add a top banner in
|
Awesome @lucabelluccini ! I'll work on a new version of the PR and as soon as it looks ok to you we will have to find a About the name of the detailed section, what about |
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.
Co-authored-by: DeDe Morton <[email protected]>
Co-authored-by: DeDe Morton <[email protected]>
Co-authored-by: DeDe Morton <[email protected]>
Co-authored-by: DeDe Morton <[email protected]>
Co-authored-by: DeDe Morton <[email protected]>
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 think we're 99.9% done. I've left few comments.
* https://github.com/elastic/kibana/blob/9cb830fe9a021cda1d091effbe3e0cd300220969/x-pack/plugins/observability/docs/openapi/slo/bundled.yaml#L453-L514[SLO Definitions Find API] (`/api/observability/slos/_definitions`) | ||
* https://github.com/elastic/kibana/blob/9cb830fe9a021cda1d091effbe3e0cd300220969/x-pack/plugins/observability/docs/openapi/slo/bundled.yaml#L368-L410[SLO Reset API] (`/api/observability/slos/${id}/_reset`) |
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.
Is there a way to point to the new Kibana SLO API docs instead of the openapi souce link?
https://www.elastic.co/docs/api/doc/kibana/v8/operation/operation-findslosop
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'll take a lookm thanks!.
That content wasn't really mine, we moved it from the mail SLO page, but I'll check the links as now we have added a link to the SLO API docs at the end of this doc.
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.
@lucabelluccini , this is interesting! I have updated (fixed) the second link (SLO RESET) because that one exist in our API docs: https://www.elastic.co/docs/api/doc/kibana/v8/operation/operation-resetsloop
But in the first one we have two problems:
- First of all there's a bug in the text and the API is not
api/observability/slos/_definition
, butinternal/observability/slos/_definitions
(based on the link). - Secondly that API call is not in the public API docs.
- Thirdly (:-D) that link doesn't work for the
main
branch, so it all looks weird.
What would you suggest? should we check with devs?
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.
@kdelemme : would you be able to share anything on this topic? This isn't really related with the goal of this PR, but it's something that we have found out.
Notes for PM / Dev review: This PR has been created as a result of @lucabelluccini request #4237 We have created a SLO Troubleshooting doc where we add details about SLO implementation that should help users to understand SLOs better and help with troubleshooting. We would like your review on this, and to decide the level and amount of information that we want to disclose on this subject. Besides the overall review of the content of the new file we also need to clarify the provided link in
Is it ok to provide such a link in that section? Also the path |
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.
Just a comment about updates. But otherwise I think this document very useful! Thanks for making this.
Co-authored-by: Kevin Delemme <[email protected]>
@lucabelluccini, and @kdelemme : one final question. Anyway I will add it in a port-PR. Also feel free to approve this as soon as you believe it's ready ;) |
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.
LGTM. Nice work!!
* slo troubleshooting doc added (cherry picked from commit 64b89f9)
* slo troubleshooting doc added (cherry picked from commit 64b89f9)
* slo troubleshooting doc added (cherry picked from commit 64b89f9)
* slo troubleshooting doc added (cherry picked from commit 64b89f9) Co-authored-by: Edu González de la Herrán <[email protected]>
* slo troubleshooting doc added (cherry picked from commit 64b89f9) Co-authored-by: Edu González de la Herrán <[email protected]>
* slo troubleshooting doc added (cherry picked from commit 64b89f9) Co-authored-by: Edu González de la Herrán <[email protected]>
Description
New document Troubleshoot SLOs created, per #4237 request.
SLO pre-requisites updated in all pages
SLO Beta instructions moved to troubleshooting doc
Preview:
Before moving forward I'd like @lucabelluccini to take a look at the current structure and decide if all suggested parts are useful.
After agreeing on main structure and areas to talk about we can discuss further the exact content.
Pending actions
Documentation sets edited in this PR
Check all that apply.
docs/en/observability/*
)docs/en/serverless/*
)docs/en/integrations/*
)Related issue
Closes #4237
Checklist
Follow-up tasks
Select one.