Bugfix: Default step for gRPC streaming query range queries (#4546)

* foo Signed-off-by: Joe Elliott <[email protected]> * fix test Signed-off-by: Joe Elliott <[email protected]> * fix an issue where exemplars were not being dumped correctly Signed-off-by: Joe Elliott <[email protected]> * changelog Signed-off-by: Joe Elliott <[email protected]> * lint Signed-off-by: Joe Elliott <[email protected]> --------- Signed-off-by: Joe Elliott <[email protected]>
grafana · Jan 13, 2025 · 424274a · 424274a
1 parent 8d2eb8e
commit 424274a
Show file tree

Hide file tree

Showing 5 changed files with 19 additions and 6 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,8 @@
 ## main / unreleased
 
+* [BUGFIX] Choose a default step for a gRPC streaming query range request if none is provided. [#4546](https://github.com/grafana/tempo/pull/4546) (@joe-elliott)
+  Fix an issue where the tempo-cli was not correctly dumping exemplar results.
+
 # v2.7.0
 
 * [CHANGE] Disable gRPC compression in the querier and distributor for performance reasons [#4429](https://github.com/grafana/tempo/pull/4429) (@carles-grafana)

diff --git a/cmd/tempo-cli/cmd-query-metrics-query-range.go b/cmd/tempo-cli/cmd-query-metrics-query-range.go
@@ -61,7 +61,6 @@ func (cmd *metricsQueryCmd) Run(_ *globalOptions) error {
 		Query: cmd.TraceQL,
 		Start: uint64(start),
 		End:   uint64(end),
-		Step:  uint64(5 * time.Second),
 	}
 
 	if cmd.UseGRPC {

diff --git a/cmd/tempo-cli/shared.go b/cmd/tempo-cli/shared.go
@@ -2,13 +2,14 @@ package main
 
 import (
 	"context"
-	"encoding/json"
 	"errors"
 	"fmt"
 	"sort"
 	"strconv"
 	"time"
 
+	"github.com/gogo/protobuf/jsonpb"
+	"github.com/gogo/protobuf/proto"
 	"github.com/google/uuid"
 	"github.com/grafana/tempo/pkg/boundedwaitgroup"
 	"github.com/grafana/tempo/tempodb/backend"
@@ -124,8 +125,10 @@ func loadBlock(r backend.Reader, c backend.Compactor, tenantID string, id backen
 	}, nil
 }
 
-func printAsJSON[T any](value T) error {
-	traceJSON, err := json.Marshal(value)
+func printAsJSON(value proto.Message) error {
+	m := jsonpb.Marshaler{}
+
+	traceJSON, err := m.MarshalToString(value)
 	if err != nil {
 		return err
 	}

diff --git a/pkg/api/http.go b/pkg/api/http.go
@@ -456,10 +456,16 @@ func BuildQueryRangeRequest(req *http.Request, searchReq *tempopb.QueryRangeRequ
 		return req
 	}
 
+	// 0 is an invalid step, so we need to calculate it if it's not provided
+	step := searchReq.Step
+	if step == 0 {
+		step = traceql.DefaultQueryRangeStep(searchReq.Start, searchReq.End)
+	}
+
 	qb := newQueryBuilder("")
 	qb.addParam(urlParamStart, strconv.FormatUint(searchReq.Start, 10))
 	qb.addParam(urlParamEnd, strconv.FormatUint(searchReq.End, 10))
-	qb.addParam(urlParamStep, time.Duration(searchReq.Step).String())
+	qb.addParam(urlParamStep, time.Duration(step).String())
 	qb.addParam(QueryModeKey, searchReq.QueryMode)
 	// New RF1 params
 	qb.addParam(urlParamBlockID, searchReq.BlockID)

diff --git a/pkg/api/http_test.go b/pkg/api/http_test.go
@@ -704,7 +704,9 @@ func TestQueryRangeRoundtrip(t *testing.T) {
 	}{
 		{
 			name: "empty",
-			req:  &tempopb.QueryRangeRequest{},
+			req: &tempopb.QueryRangeRequest{
+				Step: uint64(time.Second), // you can't actually roundtrip an empty query b/c Build/Parse will force a default step
+			},
 		},
 		{
 			name: "not empty!",