-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Metric type refactor #2905
Metric type refactor #2905
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2905 +/- ##
==========================================
- Coverage 76.88% 76.86% -0.03%
==========================================
Files 225 223 -2
Lines 16867 16939 +72
==========================================
+ Hits 12969 13020 +51
- Misses 3066 3078 +12
- Partials 832 841 +9
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
type ObservedMetric struct { | ||
*Metric | ||
Sink Sink | ||
Thresholds []Threshold |
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.
"max": sink.Max, | ||
"avg": sink.Avg, | ||
"med": sink.P(0.5), | ||
"p(90)": sink.P(0.90), |
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.
Good opportunity to fix #1253?
metrics/engine/ingester.go
Outdated
// sequential integer ID, we would be able to use a slice for these buckets | ||
// and eliminate the map loopkups altogether! | ||
|
||
samplesByMetric := make(map[*metrics.Metric][]metrics.Sample) |
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 inefficient, maybe we can avoid using output.SampleBuffer
and do this in a custom AddMetricSamples()
metrics/engine/engine.go
Outdated
metricsWithThresholds map[string]metrics.Thresholds | ||
trackedMetrics map[string]*trackedMetric |
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 need to investigate as a better and long-term solution the metric.SequenceID
option:
- We need to check if we can continue to keep in the long-term the sub-metrics as metrics.
908a243
to
b25d18d
Compare
b25d18d
to
db36955
Compare
This is outdated now. Better to re-work it and re-open after. |
No description provided.