address pr comments

Author: zhxiaogg

modules/frontend/metrics_query_range_handler.go

@@ -51,6 +51,12 @@ func newQueryRangeStreamingGRPCHandler(cfg Config, next pipeline.AsyncRoundTripp
 			return err
 		}
 
+		// Normalize exemplars before combiner creation: 0 (unspecified) or above the configured
+		// maximum defaults to MaxExemplars.
+		if req.Exemplars == 0 || req.Exemplars > cfg.Metrics.Sharder.MaxExemplars {
+			req.Exemplars = cfg.Metrics.Sharder.MaxExemplars
+		}
+
 		traceql.AlignRequest(req)
 
 		// the end time cutoff is applied here because it has to be done before combiner creation
@@ -75,14 +81,6 @@ func newQueryRangeStreamingGRPCHandler(cfg Config, next pipeline.AsyncRoundTripp
 		tenant, _ := user.ExtractOrgID(ctx)
 		start := time.Now()
 
-		// Normalize exemplars before combiner creation: 0 (unspecified) or above the configured
-		// maximum defaults to MaxExemplars. The sharder applies the same normalization to sub-requests,
-		// but runs after the combiner is created, so we must mirror it here.
-		if cfg.Metrics.Sharder.MaxExemplars > 0 {
-			if req.Exemplars == 0 || int(req.Exemplars) > cfg.Metrics.Sharder.MaxExemplars {
-				req.Exemplars = uint32(cfg.Metrics.Sharder.MaxExemplars) //nolint:gosec // G115
-			}
-		}
 		var finalResponse *tempopb.QueryRangeResponse
 		c, err := combiner.NewTypedQueryRange(req, cfg.Metrics.Sharder.MaxResponseSeries)
 		if err != nil {
@@ -141,6 +139,12 @@ func newMetricsQueryRangeHTTPHandler(cfg Config, next pipeline.AsyncRoundTripper
 			return httpInvalidRequest(err), nil
 		}
 
+		// Normalize exemplars before combiner creation: 0 (unspecified) or above the configured
+		// maximum defaults to MaxExemplars.
+		if queryRangeReq.Exemplars == 0 || queryRangeReq.Exemplars > cfg.Metrics.Sharder.MaxExemplars {
+			queryRangeReq.Exemplars = cfg.Metrics.Sharder.MaxExemplars
+		}
+
 		traceql.AlignRequest(queryRangeReq)
 
 		// the end time cutoff is applied here because it has to be done before combiner creation
@@ -156,14 +160,6 @@ func newMetricsQueryRangeHTTPHandler(cfg Config, next pipeline.AsyncRoundTripper
 		}
 		req = api.BuildQueryRangeRequest(req, queryRangeReq, "")
 
-		// Normalize exemplars before combiner creation: 0 (unspecified) or above the configured
-		// maximum defaults to MaxExemplars. The sharder applies the same normalization to sub-requests,
-		// but runs after the combiner is created, so we must mirror it here.
-		if cfg.Metrics.Sharder.MaxExemplars > 0 {
-			if queryRangeReq.Exemplars == 0 || int(queryRangeReq.Exemplars) > cfg.Metrics.Sharder.MaxExemplars {
-				queryRangeReq.Exemplars = uint32(cfg.Metrics.Sharder.MaxExemplars) //nolint:gosec // G115
-			}
-		}
 		// build and use roundtripper
 		combiner, err := combiner.NewTypedQueryRange(queryRangeReq, cfg.Metrics.Sharder.MaxResponseSeries)
 		if err != nil {

modules/frontend/metrics_query_range_handler_test.go

@@ -592,6 +592,26 @@ func TestQueryRangeHandlerExemplarNormalization(t *testing.T) {
 		}
 	})
 
+	t.Run("client requests exemplars but MaxExemplars is zero disables them", func(t *testing.T) {
+		f := frontendWithSettings(t, &mockRoundTripper{
+			responseFn: func() proto.Message { return mockResp },
+		}, nil, nil, nil, func(c *Config, _ *overrides.Config) {
+			c.Metrics.Sharder.Interval = time.Hour
+			c.Metrics.Sharder.MaxExemplars = 0
+		})
+
+		httpResp := httptest.NewRecorder()
+		f.MetricsQueryRangeHandler.ServeHTTP(httpResp, makeRequest(5)) // client requests exemplars
+		require.Equal(t, 200, httpResp.Code)
+
+		actualResp := &tempopb.QueryRangeResponse{}
+		require.NoError(t, jsonpb.Unmarshal(httpResp.Body, actualResp))
+
+		for _, s := range actualResp.Series {
+			assert.Empty(t, s.Exemplars, "exemplars should be empty when MaxExemplars is zero even if client requests them")
+		}
+	})
+
 	t.Run("client-specified exemplars capped to cfg.MaxExemplars", func(t *testing.T) {
 		f := frontendWithSettings(t, &mockRoundTripper{
 			responseFn: func() proto.Message { return mockResp },

modules/frontend/metrics_query_range_sharder.go

@@ -48,7 +48,7 @@ type QueryRangeSharderConfig struct {
 	// QueryBackendAfter determines when to query backend storage vs ingesters only.
 	QueryBackendAfter time.Duration `yaml:"query_backend_after,omitempty"`
 	Interval          time.Duration `yaml:"interval,omitempty"`
-	MaxExemplars      int           `yaml:"max_exemplars,omitempty"`
+	MaxExemplars      uint32        `yaml:"max_exemplars,omitempty"`
 	MaxResponseSeries int           `yaml:"max_response_series,omitempty"`
 	StreamingShards   int           `yaml:"streaming_shards,omitempty"`
 }
@@ -115,16 +115,11 @@ func (s queryRangeSharder) RoundTrip(pipelineRequest pipeline.Request) (pipeline
 	}
 
 	traceql.AlignRequest(req)
-
-	var maxExemplars uint32
+	
 	// Instant queries must not compute exemplars
-	if !s.instantMode && s.cfg.MaxExemplars > 0 {
-		maxExemplars = req.Exemplars
-		if maxExemplars == 0 || maxExemplars > uint32(s.cfg.MaxExemplars) {
-			maxExemplars = uint32(s.cfg.MaxExemplars) // Enforce configuration
-		}
+	if s.instantMode {
+		req.Exemplars = 0
 	}
-	req.Exemplars = maxExemplars
 
 	// if a limit is being enforced, honor the request if it is less than the limit
 	// else set it to max limit

modules/frontend/metrics_query_range_sharder_test.go

@@ -307,6 +307,17 @@ func TestExemplarsCutoff(t *testing.T) {
 		expectedBeforeCut uint32
 		expectedAfterCut  uint32
 	}{
+		{
+			// Instant queries zero exemplars before calling exemplarsCutoff; both sides must be 0.
+			name: "zero exemplars (instant mode) spanning cutoff",
+			req: tempopb.QueryRangeRequest{
+				Start:     uint64(cutoff.Add(-20 * time.Minute).UnixNano()),
+				End:       uint64(now.UnixNano()),
+				Exemplars: 0,
+			},
+			expectedBeforeCut: 0,
+			expectedAfterCut:  0,
+		},
 		{
 			// When all data is after the cutoff, all exemplars should go to the 'after' portion
 			name: "all data after cutoff",

pkg/traceql/engine_metrics_average.go

@@ -202,7 +202,7 @@ type averageSeries struct {
 	Exemplars []Exemplar
 }
 
-func newAverageSeries(l int, lenExemplars int, labels Labels) averageSeries {
+func newAverageSeries(l int, lenExemplars uint32, labels Labels) averageSeries {
 	s := averageSeries{
 		values:    make([]averageValue, l),
 		labels:    labels,
@@ -301,7 +301,7 @@ func (b *averageOverTimeSeriesAggregator) Combine(in []*tempopb.TimeSeries) {
 			countPosMapper[key] = i
 		} else if !ok {
 			lbls := getLabels(ts.Labels, "")
-			s := newAverageSeries(b.len, len(ts.Exemplars), lbls)
+			s := newAverageSeries(b.len, uint32(len(ts.Exemplars)), lbls)
 			b.weightedAverageSeries[key] = &s
 		}
 	}
@@ -583,7 +583,7 @@ func (g *avgOverTimeSpanAggregator[F, S]) getSeries(span Span) avgOverTimeSeries
 		intervals := g.intervalMapper.IntervalCount()
 		s = avgOverTimeSeries[S]{
 			vals:            g.buf.vals,
-			average:         newAverageSeries(intervals, int(g.exemplars), nil),
+			average:         newAverageSeries(intervals, g.exemplars, nil),
 			exemplarBuckets: newExemplarBucketSet(g.exemplars, g.start, g.end, g.step, g.instant),
 			initialized:     true,
 		}