diff --git a/modules/frontend/searchsharding.go b/modules/frontend/searchsharding.go index 7a22747a400..9b6a97c9256 100644 --- a/modules/frontend/searchsharding.go +++ b/modules/frontend/searchsharding.go @@ -135,7 +135,7 @@ func (s searchSharder) RoundTrip(r *http.Request) (*http.Response, error) { // build request to search ingester based on query_ingesters_until config and time range // pass subCtx in requests so we can cancel and exit early - ingesterReq, err := s.ingesterRequest(subCtx, tenantID, r, searchReq) + ingesterReq, err := s.ingesterRequest(subCtx, tenantID, r, *searchReq) if err != nil { return nil, err } @@ -433,10 +433,10 @@ func pagesPerRequest(m *backend.BlockMeta, bytesPerRequest int) int { // that covers the ingesters. If nil is returned for the http.Request then there is no ingesters query. // since this function modifies searchReq.Start and End we are taking a value instead of a pointer to prevent it from // unexpectedly changing the passed searchReq. -func (s *searchSharder) ingesterRequest(ctx context.Context, tenantID string, parent *http.Request, searchReq *tempopb.SearchRequest) (*http.Request, error) { +func (s *searchSharder) ingesterRequest(ctx context.Context, tenantID string, parent *http.Request, searchReq tempopb.SearchRequest) (*http.Request, error) { // request without start or end, search only in ingester if searchReq.Start == 0 || searchReq.End == 0 { - return buildIngesterRequest(ctx, tenantID, parent, searchReq) + return buildIngesterRequest(ctx, tenantID, parent, &searchReq) } now := time.Now() @@ -463,7 +463,7 @@ func (s *searchSharder) ingesterRequest(ctx context.Context, tenantID string, pa searchReq.Start = ingesterStart searchReq.End = ingesterEnd - return buildIngesterRequest(ctx, tenantID, parent, searchReq) + return buildIngesterRequest(ctx, tenantID, parent, &searchReq) } func buildIngesterRequest(ctx context.Context, tenantID string, parent *http.Request, searchReq *tempopb.SearchRequest) (*http.Request, error) { diff --git a/modules/frontend/searchsharding_test.go b/modules/frontend/searchsharding_test.go index c69636c7f82..3d404492b38 100644 --- a/modules/frontend/searchsharding_test.go +++ b/modules/frontend/searchsharding_test.go @@ -8,6 +8,7 @@ import ( "io" "net/http" "net/http/httptest" + "reflect" "strconv" "strings" "sync" @@ -388,7 +389,8 @@ func TestIngesterRequest(t *testing.T) { searchReq, err := api.ParseSearchRequest(req) require.NoError(t, err) - actualReq, err := s.ingesterRequest(context.Background(), "test", req, searchReq) + copyReq := searchReq + actualReq, err := s.ingesterRequest(context.Background(), "test", req, *searchReq) if tc.expectedError != nil { assert.Equal(t, tc.expectedError, err) continue @@ -399,6 +401,10 @@ func TestIngesterRequest(t *testing.T) { } else { assert.Equal(t, tc.expectedURI, actualReq.RequestURI) } + + // it may seem odd to test that the searchReq is not modified, but this is to prevent an issue that + // occurs if the ingesterRequest method is changed to take a searchReq pointer + require.True(t, reflect.DeepEqual(copyReq, searchReq)) } }