Backend Search: Two fixes (#1221)

* don't pass 0's for unset params Signed-off-by: Joe Elliott <number101010@gmail.com> * prioritize ingester request Signed-off-by: Joe Elliott <number101010@gmail.com> * improved test Signed-off-by: Joe Elliott <number101010@gmail.com>
grafana · Jan 11, 2022 · c7d9d18 · c7d9d18
1 parent f73913f
commit c7d9d18
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 13 deletions.
diff --git a/modules/frontend/searchsharding.go b/modules/frontend/searchsharding.go
@@ -206,9 +206,11 @@ func (s searchSharder) RoundTrip(r *http.Request) (*http.Response, error) {
 			return nil, err
 		}
 	}
-	// add ingester request if we have one
+	// add ingester request if we have one. it's important to add the ingeste request to
+	// the beginning of the slice so it is prioritized over the possibly enormous
+	// number of backend requests
 	if ingesterReq != nil {
-		reqs = append(reqs, ingesterReq)
+		reqs = append([]*http.Request{ingesterReq}, reqs...)
 	}
 	span.SetTag("request-count", len(reqs))
 

diff --git a/pkg/api/http.go b/pkg/api/http.go
@@ -272,23 +272,32 @@ func BuildSearchRequest(req *http.Request, searchReq *tempopb.SearchRequest) (*h
 	}
 
 	q := req.URL.Query()
+
 	q.Set(urlParamStart, strconv.FormatUint(uint64(searchReq.Start), 10))
 	q.Set(urlParamEnd, strconv.FormatUint(uint64(searchReq.End), 10))
-	q.Set(urlParamLimit, strconv.FormatUint(uint64(searchReq.Limit), 10))
-	q.Set(urlParamMaxDuration, strconv.FormatUint(uint64(searchReq.MaxDurationMs), 10)+"ms")
-	q.Set(urlParamMinDuration, strconv.FormatUint(uint64(searchReq.MinDurationMs), 10)+"ms")
+	if searchReq.Limit != 0 {
+		q.Set(urlParamLimit, strconv.FormatUint(uint64(searchReq.Limit), 10))
+	}
+	if searchReq.MaxDurationMs != 0 {
+		q.Set(urlParamMaxDuration, strconv.FormatUint(uint64(searchReq.MaxDurationMs), 10)+"ms")
+	}
+	if searchReq.MinDurationMs != 0 {
+		q.Set(urlParamMinDuration, strconv.FormatUint(uint64(searchReq.MinDurationMs), 10)+"ms")
+	}
 
-	builder := &strings.Builder{}
-	encoder := logfmt.NewEncoder(builder)
+	if len(searchReq.Tags) > 0 {
+		builder := &strings.Builder{}
+		encoder := logfmt.NewEncoder(builder)
 
-	for k, v := range searchReq.Tags {
-		err := encoder.EncodeKeyval(k, v)
-		if err != nil {
-			return nil, err
+		for k, v := range searchReq.Tags {
+			err := encoder.EncodeKeyval(k, v)
+			if err != nil {
+				return nil, err
+			}
 		}
-	}
 
-	q.Set(urlParamTags, builder.String())
+		q.Set(urlParamTags, builder.String())
+	}
 
 	req.URL.RawQuery = q.Encode()
 

diff --git a/pkg/api/http_test.go b/pkg/api/http_test.go
@@ -392,6 +392,52 @@ func TestBuildSearchRequest(t *testing.T) {
 			},
 			query: "?end=20&limit=50&maxDuration=40ms&minDuration=30ms&start=10&tags=foo%3Dbar",
 		},
+		{
+			req: &tempopb.SearchRequest{
+				Tags: map[string]string{
+					"foo": "bar",
+				},
+				Start:         10,
+				End:           20,
+				MaxDurationMs: 30,
+				Limit:         50,
+			},
+			query: "?end=20&limit=50&maxDuration=30ms&start=10&tags=foo%3Dbar",
+		},
+		{
+			req: &tempopb.SearchRequest{
+				Tags: map[string]string{
+					"foo": "bar",
+				},
+				Start:         10,
+				End:           20,
+				MinDurationMs: 30,
+				Limit:         50,
+			},
+			query: "?end=20&limit=50&minDuration=30ms&start=10&tags=foo%3Dbar",
+		},
+		{
+			req: &tempopb.SearchRequest{
+				Tags: map[string]string{
+					"foo": "bar",
+				},
+				Start:         10,
+				End:           20,
+				MinDurationMs: 30,
+				MaxDurationMs: 40,
+			},
+			query: "?end=20&maxDuration=40ms&minDuration=30ms&start=10&tags=foo%3Dbar",
+		},
+		{
+			req: &tempopb.SearchRequest{
+				Tags:          map[string]string{},
+				Start:         10,
+				End:           20,
+				MinDurationMs: 30,
+				MaxDurationMs: 40,
+			},
+			query: "?end=20&maxDuration=40ms&minDuration=30ms&start=10",
+		},
 	}
 
 	for _, tc := range tests {