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.

Sign up for GitHub

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

Jump to bottom

Docs: Basic documentation for deploying the ruler using jsonnet. #2127

Merged
merged 9 commits into from
Jun 24, 2022
Merged

Docs: Basic documentation for deploying the ruler using jsonnet. #2127

merged 9 commits into from
Jun 24, 2022

Conversation

stevesg
Copy link
Contributor

@stevesg stevesg commented Jun 17, 2022 •
edited
Loading

Documentation deploying the ruler using jsonnet and the new configuration options introduced in #2073

Contributes to #1369

@stevesg stevesg added the type/docs Improvements or additions to documentation label Jun 17, 2022
pracucci
pracucci approved these changes Jun 17, 2022
View reviewed changes
Copy link
Collaborator

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

Very nice! Few comments, nothing critical.

docs/sources/operators-guide/deploying-grafana-mimir/jsonnet/configuring.md Outdated Show resolved Hide resolved
docs/sources/operators-guide/deploying-grafana-mimir/jsonnet/configuring.md Outdated Show resolved Hide resolved
docs/sources/operators-guide/deploying-grafana-mimir/jsonnet/configuring.md Outdated Show resolved Hide resolved
docs/sources/operators-guide/deploying-grafana-mimir/jsonnet/configuring.md Outdated Show resolved Hide resolved
osg-grafana
osg-grafana suggested changes Jun 22, 2022
View reviewed changes
Copy link
Contributor

@osg-grafana osg-grafana left a comment

Choose a reason for hiding this comment

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

I think there is a broken link in here, in addition to my feedback. I want to test this link before this gets committed.

@stevesg
Copy link
Contributor Author

stevesg commented Jun 22, 2022 •
edited
Loading

I presume you are talking about the ruler_storage link. This links works fine when I test locally. I can also see other references in the locally generated HTML:

<li><a href=[#ruler_storage](view-source:http://localhost:3002/docs/mimir/next/operators-guide/configuring/reference-configuration-parameters/#ruler_storage)>ruler_storage</a></li>

@osg-grafana Can you be more precise with your concern?

@stevesg @osg-grafana
Apply suggestions from code review
07b488a 
Co-authored-by: Ursula Kallio <ursula.kallio@grafana.com>
@stevesg
Copy link
Contributor Author

stevesg commented Jun 22, 2022

I see - I don't think the link checker is correctly picking up that links exist to sections of the configuration reference.

e.g. This works fine: https://grafana.com/docs/mimir/latest/operators-guide/configuring/reference-configuration-parameters/#ruler_storage

I can remove the reference if this is problematic to unblock this change.

@pracucci
Copy link
Collaborator

I see - I don't think the link checker is correctly picking up that links exist to sections of the configuration reference.

Let me take a quick look.

pracucci
pracucci reviewed Jun 22, 2022
View reviewed changes
Copy link
Collaborator

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

@stevesg I left a couple of comments which should fix the doc validation. Could you give it a try, please?

docs/sources/operators-guide/deploying-grafana-mimir/jsonnet/configuring-ruler.md Outdated Show resolved Hide resolved
docs/sources/operators-guide/deploying-grafana-mimir/jsonnet/configuring-ruler.md Outdated Show resolved Hide resolved
pracucci
pracucci approved these changes Jun 22, 2022
View reviewed changes
Copy link
Collaborator

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

👏

docs/sources/operators-guide/deploying-grafana-mimir/jsonnet/configuring-ruler.md Outdated Show resolved Hide resolved
docs/sources/operators-guide/deploying-grafana-mimir/jsonnet/configuring-ruler.md Outdated Show resolved Hide resolved
docs/sources/operators-guide/deploying-grafana-mimir/jsonnet/configuring-ruler.md Show resolved Hide resolved
pracucci
pracucci approved these changes Jun 24, 2022
View reviewed changes
Copy link
Collaborator

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

Great job!

docs/sources/operators-guide/deploying-grafana-mimir/jsonnet/configuring-ruler.md Outdated Show resolved Hide resolved
docs/sources/operators-guide/deploying-grafana-mimir/jsonnet/configuring-ruler.md Outdated Show resolved Hide resolved
osg-grafana
osg-grafana approved these changes Jun 24, 2022
View reviewed changes
Copy link
Contributor

@osg-grafana osg-grafana left a comment

Choose a reason for hiding this comment

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

Excellent work! :)

@pracucci
Copy link
Collaborator

Merging to make sure it gets included in Mimir 2.2.0.

@pracucci pracucci merged commit b85d722 into main Jun 24, 2022
@pracucci pracucci deleted the remote-ruler-jsonnet-docs branch June 24, 2022 16:31
rlex added a commit to rlex/mimir that referenced this pull request Jun 28, 2022
@rlex
Merge branch 'main' into helm_prometheusrule
208d9c8 
* main: (63 commits)
  Add new section on website for links to blog posts, podcasts and talks. (grafana#2216)
  Rename codified errors to errors catalog (grafana#2256)
  Helm: add a step to contributing doc (grafana#2257)
  Signal that 2.2 release is now in progress. (grafana#2254)
  Removed migration of alertmanager local state files from old hierarchy (Cortex 1.8 and earlier) (grafana#2253)
  operations/mimir: Change multi_zone_ingester_max_unavailable to 25 (grafana#2251)
  Helm: weekly release (grafana#2252)
  Jsonnet: Configure ingester max global metadata per user and per metric (grafana#2250)
  Helm: metamonitor naming (grafana#2236)
  Mimir documentation about out-of-order (grafana#2183)
  Vendor latest mimir-prometheus/main (grafana#2243)
  Set CODEOWNERS to primary technical writer (grafana#2242)
  Use BasicLifecycler for distributors and auto-forget (grafana#2154)
  Docs: Basic documentation for deploying the ruler using jsonnet. (grafana#2127)
  Fix post merge reviews on 2187 (grafana#2230)
  Add tests for user metadata in the ingester (grafana#2184)
  Change the error message template for per-tenant limits (grafana#2234)
  helm: meta-monitoring (grafana#2068)
  Article about migrating from Consul to memberlist. Added documentation for /memberlist endpoint. (grafana#2166)
  Update runbooks to mention possibility to investigate memberlist KV store in various alerts (grafana#2158)
  ...
masonmei pushed a commit to udmire/mimir that referenced this pull request Jul 11, 2022
@stevesg @osg-grafana
Docs: Basic documentation for deploying the ruler using jsonnet. (gra…
ed69f1a 
…fana#2127)

* Docs: Basic documentation for deploying the ruler using jsonnet.

* Fix doc links.

* Move to new file, fix indents, minor rewording.

* Make checks on first migration step more explicit.

* Review comments: Remove redundant text. Add link to configuration parameters.

* Apply suggestions from code review

Co-authored-by: Ursula Kallio <ursula.kallio@grafana.com>

* Review comments.

* Fix doc link filename

* Review comments

Co-authored-by: Ursula Kallio <ursula.kallio@grafana.com>
@GrafanaWriter GrafanaWriter mentioned this pull request Jul 13, 2023
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pracucci pracucci pracucci approved these changes

@osg-grafana osg-grafana osg-grafana approved these changes

Assignees
No one assigned
Labels
type/docs Improvements or additions to documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@stevesg @pracucci @osg-grafana