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

chore: make mixin range interval configurable #13925

Conversation

jmichalek132
Copy link
Contributor

What this PR does / why we need it:

Address last recording rule which has a hardcoded range interval which is too small for scrape interval of 1m by making it configurable using existing variables.

Which issue(s) this PR fixes:
Fixes #13630

Special notes for your reviewer:

Checklist

  • Reviewed the CONTRIBUTING.md guide (required)
  • Documentation added
  • Tests updated
  • Title matches the required conventional commits format, see here
    • Note that Promtail is considered to be feature complete, and future development for logs collection will be in Grafana Alloy. As such, feat PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.
  • Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
  • For Helm chart changes bump the Helm chart version in production/helm/loki/Chart.yaml and update production/helm/loki/CHANGELOG.md and production/helm/loki/README.md. Example PR
  • If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

@jmichalek132 jmichalek132 requested a review from a team as a code owner August 19, 2024 14:49
@CLAassistant
Copy link

CLAassistant commented Aug 19, 2024

CLA assistant check
All committers have signed the CLA.

@pull-request-size pull-request-size bot added size/M and removed size/S labels Aug 19, 2024
@jmichalek132 jmichalek132 force-pushed the jm-chore-make-mixin-range-interval-configurable branch from c033116 to bb4dfd4 Compare August 19, 2024 14:59
@pull-request-size pull-request-size bot added size/L and removed size/M labels Aug 19, 2024
@cstyan
Copy link
Contributor

cstyan commented Aug 19, 2024

hey @jmichalek132 thanks for this PR, but unfortunately right now this won't work as expected because we're still on an older version of the mixin-utils library that the histogram function is pulled from. The version we're on doesn't take in any parameter for the interval.

Can you run jb update and then generate the dashboards again?

@jmichalek132
Copy link
Contributor Author

hey @jmichalek132 thanks for this PR, but unfortunately right now this won't work as expected because we're still on an older version of the mixin-utils library that the histogram function is pulled from. The version we're on doesn't take in any parameter for the interval.

Can you run jb update and then generate the dashboards again?

Yeah, sorry my bad I lost that change as I switched devices to work on the PR. Fixed now.

@cstyan
Copy link
Contributor

cstyan commented Aug 20, 2024

@jmichalek132 thanks!

@cstyan cstyan merged commit 6284ed5 into grafana:main Aug 20, 2024
62 checks passed
@jmichalek132 jmichalek132 deleted the jm-chore-make-mixin-range-interval-configurable branch August 21, 2024 07:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Loki mixin: alert and recording rules make range interval configurable
3 participants