Skip to content

Usage group support for ingestion limits #3914

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

Merged
merged 5 commits into from
Feb 13, 2025
Merged

Conversation

aleks-p
Copy link
Contributor

@aleks-p aleks-p commented Feb 13, 2025

Continuation of #3879, adding support for reacting to externally configured ingestion limits for usage groups. The config model is extended from before:

ingestion_limit:
  period_type: hour
  period_limit_mb: 2048
  limit_reached: true
  next_limit_reset: 1738274400
  sampling:
    num_requests: 1
    period: 2m
  # new field
  usage_groups:
    group-1:
      limit_reached: true
      period_limit_mb: 1024
    group-2:
      limit_reached: false
      period_limit_mb: 128

When the limit is reached, we add the usage group to the error response:

resource_exhausted: limit of 1.0 GiB/hour reached for usage group group-1, next reset at 2025-02-13T19:00:00Z

Usage groups use the global (tenant-level) period type and sampling config for simplicity, this might change in the future if needed.

@aleks-p aleks-p requested a review from a team as a code owner February 13, 2025 19:13
Copy link
Contributor

@bryanhuhta bryanhuhta left a comment

Choose a reason for hiding this comment

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

LGTM!

@aleks-p aleks-p force-pushed the ingestion-limits-usage-groups branch from 91553fd to ed98441 Compare February 13, 2025 19:33
Comment on lines +183 to +195
matched := false
for _, lbl := range lbls {
if lbl.Name == m.Name && !m.Matches(lbl.Value) {
return false
if lbl.Name == m.Name {
if !m.Matches(lbl.Value) {
return false
}
matched = true
break
}
}
if !matched {
return false
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unrelated to this PR, but when the request and usage group labels are disjoint, we report this as a match. This change makes sure we have a match on all usage group labels.

Copy link
Contributor

@bryanhuhta bryanhuhta left a comment

Choose a reason for hiding this comment

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

approve2

@aleks-p aleks-p merged commit a4297db into main Feb 13, 2025
21 checks passed
@aleks-p aleks-p deleted the ingestion-limits-usage-groups branch February 13, 2025 19:52
@aleks-p aleks-p mentioned this pull request Feb 27, 2025
3 tasks
shelldandy pushed a commit to shelldandy/pyroscope that referenced this pull request Mar 14, 2025
* Apply ingestion limits to usage groups

* Check actual usage groups in the request

* Make test more robust

* Fix case where usage groups can match against a disjoint set of request labels

* Reduce nesting in checkUsageGroupsIngestLimit
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.

2 participants