adds batch based metrics #2510
adds batch based metrics #2510
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2510 +/- ##
==========================================
+ Coverage 63.45% 63.56% +0.10%
==========================================
Files 163 163
Lines 14263 14305 +42
==========================================
+ Hits 9051 9093 +42
- Misses 4499 4500 +1
+ Partials 713 712 -1
|
pkg/storage/batch.go
Outdated
| filteredRefs: promauto.With(r).NewCounter(prometheus.CounterOpts{ | ||
| Namespace: "loki", | ||
| Subsystem: "store", | ||
| Name: "chunk_refs_filtered_downloaded_total", |
There was a problem hiding this comment.
Should we call this something like chunk_refs_post_filtering_total because the refs are downloaded already, we are just filtering them? Just a suggestion, would love to hear your thoughts on it.
There was a problem hiding this comment.
I think on this same line of thinking chunk_refs_pre_filtering and chunk_refs_post_filtering seem intuitive to me?
Also the "Subsystem" store would index make this easier to understand in the future?
pkg/storage/batch.go
Outdated
| batches: promauto.With(r).NewHistogram(prometheus.HistogramOpts{ | ||
| Namespace: "loki", | ||
| Subsystem: "store", | ||
| Name: "effective_batch_size", |
There was a problem hiding this comment.
Should we prefix the name with chunks or maybe call it chunks_per_batch_post_filtering ?
| @@ -510,7 +567,7 @@ func fetchChunkBySeries(ctx context.Context, chunks []*LazyChunk, matchers []*la | |||
|
|
|||
| // Now that we have the first chunk for each series loaded, | |||
| // we can proceed to filter the series that don't match. | |||
| chksBySeries = filterSeriesByMatchers(chksBySeries, matchers) | |||
| chksBySeries = filterSeriesByMatchers(chksBySeries, matchers, metrics) | |||
|
|
|||
| var allChunks []*LazyChunk | |||
| for _, series := range chksBySeries { | |||
There was a problem hiding this comment.
Wondering what a good way would be to also track how many series there were.
I'm not sure this is a fully vetted thought yet but part of me thinks we will want to know something about the number of series involved too.
|
Cool stuff ! |
This should help us prove which parts of the batching code deserve attention for improvement.