-
Notifications
You must be signed in to change notification settings - Fork 3.8k
feat: return ReasonFailed from ingest-limits frontend #18123
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
Conversation
| logger: logger, | ||
| gatherer: gatherer, | ||
| assignedPartitionsCache: assignedPartitionsCache, | ||
| streams: promauto.With(reg).NewCounter( |
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.
Thanks to this I was able to move the metrics out of ring.go and into frontend.go.
|
|
||
| // ReasonMaxStreams is returned when a stream cannot be accepted because | ||
| // the tenant has either reached or exceeded their maximum stream limit. | ||
| ReasonMaxStreams |
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.
Renamed to remove the Exceeds.
| ReasonExceedsMaxStreams Reason = iota | ||
| // ReasonFailed is the reason returned when a stream cannot be checked | ||
| // against limits due to an error. | ||
| ReasonFailed Reason = iota + 1 |
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 should have been iota + 1 to start with, made a mistake here.
e750fdb to
3c63b26
Compare
| for _, resp := range resps { | ||
| results = append(results, resp.Results...) | ||
| // If the entire call failed, then all streams failed. | ||
| for _, stream := range req.Streams { |
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 is implemented as per https://github.com/grafana/deployment_tools/blob/master/docs/loki/limits.md#apis.
This commit updates the ingest-limits frontend to return streams with ReasonFailed if that stream could not be checked against per-tenant limits.
3c63b26 to
821730b
Compare
What this PR does / why we need it:
This commit updates the ingest-limits frontend to return streams with
ReasonFailedif that stream could not be checked against per-tenant limits.Stack on top of #18055.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Checklist
CONTRIBUTING.mdguide (required)featPRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.docs/sources/setup/upgrade/_index.mddeprecated-config.yamlanddeleted-config.yamlfiles respectively in thetools/deprecated-config-checkerdirectory. Example PR