feat: reduce allocs in query sharder (#3932)

* reduce allocs in query sharder * changelog * expose a single method to generate the url with query params * move helper method to common api/http.go
grafana · Aug 7, 2024 · eae8c17 · eae8c17
1 parent b12c714
commit eae8c17
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 9 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -46,6 +46,8 @@
 * [ENHANCEMENT] Reduce log level verbosity for e2e tests[#3900](https://github.com/grafana/tempo/pull/3900) (@javiermolinar)
 * [ENHANCEMENT] Added new Traces api V2[#3912](https://github.com/grafana/tempo/pull/3912) (@javiermolinar)
 * [ENHANCEMENT] Update to the latest dskit [#3915](https://github.com/grafana/tempo/pull/3915) (@andreasgerstmayr)
+* [ENHANCEMENT] Reduce allocs building queriers sharded requests [#3932](https://github.com/grafana/tempo/pull/3932) (@javiermolinar)
+
 * [ENHANCEMENT] Implement polling tenants concurrently [#3647](https://github.com/grafana/tempo/pull/3647) (@zalegrala)
 * [BUGFIX] Fix panic in certain metrics queries using `rate()` with `by` [#3847](https://github.com/grafana/tempo/pull/3847) (@stoewer)
 * [BUGFIX] Fix double appending the primary iterator on second pass with event iterator [#3903](https://github.com/grafana/tempo/pull/3903) (@ie-pham)

diff --git a/modules/frontend/traceid_sharder.go b/modules/frontend/traceid_sharder.go
@@ -12,6 +12,7 @@ import (
 	"github.com/grafana/tempo/modules/frontend/combiner"
 	"github.com/grafana/tempo/modules/frontend/pipeline"
 	"github.com/grafana/tempo/modules/querier"
+	"github.com/grafana/tempo/pkg/api"
 	"github.com/grafana/tempo/pkg/blockboundary"
 )
 
@@ -71,22 +72,20 @@ func (s *asyncTraceSharder) buildShardedRequests(ctx context.Context, parent *ht
 	}
 
 	reqs := make([]*http.Request, s.cfg.QueryShards)
+	params := map[string]string{}
 	// build sharded block queries
 	for i := 0; i < len(s.blockBoundaries); i++ {
 		reqs[i] = parent.Clone(ctx)
-
-		q := reqs[i].URL.Query()
 		if i == 0 {
 			// ingester query
-			q.Add(querier.QueryModeKey, querier.QueryModeIngesters)
+			params[querier.QueryModeKey] = querier.QueryModeIngesters
 		} else {
 			// block queries
-			q.Add(querier.BlockStartKey, hex.EncodeToString(s.blockBoundaries[i-1]))
-			q.Add(querier.BlockEndKey, hex.EncodeToString(s.blockBoundaries[i]))
-			q.Add(querier.QueryModeKey, querier.QueryModeBlocks)
+			params[querier.BlockStartKey] = hex.EncodeToString(s.blockBoundaries[i-1])
+			params[querier.BlockEndKey] = hex.EncodeToString(s.blockBoundaries[i])
+			params[querier.QueryModeKey] = querier.QueryModeBlocks
 		}
-		reqs[i].URL.RawQuery = q.Encode()
-
+		reqs[i] = api.BuildQueryRequest(reqs[i], params)
 		prepareRequestForQueriers(reqs[i], userID)
 	}
 

diff --git a/modules/frontend/traceid_sharder_test.go b/modules/frontend/traceid_sharder_test.go
@@ -29,5 +29,5 @@ func TestBuildShardedRequests(t *testing.T) {
 	require.Len(t, shardedReqs, queryShards)
 
 	require.Equal(t, "/querier?mode=ingesters", shardedReqs[0].RequestURI)
-	require.Equal(t, "/querier?blockEnd=ffffffffffffffffffffffffffffffff&blockStart=00000000000000000000000000000000&mode=blocks", shardedReqs[1].RequestURI)
+	urisEqual(t, []string{"/querier?blockEnd=ffffffffffffffffffffffffffffffff&blockStart=00000000000000000000000000000000&mode=blocks"}, []string{shardedReqs[1].RequestURI})
 }
diff --git a/pkg/api/http.go b/pkg/api/http.go
@@ -501,6 +501,21 @@ func BuildQueryRangeRequest(req *http.Request, searchReq *tempopb.QueryRangeRequ
 	return req
 }
 
+// Generic helper to append query parameters to an http request with less allocations
+func BuildQueryRequest(req *http.Request, queryParams map[string]string) *http.Request {
+	if req == nil {
+		req = &http.Request{
+			URL: &url.URL{},
+		}
+	}
+	qb := newQueryBuilder(req.URL.RawQuery)
+	for k, v := range queryParams {
+		qb.addParam(k, v)
+	}
+	req.URL.RawQuery = qb.query()
+	return req
+}
+
 func bounds(vals url.Values) (time.Time, time.Time, error) {
 	var (
 		now      = time.Now()