From 6743584939c7e25b6f2d67964a4ed245276aa671 Mon Sep 17 00:00:00 2001
From: Filip Petkovski <filip.petkovsky@gmail.com>
Date: Wed, 13 Jul 2022 19:23:17 +0200
Subject: [PATCH] First code review pass

Signed-off-by: Filip Petkovski <filip.petkovsky@gmail.com>
---
 pkg/querysharding/analysis.go   | 8 ++++----
 pkg/querysharding/analyzer.go   | 2 +-
 pkg/store/prometheus.go         | 1 -
 test/e2e/query_frontend_test.go | 5 ++---
 4 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/pkg/querysharding/analysis.go b/pkg/querysharding/analysis.go
index 90f9fcc8069..d46a0048c60 100644
--- a/pkg/querysharding/analysis.go
+++ b/pkg/querysharding/analysis.go
@@ -29,7 +29,7 @@ func newShardableByLabels(labels []string, by bool) QueryAnalysis {
 	}
 }
 
-func (q QueryAnalysis) scopeToLabels(labels []string, by bool) QueryAnalysis {
+func (q *QueryAnalysis) scopeToLabels(labels []string, by bool) QueryAnalysis {
 	labels = without(labels, excludedLabels)
 
 	if q.shardingLabels == nil {
@@ -52,11 +52,11 @@ func (q QueryAnalysis) scopeToLabels(labels []string, by bool) QueryAnalysis {
 	}
 }
 
-func (q QueryAnalysis) IsShardable() bool {
+func (q *QueryAnalysis) IsShardable() bool {
 	return len(q.shardingLabels) > 0
 }
 
-func (q QueryAnalysis) ShardingLabels() []string {
+func (q *QueryAnalysis) ShardingLabels() []string {
 	if len(q.shardingLabels) == 0 {
 		return nil
 	}
@@ -64,7 +64,7 @@ func (q QueryAnalysis) ShardingLabels() []string {
 	return q.shardingLabels
 }
 
-func (q QueryAnalysis) ShardBy() bool {
+func (q *QueryAnalysis) ShardBy() bool {
 	return q.shardBy
 }
 
diff --git a/pkg/querysharding/analyzer.go b/pkg/querysharding/analyzer.go
index e3fe99f951a..b0e5538efae 100644
--- a/pkg/querysharding/analyzer.go
+++ b/pkg/querysharding/analyzer.go
@@ -27,7 +27,7 @@ func NewQueryAnalyzer() *QueryAnalyzer {
 
 // Analyze uses the following algorithm:
 // * if a query has subqueries, such as label_join or label_replace,
-//   then treat the query as non shardable.
+//   or has functions which cannot be sharded, then treat the query as non shardable.
 // * if the query's root expression has grouping labels,
 //   then treat the query as shardable by those labels.
 // * if the query's root expression has no grouping labels,
diff --git a/pkg/store/prometheus.go b/pkg/store/prometheus.go
index 93991e06a21..8fbfa12d0ff 100644
--- a/pkg/store/prometheus.go
+++ b/pkg/store/prometheus.go
@@ -201,7 +201,6 @@ func (p *PrometheusStore) Series(r *storepb.SeriesRequest, s storepb.Store_Serie
 	}
 
 	q := &prompb.Query{StartTimestampMs: r.MinTime, EndTimestampMs: r.MaxTime}
-	level.Info(p.logger).Log("min_time", r.MinTime, "max_time", r.MaxTime)
 	for _, m := range matchers {
 		pm := &prompb.LabelMatcher{Name: m.Name, Value: m.Value}
 
diff --git a/test/e2e/query_frontend_test.go b/test/e2e/query_frontend_test.go
index 84f115c0a13..651f4e10c9c 100644
--- a/test/e2e/query_frontend_test.go
+++ b/test/e2e/query_frontend_test.go
@@ -9,9 +9,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/thanos-io/thanos/pkg/block/metadata"
-	"github.com/thanos-io/thanos/pkg/testutil/e2eutil"
-
 	"github.com/efficientgo/e2e"
 	"github.com/efficientgo/e2e/matchers"
 	"github.com/pkg/errors"
@@ -19,10 +16,12 @@ import (
 	"github.com/prometheus/prometheus/model/labels"
 	"github.com/prometheus/prometheus/model/timestamp"
 
+	"github.com/thanos-io/thanos/pkg/block/metadata"
 	"github.com/thanos-io/thanos/pkg/cacheutil"
 	"github.com/thanos-io/thanos/pkg/promclient"
 	"github.com/thanos-io/thanos/pkg/queryfrontend"
 	"github.com/thanos-io/thanos/pkg/testutil"
+	"github.com/thanos-io/thanos/pkg/testutil/e2eutil"
 	"github.com/thanos-io/thanos/test/e2e/e2ethanos"
 )