-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
feat: aggregate metrics back into Loki #13621
Conversation
* ignore aggregated metric streams from limits
pkg/detection/detection.go
Outdated
) | ||
|
||
const ( | ||
AggregatedMetricLabel = "__aggregated_metric__" |
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.
I'm open to other names. Maybe __rollup__
? or __service_rollup__
in case we want to add other rollups in the future?
pkg/pattern/metric/push.go
Outdated
case <-p.quit: | ||
cancel() | ||
return | ||
// TODO(twhitney): could buffer these into fewer pushes? |
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.
We can't push line by line !
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.
agreed, we need to buffer these and push once every 10s. though it's not currently line by line, but it is 1 per stream every 10s.
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.
we also need to increment a single int64
for each push, rather than storing all the raw samples. I can make both those changes here, or follow up in a subsequent PR?
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.
another option would be to use something like a buffered channel and call the API when the channel is full?
closing in favor of #13731 |
What this PR does / why we need it:
This feature persists aggregated metrics back into Loki as a special stream that can be queried from Explore Logs
Which issue(s) this PR fixes:
Fixes #13110