-
Notifications
You must be signed in to change notification settings - Fork 288
more doc improvements #1952
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
more doc improvements #1952
Conversation
WalkthroughThis PR reorganizes playbook documentation: consolidates many playbook pages into a centralized playbook-reference index, updates Sphinx redirect mappings and cross-reference anchors, removes several legacy pages, adds a new Silencer Playbooks page, and adjusts multiple toctrees and minor wording across docs. Changes
Sequence Diagram(s)sequenceDiagram
participant Prom as Prometheus
participant Doc as Docs (site/navigation)
participant Rob as Robusta Playbook Engine
participant Sil as Silencer Action
participant Act as Action(s)
participant Sink as Sink (Slack/Teams/etc.)
Note over Doc: Documentation now describes silencer behavior\nand centralized playbook reference
Prom->>Rob: Alert event
alt Silencer condition matched
Rob->>Sil: Evaluate silencer
Sil-->>Rob: Silenced (stop)
note right of Rob `#dff0d8`: Processing halted — no actions or sinks
else No silencer match
Rob->>Act: Execute actions/enrichers
Act-->>Rob: Enriched event
Rob->>Sink: Deliver notification
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
docs/conf.py(1 hunks)docs/configuration/alertmanager-integration/grafana-self-hosted.rst(1 hunks)docs/index.rst(1 hunks)docs/playbook-reference/builtin-alert-enrichment.rst(1 hunks)docs/playbook-reference/defining-playbooks/builtin-playbooks.rst(1 hunks)docs/playbook-reference/defining-playbooks/creating-notifications.rst(1 hunks)docs/playbook-reference/defining-playbooks/playbook-advanced.rst(1 hunks)docs/playbook-reference/defining-playbooks/playbook-basics.rst(1 hunks)docs/playbook-reference/index.rst(1 hunks)docs/playbook-reference/overview.rst(1 hunks)docs/playbook-reference/prometheus-examples/index.rst(0 hunks)docs/playbook-reference/prometheus-examples/link-alert-enrichment.rst(1 hunks)docs/setup-robusta/installation-faq.rst(1 hunks)
💤 Files with no reviewable changes (1)
- docs/playbook-reference/prometheus-examples/index.rst
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: run_tests
- GitHub Check: run_tests
- GitHub Check: Deploy docs
- GitHub Check: Deploy docs
🔇 Additional comments (13)
docs/playbook-reference/overview.rst (1)
9-12: New Quick Start section improves navigability.The addition of a Quick Start section is well-placed and provides clear guidance for new users. The cross-reference to Playbook Basics is appropriate.
docs/playbook-reference/index.rst (1)
9-9: Navigation text updated to match heading rename.The toctree entry display text is updated from "Creating Notifications" to "Playbook Notifications" to align with the heading change in the target file. The reference path remains unchanged.
docs/playbook-reference/prometheus-examples/link-alert-enrichment.rst (1)
11-11: Introductory text usefully generalized.The update makes it clear that
KubeContainerCPURequestAlertis an example, and the guide applies to any alert. This improves accessibility of the documentation.docs/setup-robusta/installation-faq.rst (1)
67-67: No issues found with the install-all-in-one reference.The
install-all-in-oneanchor is properly defined indocs/setup-robusta/installation/all-in-one-installation.rst:3and is correctly referenced at line 67. The target is appropriate for the FAQ context—linking "Robusta's all-in-one package" to the all-in-one installation documentation when answering whether Robusta replaces monitoring tools.docs/configuration/alertmanager-integration/grafana-self-hosted.rst (1)
95-96: ****The underline is not shortened. Verification shows the heading is 45 characters and the underline is 46 characters, which correctly exceeds the minimum requirement for reStructuredText formatting. No issues exist with this code.
Likely an incorrect or invalid review comment.
docs/playbook-reference/defining-playbooks/creating-notifications.rst (1)
1-2: Backward compatibility verified via URL redirects.The old file path
configuration/defining-playbooks/creating-notifications.htmlhas a redirect indocs/conf.py(line 92) pointing to the new path, ensuring external links remain functional. The heading rename to "Playbook Notifications" is consistent and RST auto-generates the anchor correctly. No further action needed.docs/playbook-reference/defining-playbooks/playbook-basics.rst (1)
189-190: Cross-reference is valid.Verification confirms the anchor "Playbook Notifications" is properly set in
creating-notifications.rstwith correct reStructuredText heading format. The reference on line 189 ofplaybook-basics.rstcorrectly points to this anchor.docs/playbook-reference/defining-playbooks/builtin-playbooks.rst (1)
28-28: LGTM! Link properly updated to point to consolidated documentation.The link correctly references the new unified "Enrich Alerts" page, aligning with the documentation reorganization across this PR.
docs/index.rst (1)
147-147: LGTM! Navigation consolidation improves documentation structure.Merging the builtin and custom alert enrichment entries into a single "Enrich Alerts" page simplifies navigation and provides a better user experience.
docs/conf.py (1)
111-112: LGTM! Redirects properly maintain backward compatibility.Both old documentation paths now correctly redirect to the consolidated "Enrich Alerts" page, ensuring existing links continue to work after the reorganization.
docs/playbook-reference/builtin-alert-enrichment.rst (3)
3-11: LGTM! Excellent improvements to the introduction.The new heading "Enrich Alerts" better captures the page's comprehensive scope, and the introductory text is more engaging. The note about HolmesGPT is particularly valuable for discoverability of AI-powered enrichment features.
15-22: LGTM! Bullet points improve clarity.The restructured content with bullet points clearly communicates what gets enriched automatically, making it much easier to scan and understand the builtin capabilities.
45-60: Toctree references verified—no issues found.Both referenced files exist in the repository at the correct locations:
docs/playbook-reference/prometheus-examples/bash-alert-enrichment.rst✓docs/playbook-reference/prometheus-examples/link-alert-enrichment.rst✓The toctree references are valid and correctly formatted.
docs/playbook-reference/defining-playbooks/playbook-advanced.rst
Outdated
Show resolved
Hide resolved
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
docs/conf.py(5 hunks)docs/help.rst(1 hunks)docs/index.rst(1 hunks)docs/notification-routing/routing-silencing.rst(1 hunks)docs/playbook-reference/actions/develop-actions/index.rst(1 hunks)docs/playbook-reference/actions/develop-actions/loading-custom-actions.rst(1 hunks)docs/playbook-reference/actions/develop-actions/playbook-repositories.rst(1 hunks)docs/playbook-reference/actions/index.rst(1 hunks)docs/playbook-reference/builtin-alert-enrichment.rst(1 hunks)docs/playbook-reference/defining-playbooks/external-playbook-repositories.rst(1 hunks)docs/playbook-reference/defining-playbooks/index.rst(1 hunks)docs/playbook-reference/defining-playbooks/playbook-advanced.rst(0 hunks)docs/playbook-reference/defining-playbooks/playbook-basics.rst(0 hunks)docs/playbook-reference/defining-playbooks/silencer-playbooks.rst(1 hunks)docs/playbook-reference/defining-playbooks/trigger-action-binding.rst(0 hunks)docs/playbook-reference/index.rst(1 hunks)docs/playbook-reference/kubernetes-examples/kubernetes-change-notifications.rst(1 hunks)docs/playbook-reference/overview.rst(0 hunks)docs/playbook-reference/triggers/index.rst(1 hunks)docs/playbook-reference/triggers/kubernetes.rst(1 hunks)docs/setup-robusta/installation/_generate_config.jinja(1 hunks)docs/track-changes/kubernetes-changes.rst(1 hunks)
💤 Files with no reviewable changes (4)
- docs/playbook-reference/defining-playbooks/trigger-action-binding.rst
- docs/playbook-reference/overview.rst
- docs/playbook-reference/defining-playbooks/playbook-advanced.rst
- docs/playbook-reference/defining-playbooks/playbook-basics.rst
✅ Files skipped from review due to trivial changes (8)
- docs/playbook-reference/kubernetes-examples/kubernetes-change-notifications.rst
- docs/playbook-reference/actions/index.rst
- docs/playbook-reference/actions/develop-actions/playbook-repositories.rst
- docs/playbook-reference/actions/develop-actions/loading-custom-actions.rst
- docs/playbook-reference/triggers/kubernetes.rst
- docs/playbook-reference/defining-playbooks/external-playbook-repositories.rst
- docs/setup-robusta/installation/_generate_config.jinja
- docs/playbook-reference/triggers/index.rst
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/playbook-reference/builtin-alert-enrichment.rst
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: run_tests
- GitHub Check: run_tests
- GitHub Check: Deploy docs
🔇 Additional comments (10)
docs/help.rst (1)
3-4: Well-structured internal anchor placement.The addition of the
.. _Getting Support:cross-reference target before the heading follows proper reStructuredText conventions and enables other documentation pages to reference this section using Sphinx's:ref:role. The blank line after the anchor target follows standard RST formatting.docs/playbook-reference/actions/develop-actions/index.rst (1)
21-21: Path is correctly resolved to existing documentation file.The relative path
../../defining-playbooks/external-playbook-repositoriescorrectly resolves todocs/playbook-reference/defining-playbooks/external-playbook-repositories.rst, which exists. The toctree entry is properly formatted and positioned within the Develop Actions section.docs/notification-routing/routing-silencing.rst (1)
38-38: LGTM!The cross-reference to the new Silencer Playbooks documentation is correctly formatted and the target label exists in the new file.
docs/track-changes/kubernetes-changes.rst (1)
8-8: LGTM!The shortened label "K8s Change Notification" is consistent with the renaming pattern across the documentation.
docs/playbook-reference/defining-playbooks/silencer-playbooks.rst (1)
1-112: Excellent documentation for Silencer Playbooks!The new Silencer Playbooks documentation is well-structured with:
- Clear introduction explaining purpose and benefits
- Multiple practical examples with YAML code blocks
- Proper cross-references to detailed action documentation
- Important usage notes (e.g., silencers must be defined before other playbooks)
The content is comprehensive and should help users effectively implement silencing strategies.
docs/playbook-reference/index.rst (1)
1-351: Well-structured narrative documentation.The consolidated playbook documentation provides a comprehensive, narrative-driven guide that covers:
- Playbook anatomy and workflow
- Defining custom playbooks with practical examples
- Trigger and action concepts with hierarchies
- Advanced topics like filters, multiple playbooks, and global configuration
This consolidation improves the documentation by providing a cohesive learning path instead of scattered pages.
docs/conf.py (2)
113-114: LGTM on consolidation redirects.The redirects properly route old playbook documentation pages to the new consolidated index at
/master/playbook-reference/index.html. This maintains backward compatibility while supporting the documentation restructuring.Also applies to: 215-217, 220-220
95-99: Redirect anchor targets confirmed valid.Verification complete. Both anchor targets referenced in the redirects exist in
docs/playbook-reference/index.rst:
#matching-actions-to-triggers: Explicit anchor at line 188 ✓#using-filters-to-restrict-triggers: Section header at line 63 (auto-generates anchor) ✓docs/index.rst (1)
147-155: All toctree targets verified—documentation references are valid.All 8 new entries in the Advanced - Playbooks section reference existing documentation files. The reorganization is complete and safe to merge.
docs/playbook-reference/defining-playbooks/index.rst (1)
11-12: All removed toctree entries have corresponding redirects and content remains accessible.Verification confirms all four removed pages (playbook-basics, playbook-advanced, trigger-action-binding, external-playbook-repositories) have corresponding redirects in conf.py. Three pages are consolidated into index.rst with proper section anchors (#understanding-actions auto-generated, #matching-actions-to-triggers explicitly defined). The external-playbook-repositories page remains as a separate file while being properly redirected from old bookmark paths.
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.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
docs/configuration/alertmanager-integration/alert-custom-prometheus.rst(2 hunks)docs/configuration/holmesgpt/getting-started.rst(1 hunks)docs/configuration/resource-recommender.rst(1 hunks)docs/help.rst(2 hunks)docs/how-it-works/coverage.rst(2 hunks)docs/index.rst(1 hunks)docs/notification-routing/configuring-sinks.rst(1 hunks)docs/notification-routing/index.rst(1 hunks)docs/playbook-reference/kubernetes-examples/kubernetes-change-notifications.rst(1 hunks)docs/playbook-reference/triggers/kubernetes.rst(1 hunks)docs/track-changes/kubernetes-changes.rst(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- docs/playbook-reference/kubernetes-examples/kubernetes-change-notifications.rst
- docs/index.rst
- docs/track-changes/kubernetes-changes.rst
- docs/help.rst
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: run_tests
- GitHub Check: run_tests
- GitHub Check: Deploy docs
🔇 Additional comments (13)
docs/playbook-reference/triggers/kubernetes.rst (1)
10-12: Link label updated to reflect documentation naming convention.The link label has been updated from "Kubernetes Change Notifications" to "Change Tracking Playbooks" to align with broader documentation restructuring. The target path remains unchanged, suggesting Sphinx redirects or maintained path structure handles backward compatibility. This appears consistent with the PR's documentation improvements.
docs/configuration/alertmanager-integration/alert-custom-prometheus.rst (2)
13-14: Inconsistency between AI summary and actual code.The AI summary states that the introductory sentence about Robusta generating alerts by listening to the Kubernetes APIServer was removed, but this text still appears in the current file (lines 13–14). This suggests the summary may be inaccurate or incomplete.
11-11: Both cross-reference anchors are correctly defined and will resolve properly.The verification confirms:
Playbook Basicsanchor exists atdocs/playbook-reference/index.rst:2builtin-alert-enrichmentanchor exists atdocs/playbook-reference/builtin-alert-enrichment.rst:1The documentation restructuring has not introduced broken links. Both references at lines 11 and 111 in
alert-custom-prometheus.rstwill render correctly.docs/configuration/holmesgpt/getting-started.rst (4)
147-160: Approve: "Using Existing Secrets" section is clear and well-structured.The YAML example correctly demonstrates how to inject the
ROBUSTA_UI_TOKENviasecretKeyRef, and the guidance about using existing Kubernetes secrets is practical and helpful. The syntax and configuration structure align with standard Helm/Kubernetes practices.
162-178: Approve: "Common Issues" section provides valuable troubleshooting guidance.The section covers three common pain points with concise, actionable solutions. The advice about Azure token limits and links to external configuration resources are particularly helpful. The formatting and organization are clear and easy to scan.
145-146: Cross-reference anchor is correctly defined and referenced—no issues found.The anchor
.. _Reading the Robusta UI Token from a secret in HolmesGPT:is properly referenced indocs/help.rstusing the RST cross-reference syntax:ref:Using Existing Secrets `` with matching anchor name and context. Documentation formatting and structure are correct.
176-176: External documentation links verified as current and valid.The HolmesGPT data sources documentation at line 176 is available and accessible. The referenced URL structure and content alignment appear appropriate for the getting-started context.
Note: Line 184 could not be fully evaluated as the specific URL content was not provided in the code snippet.
docs/how-it-works/coverage.rst (1)
42-42: Verify Notification Routing documentation path matches resolved reference.Line 42 updates the reference to point to
:doc:Notification Routing </notification-routing/index>`. Ensure this path correctly resolves to the Notification Routing index page and that the link text accurately describes the target content.docs/notification-routing/index.rst (1)
71-71: The PagerDuty sink link fix is correct; no issues remain.The change corrects a broken documentation link. The PagerDuty.rst file exists on disk with PascalCase naming, so the updated link path (
../configuration/sinks/PagerDuty) is accurate. The previous lowercase link would have resulted in a broken reference on case-sensitive file systems. The naming inconsistency across other sink files (some use PascalCase likeDataDog.rst, others use lowercase likeslack.rst) is pre-existing in the repository and unrelated to this change.docs/notification-routing/configuring-sinks.rst (1)
12-12: ****The cross-references in configuring-sinks.rst are valid and do not require explicit anchor definitions. In reStructuredText/Sphinx, section headings automatically create implicit reference targets. The heading "MS Teams" in ms-teams.rst and "Slack" in slack.rst are automatically referenceable as
:ref:MS Teamsand `:ref:`Slackrespectively, without needing explicit.. _MS Teams:or.. _Slack:anchor definitions.Likely an incorrect or invalid review comment.
docs/configuration/resource-recommender.rst (3)
9-10: Content improvements align with PR objectives.The refinements to the bullet points improve clarity: line 9 now properly reflects that reports can be sent to "Slack or other sinks," and line 10 simplifies the wording while maintaining context clarity within the KRR section.
13-14: Heading formatting is correct.The section heading underline properly follows reStructuredText conventions and aligns with surrounding document structure.
4-5: Introduction revision improves technical clarity.The rewritten introduction better specifies KRR as a CLI tool and clearly describes its data source (Prometheus) and outputs (resource recommendations). This aligns with the PR goal of documentation improvements.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
No description provided.