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

Enable -ingester.error-sample-rate by default and promote from experimental to advanced #7807

Merged
merged 3 commits into from
Apr 5, 2024

Conversation

pracucci
Copy link
Collaborator

@pracucci pracucci commented Apr 4, 2024

What this PR does

At Grafana Labs, we're running Mimir with -ingester.error-sample-rate=10 since several months and we're happy with it. I think it's a reasonable default for Mimir, so I suggest to enable the ingester error sampling by default and promote the feature from experimental to advanced.

Which issue(s) this PR fixes or relates to

N/A

Checklist

  • Tests updated.
  • Documentation added.
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
  • about-versioning.md updated with experimental features.

@pracucci pracucci marked this pull request as ready for review April 4, 2024 15:02
@pracucci pracucci requested review from jdbaldry and a team as code owners April 4, 2024 15:02
Copy link
Member

@jdbaldry jdbaldry left a comment

Choose a reason for hiding this comment

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

Do we talk about logging behavior anywhere in the docs that we should consider including this information in? Does it justify formal documentation beyond the reference material?

…mental to advanced

Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
@pracucci pracucci force-pushed the enable-errors-sampling-by-default branch from d4593f7 to a8ad4b5 Compare April 5, 2024 07:04
@pracucci
Copy link
Collaborator Author

pracucci commented Apr 5, 2024

Do we talk about logging behavior anywhere in the docs that we should consider including this information in? Does it justify formal documentation beyond the reference material?

@jdbaldry You raised a good point! I think we should document it in our runbooks. Please take a look at the changes I did.

Note: I also grepped "log" in the rest of the doc and I haven't find any other place where we explicitly mention logs like we do in runbooks.

Copy link
Member

@jdbaldry jdbaldry left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for thinking about where this might need to be documented, I think the extra documentation is a useful reminder in the runbooks :)

@pracucci pracucci enabled auto-merge (squash) April 5, 2024 07:33
@pracucci pracucci merged commit 47ddfd5 into main Apr 5, 2024
29 checks passed
@pracucci pracucci deleted the enable-errors-sampling-by-default branch April 5, 2024 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants