chore(querier): Refactor the store and querier interface. #15969
Conversation
| @@ -1,4 +1,4 @@ | |||
| package limists | |||
| package limits | |||
| SelectSamples(ctx context.Context, req logql.SelectSampleParams) (iter.SampleIterator, error) | ||
| SelectLogs(ctx context.Context, req logql.SelectLogParams) (iter.EntryIterator, error) | ||
| SelectSeries(ctx context.Context, req logql.SelectLogParams) ([]logproto.SeriesIdentifier, error) | ||
| LabelValuesForMetricName(ctx context.Context, userID string, from, through model.Time, metricName string, labelName string, matchers ...*labels.Matcher) ([]string, error) | ||
| LabelNamesForMetricName(ctx context.Context, userID string, from, through model.Time, metricName string, matchers ...*labels.Matcher) ([]string, error) | ||
| Stats(ctx context.Context, userID string, from, through model.Time, matchers ...*labels.Matcher) (*stats.Stats, error) | ||
| Volume(ctx context.Context, userID string, from, through model.Time, limit int32, targetLabels []string, aggregateBy string, matchers ...*labels.Matcher) (*logproto.VolumeResponse, error) | ||
| GetShards( | ||
| ctx context.Context, | ||
| userID string, | ||
| from, through model.Time, | ||
| targetBytesPerShard uint64, | ||
| predicate chunk.Predicate, | ||
| ) (*logproto.ShardsResponse, error) |
There was a problem hiding this comment.
I often struggle to understand when interface composition and when separate definition is better. From the PR alone it's not clear to me why making the function definitions separately in this interface has advantages. Could you elaborate? I assume this is kinda necessary for future work?
There was a problem hiding this comment.
This change reduces the Store interface to only include methods actually used by the querier.
Previously, the interface included unused methods from storage.SelectStore and
index interfaces, creating unnecessary coupling between packages.
Following Go's interface segregation principle, interfaces should only declare
methods that clients need. This makes the code more maintainable since:
- Changes to unused methods in storage/index packages won't affect the querier
- The querier's dependencies are explicitly visible through its minimal interface
Interface composition is still valuable when building upon well-known Go
or project wide patterns (like io.Writer or iter.Iterator), but wasn't necessary in this case.
pkg/querier/tail/metrics.go
Outdated
|
|
||
| import ( | ||
| "github.com/prometheus/client_golang/prometheus" | ||
| "github.com/prometheus/client_golang/prometheus/promauto" | ||
| ) | ||
|
|
||
| type Metrics struct { | ||
| type TailMetrics struct { |
There was a problem hiding this comment.
These should be called Metrics like before, see linter error.
What this PR does / why we need it:
This PR moves everything related to tailing into it's own package but also surface better interface for the store and the querier.
This is required before adding a new storage. Next I'm going to allow merging multiple store from different period that not specifically chunks store.
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