Add a startTime and endTime parameter to the trace by id query API to improve query performance

docs/tempo/website/api_docs/_index.md

@@ -97,7 +97,7 @@ GET /api/traces/<traceid>
 The following query API is also provided on the querier service for _debugging_ purposes.
 
 ```
-GET /querier/api/traces/<traceid>?mode=xxxx&blockStart=0000&blockEnd=FFFF
+GET /querier/api/traces/<traceid>?mode=xxxx&blockStart=0000&blockEnd=FFFF&start=<start>&end=<end>
 ```
 Parameters:
 - `mode = (blocks|ingesters|all)`
@@ -111,6 +111,10 @@ Parameters:
   Specifies the blockID finish boundary. If specified, the querier will only search blocks with IDs < blockEnd.
   Default = `FFFFFFFF-FFFF-FFFF-FFFF-FFFFFFFFFFFF`
   Example: `blockStart=FFFFFFFF-FFFF-FFFF-FFFF-456787652341`
+- `start = (unix epoch seconds)`
+  Optional.  Along with `end` define a time range from which traces should be returned. 
+- `end = (unix epoch seconds)`
+  Optional.  Along with `start` define a time range from which traces should be returned. Providing both `start` and `end` will include blocks for the specified time range only. If the parameters are not provided then Tempo will check for the trace across all blocks in backend. If the parameters are provided, it will only check in the blocks within the specified time range, this can result in trace not being found or partial results if it does not fall in the specified time range.
 
 Note that this API is not meant to be used directly unless for debugging the sharding functionality of the query 
 frontend.

docs/tempo/website/configuration/_index.md

@@ -763,10 +763,13 @@ storage:
             [search_encoding: <string> | default = none]
 
             # When a span is written to the WAL it adjusts the start and end times of the block it is written to.
-            # This block start and end time range is then used when choosing blocks for search. To prevent spans too far
+            # This block start and end time range is then used when choosing blocks for search.
+            # This is also used for querying traces by ID when the start and end parameters are specified. To prevent spans too far
             # in the past or future from impacting the block start and end times we use this configuration option.
             # This option only allows spans that occur within the configured duration to adjust the block start and
-            # end times.
+            # end times. 
+            # This can result in trace not being found if the trace falls outside the slack configuration value as the
+            # start and end times of the block will not be updated in this case.
             [ingestion_time_range_slack: <duration> | default = 2m]
 
         # block configuration

modules/frontend/frontend.go

@@ -107,6 +107,16 @@ func newTraceByIDMiddleware(cfg Config, logger log.Logger) Middleware {
 				}, nil
 			}
 
+			//validate start and end parameter
+			_, _, _, _, _, reqErr := api.ValidateAndSanitizeRequest(r)
+			if reqErr != nil {
+				return &http.Response{
+					StatusCode: http.StatusBadRequest,
+					Body:       io.NopCloser(strings.NewReader(reqErr.Error())),
+					Header:     http.Header{},
+				}, nil
+			}
+
 			// check marshalling format
 			marshallingFormat := api.HeaderAcceptJSON
 			if r.Header.Get(api.HeaderAccept) == api.HeaderAcceptProtobuf {

modules/frontend/searchsharding_test.go

@@ -31,7 +31,7 @@ type mockReader struct {
 	metas []*backend.BlockMeta
 }
 
-func (m *mockReader) Find(ctx context.Context, tenantID string, id common.ID, blockStart string, blockEnd string) ([]*tempopb.Trace, []error, error) {
+func (m *mockReader) Find(ctx context.Context, tenantID string, id common.ID, blockStart string, blockEnd string, timeStart int64, timeEnd int64) ([]*tempopb.Trace, []error, error) {
 	return nil, nil, nil
 }
 

modules/querier/http.go

@@ -8,20 +8,19 @@ import (
 
 	"github.com/golang/protobuf/jsonpb"
 	"github.com/golang/protobuf/proto"
-	"github.com/google/uuid"
 	"github.com/gorilla/mux"
 	"github.com/grafana/tempo/pkg/api"
 	"github.com/grafana/tempo/pkg/tempopb"
-	"github.com/grafana/tempo/tempodb"
 	"github.com/opentracing/opentracing-go"
 	ot_log "github.com/opentracing/opentracing-go/log"
-	"github.com/pkg/errors"
 )
 
 const (
 	BlockStartKey = "blockStart"
 	BlockEndKey   = "blockEnd"
 	QueryModeKey  = "mode"
+	TimeStartKey  = "start"
+	TimeEndKey    = "end"
 
 	QueryModeIngesters = "ingesters"
 	QueryModeBlocks    = "blocks"
@@ -44,7 +43,7 @@ func (q *Querier) TraceByIDHandler(w http.ResponseWriter, r *http.Request) {
 	}
 
 	// validate request
-	blockStart, blockEnd, queryMode, err := validateAndSanitizeRequest(r)
+	blockStart, blockEnd, queryMode, timeStart, timeEnd, err := api.ValidateAndSanitizeRequest(r)
 	if err != nil {
 		http.Error(w, err.Error(), http.StatusBadRequest)
 		return
@@ -53,14 +52,16 @@ func (q *Querier) TraceByIDHandler(w http.ResponseWriter, r *http.Request) {
 		ot_log.String("msg", "validated request"),
 		ot_log.String("blockStart", blockStart),
 		ot_log.String("blockEnd", blockEnd),
-		ot_log.String("queryMode", queryMode))
+		ot_log.String("queryMode", queryMode),
+		ot_log.String("timeStart", fmt.Sprint(timeStart)),
+		ot_log.String("timeEnd", fmt.Sprint(timeEnd)))
 
 	resp, err := q.FindTraceByID(ctx, &tempopb.TraceByIDRequest{
 		TraceID:    byteID,
 		BlockStart: blockStart,
 		BlockEnd:   blockEnd,
 		QueryMode:  queryMode,
-	})
+	}, timeStart, timeEnd)
 	if err != nil {
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
@@ -98,53 +99,6 @@ func (q *Querier) TraceByIDHandler(w http.ResponseWriter, r *http.Request) {
 	w.Header().Set(api.HeaderContentType, api.HeaderAcceptJSON)
 }
 
-// return values are (blockStart, blockEnd, queryMode, error)
-func validateAndSanitizeRequest(r *http.Request) (string, string, string, error) {
-	q := r.URL.Query().Get(QueryModeKey)
-
-	// validate queryMode. it should either be empty or one of (QueryModeIngesters|QueryModeBlocks|QueryModeAll)
-	var queryMode string
-	if len(q) == 0 || q == QueryModeAll {
-		queryMode = QueryModeAll
-	} else if q == QueryModeIngesters {
-		queryMode = QueryModeIngesters
-	} else if q == QueryModeBlocks {
-		queryMode = QueryModeBlocks
-	} else {
-		return "", "", "", fmt.Errorf("invalid value for mode %s", q)
-	}
-
-	// no need to validate/sanitize other parameters if queryMode == QueryModeIngesters
-	if queryMode == QueryModeIngesters {
-		return "", "", queryMode, nil
-	}
-
-	start := r.URL.Query().Get(BlockStartKey)
-	end := r.URL.Query().Get(BlockEndKey)
-
-	// validate start. it should either be empty or a valid uuid
-	if len(start) == 0 {
-		start = tempodb.BlockIDMin
-	} else {
-		_, err := uuid.Parse(start)
-		if err != nil {
-			return "", "", "", errors.Wrap(err, "invalid value for blockStart")
-		}
-	}
-
-	// validate end. it should either be empty or a valid uuid
-	if len(end) == 0 {
-		end = tempodb.BlockIDMax
-	} else {
-		_, err := uuid.Parse(end)
-		if err != nil {
-			return "", "", "", errors.Wrap(err, "invalid value for blockEnd")
-		}
-	}
-
-	return start, end, queryMode, nil
-}
-
 func (q *Querier) SearchHandler(w http.ResponseWriter, r *http.Request) {
 	isSearchBlock := api.IsSearchBlock(r)
 

modules/querier/querier.go

@@ -168,7 +168,7 @@ func (q *Querier) stopping(_ error) error {
 }
 
 // FindTraceByID implements tempopb.Querier.
-func (q *Querier) FindTraceByID(ctx context.Context, req *tempopb.TraceByIDRequest) (*tempopb.TraceByIDResponse, error) {
+func (q *Querier) FindTraceByID(ctx context.Context, req *tempopb.TraceByIDRequest, timeStart int64, timeEnd int64) (*tempopb.TraceByIDResponse, error) {
 	if !validation.ValidTraceID(req.TraceID) {
 		return nil, fmt.Errorf("invalid trace id")
 	}
@@ -217,7 +217,9 @@ func (q *Querier) FindTraceByID(ctx context.Context, req *tempopb.TraceByIDReque
 	var failedBlocks int
 	if req.QueryMode == QueryModeBlocks || req.QueryMode == QueryModeAll {
 		span.LogFields(ot_log.String("msg", "searching store"))
-		partialTraces, blockErrs, err := q.store.Find(opentracing.ContextWithSpan(ctx, span), userID, req.TraceID, req.BlockStart, req.BlockEnd)
+		span.LogFields(ot_log.String("timeStart", fmt.Sprint(timeStart)))
+		span.LogFields(ot_log.String("timeEnd", fmt.Sprint(timeEnd)))
+		partialTraces, blockErrs, err := q.store.Find(opentracing.ContextWithSpan(ctx, span), userID, req.TraceID, req.BlockStart, req.BlockEnd, timeStart, timeEnd)
 		if err != nil {
 			return nil, errors.Wrap(err, "error querying store in Querier.FindTraceByID")
 		}

pkg/api/http.go

@@ -14,6 +14,7 @@ import (
 	"github.com/gorilla/mux"
 	"github.com/grafana/tempo/pkg/tempopb"
 	"github.com/grafana/tempo/pkg/util"
+	"github.com/grafana/tempo/tempodb"
 	"github.com/grafana/tempo/tempodb/backend"
 )
 
@@ -53,6 +54,13 @@ const (
 	PathSearchTagValues = "/api/search/tag/{tagName}/values"
 	PathEcho            = "/api/echo"
 
+	QueryModeKey       = "mode"
+	QueryModeIngesters = "ingesters"
+	QueryModeBlocks    = "blocks"
+	QueryModeAll       = "all"
+	BlockStartKey      = "blockStart"
+	BlockEndKey        = "blockEnd"
+
 	defaultLimit = 20
 )
 
@@ -372,3 +380,75 @@ func extractQueryParam(r *http.Request, param string) (string, bool) {
 	value := r.URL.Query().Get(param)
 	return value, value != ""
 }
+
+// ValidateAndSanitizeRequest validates params for trace by id api
+// return values are (blockStart, blockEnd, queryMode, start, end, error)
+func ValidateAndSanitizeRequest(r *http.Request) (string, string, string, int64, int64, error) {
+	q, _ := extractQueryParam(r, QueryModeKey)
+
+	// validate queryMode. it should either be empty or one of (QueryModeIngesters|QueryModeBlocks|QueryModeAll)
+	var queryMode string
+	var startTime int64
+	var endTime int64
+	var blockStart string
+	var blockEnd string
+	if len(q) == 0 || q == QueryModeAll {
+		queryMode = QueryModeAll
+	} else if q == QueryModeIngesters {
+		queryMode = QueryModeIngesters
+	} else if q == QueryModeBlocks {
+		queryMode = QueryModeBlocks
+	} else {
+		return "", "", "", 0, 0, fmt.Errorf("invalid value for mode %s", q)
+	}
+
+	// no need to validate/sanitize other parameters if queryMode == QueryModeIngesters
+	if queryMode == QueryModeIngesters {
+		return "", "", queryMode, 0, 0, nil
+	}
+
+	if start, ok := extractQueryParam(r, BlockStartKey); ok {
+		_, err := uuid.Parse(start)
+		if err != nil {
+			return "", "", "", 0, 0, fmt.Errorf("invalid value for blockstart: %w", err)
+		}
+		blockStart = start
+	} else {
+		blockStart = tempodb.BlockIDMin
+	}
+
+	if end, ok := extractQueryParam(r, BlockEndKey); ok {
+		_, err := uuid.Parse(end)
+		if err != nil {
+			return "", "", "", 0, 0, fmt.Errorf("invalid value for blockEnd: %w", err)
+		}
+		blockEnd = end
+	} else {
+		blockEnd = tempodb.BlockIDMax
+	}
+
+	if s, ok := extractQueryParam(r, urlParamStart); ok {
+		var err error
+		startTime, err = strconv.ParseInt(s, 10, 64)
+		if err != nil {
+			return "", "", "", 0, 0, fmt.Errorf("invalid start: %w", err)
+		}
+	} else {
+		startTime = 0
+	}
+
+	if s, ok := extractQueryParam(r, urlParamEnd); ok {
+		var err error
+		endTime, err = strconv.ParseInt(s, 10, 64)
+		if err != nil {
+			return "", "", "", 0, 0, fmt.Errorf("invalid end: %w", err)
+		}
+	} else {
+		endTime = 0
+	}
+
+	if startTime != 0 && endTime != 0 && endTime <= startTime {
+		return "", "", "", 0, 0, fmt.Errorf("http parameter start must be before end. received start=%d end=%d", startTime, endTime)
+	}
+	return blockStart, blockEnd, queryMode, startTime, endTime, nil
+}

pkg/api/http_test.go

@@ -374,6 +374,75 @@ func TestBuildSearchBlockRequest(t *testing.T) {
 	}
 }
 
+func TestValidateAndSanitizeRequest(t *testing.T) {
+	tests := []struct {
+		httpReq       *http.Request
+		queryMode     string
+		startTime     int64
+		endTime       int64
+		blockStart    string
+		blockEnd      string
+		expectedError string
+	}{
+		{
+			httpReq:    httptest.NewRequest("GET", "/api/traces/1234?blockEnd=ffffffffffffffffffffffffffffffff&blockStart=00000000000000000000000000000000&mode=blocks&start=1&end=2", nil),
+			queryMode:  "blocks",
+			startTime:  1,
+			endTime:    2,
+			blockStart: "00000000000000000000000000000000",
+			blockEnd:   "ffffffffffffffffffffffffffffffff",
+		},
+		{
+			httpReq:    httptest.NewRequest("GET", "/api/traces/1234?blockEnd=ffffffffffffffffffffffffffffffff&blockStart=00000000000000000000000000000000&mode=blocks", nil),
+			queryMode:  "blocks",
+			startTime:  0,
+			endTime:    0,
+			blockStart: "00000000000000000000000000000000",
+			blockEnd:   "ffffffffffffffffffffffffffffffff",
+		},
+		{
+			httpReq:    httptest.NewRequest("GET", "/api/traces/1234?mode=blocks", nil),
+			queryMode:  "blocks",
+			startTime:  0,
+			endTime:    0,
+			blockStart: "00000000-0000-0000-0000-000000000000",
+			blockEnd:   "FFFFFFFF-FFFF-FFFF-FFFF-FFFFFFFFFFFF",
+		},
+		{
+			httpReq:    httptest.NewRequest("GET", "/api/traces/1234?mode=blocks&blockStart=12345678000000001235000001240000&blockEnd=ffffffffffffffffffffffffffffffff", nil),
+			queryMode:  "blocks",
+			startTime:  0,
+			endTime:    0,
+			blockStart: "12345678000000001235000001240000",
+			blockEnd:   "ffffffffffffffffffffffffffffffff",
+		},
+		{
+			httpReq:       httptest.NewRequest("GET", "/api/traces/1234?mode=blocks&blockStart=12345678000000001235000001240000&blockEnd=ffffffffffffffffffffffffffffffff&start=1&end=1", nil),
+			queryMode:     "blocks",
+			startTime:     0,
+			endTime:       0,
+			blockStart:    "12345678000000001235000001240000",
+			blockEnd:      "ffffffffffffffffffffffffffffffff",
+			expectedError: "http parameter start must be before end. received start=1 end=1",
+		},
+	}
+
+	for _, tc := range tests {
+		blockStart, blockEnd, queryMode, startTime, endTime, err := ValidateAndSanitizeRequest(tc.httpReq)
+		if len(tc.expectedError) != 0 {
+			assert.EqualError(t, err, tc.expectedError)
+			continue
+		}
+		assert.NoError(t, err)
+		assert.Equal(t, tc.queryMode, queryMode)
+		assert.Equal(t, tc.blockStart, blockStart)
+		assert.Equal(t, tc.blockEnd, blockEnd)
+		assert.Equal(t, tc.startTime, startTime)
+		assert.Equal(t, tc.endTime, endTime)
+	}
+
+}
+
 func TestBuildSearchRequest(t *testing.T) {
 	tests := []struct {
 		req     *tempopb.SearchRequest

tempodb/compactor_test.go

@@ -165,7 +165,7 @@ func TestCompaction(t *testing.T) {
 
 	// now see if we can find our ids
 	for i, id := range allIds {
-		trs, failedBlocks, err := rw.Find(context.Background(), testTenantID, id, BlockIDMin, BlockIDMax)
+		trs, failedBlocks, err := rw.Find(context.Background(), testTenantID, id, BlockIDMin, BlockIDMax, 0, 0)
 		require.NoError(t, err)
 		require.Nil(t, failedBlocks)
 		require.NotNil(t, trs)
@@ -292,7 +292,7 @@ func TestSameIDCompaction(t *testing.T) {
 
 	// search for all ids
 	for i, id := range allIds {
-		trs, failedBlocks, err := rw.Find(context.Background(), testTenantID, id, BlockIDMin, BlockIDMax)
+		trs, failedBlocks, err := rw.Find(context.Background(), testTenantID, id, BlockIDMin, BlockIDMax, 0, 0)
 		assert.NoError(t, err)
 		assert.Nil(t, failedBlocks)
 
@@ -373,7 +373,7 @@ func TestCompactionUpdatesBlocklist(t *testing.T) {
 	// Make sure all expected traces are found.
 	for i := 0; i < blockCount; i++ {
 		for j := 0; j < recordCount; j++ {
-			trace, failedBlocks, err := rw.Find(context.TODO(), testTenantID, makeTraceID(i, j), BlockIDMin, BlockIDMax)
+			trace, failedBlocks, err := rw.Find(context.TODO(), testTenantID, makeTraceID(i, j), BlockIDMin, BlockIDMax, 0, 0)
 			require.NotNil(t, trace)
 			require.Greater(t, len(trace), 0)
 			require.NoError(t, err)