[TraceQL Metrics] Remove all obsolete settings and code for RF3 metri…

…cs (#3995) (#4010) * Remove all obsolete settings and code for RF3 metrics. The only path going forward is RF1-based * Remove remaining references to rf1_read_path * changelog * Remove another reference to obsolete sampling rate * Update error messages for correctness (cherry picked from commit 43d9326) Co-authored-by: Martin Disibio <[email protected]>
grafana · Aug 27, 2024 · 3e2a967 · 3e2a967
1 parent caebd39
commit 3e2a967
Show file tree

Hide file tree

Showing 26 changed files with 241 additions and 1,477 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,6 @@
 ## main / unreleased
 
+* [FEATURE] TraceQL support for instrumentation scope [#3967](https://github.com/grafana/tempo/pull/3967) (@ie-pham)
 * [ENHANCEMENT] Add bytes and spans received to usage stats [#3983](https://github.com/grafana/tempo/pull/3983) (@joe-elliott)
 
 # v2.6.0-rc.0
@@ -18,6 +19,7 @@
 * [FEATURE] TraceQL support for event:timeSinceStart [#3908](https://github.com/grafana/tempo/pull/3908) (@ie-pham)
 * [FEATURE] Autocomplete support for events and links [#3846](https://github.com/grafana/tempo/pull/3846) (@ie-pham)
 * [FEATURE] Flush and query RF1 blocks for TraceQL metric queries [#3628](https://github.com/grafana/tempo/pull/3628) [#3691](https://github.com/grafana/tempo/pull/3691) [#3723](https://github.com/grafana/tempo/pull/3723) (@mapno)
+* [FEATURE] Flush and query RF1 blocks for TraceQL metric queries [#3628](https://github.com/grafana/tempo/pull/3628) [#3691](https://github.com/grafana/tempo/pull/3691) [#3723](https://github.com/grafana/tempo/pull/3723) (@mapno) [#3995](https://github.com/grafana/tempo/pull/3995) (@mdisibio)
 * [FEATURE] Add new compare() metrics function [#3695](https://github.com/grafana/tempo/pull/3695) (@mdisibio)
 * [FEATURE] Add new api `/api/metrics/query` for instant metrics queries [#3859](https://github.com/grafana/tempo/pull/3859) (@mdisibio)
 * [FEATURE] Add a `q` parameter to `/api/v2/serach/tags` for tag name filtering [#3822](https://github.com/grafana/tempo/pull/3822) (@joe-elliott)

diff --git a/docs/sources/tempo/configuration/_index.md b/docs/sources/tempo/configuration/_index.md
@@ -654,9 +654,6 @@ query_frontend:
         # If set to a non-zero value, it's value will be used to decide if query is within SLO or not.
         # Query is within SLO if it returned 200 within duration_slo seconds OR processed throughput_slo bytes/s data.
         [throughput_bytes_slo: <float> | default = 0 ]
-
-        # If set to true, TraceQL metric queries will use RF1 blocks built and flushed by the metrics-generator.
-        [rf1_read_path: <bool> | default = false]
 ```
 
 ## Querier

diff --git a/integration/e2e/config-query-range.yaml b/integration/e2e/config-query-range.yaml
@@ -11,7 +11,6 @@ query_frontend:
     query_ingesters_until: 0
   metrics:
     exemplars: true
-    rf1_read_path: true
 
 distributor:
   receivers:

diff --git a/modules/frontend/combiner/metrics_query_range.go b/modules/frontend/combiner/metrics_query_range.go
@@ -31,22 +31,6 @@ func NewQueryRange(req *tempopb.QueryRangeRequest, trackDiffs bool) (Combiner, e
 				}
 			}
 
-			samplingRate := resp.RequestData()
-			if samplingRate != nil {
-				fRate := samplingRate.(float64)
-
-				if fRate <= 1.0 {
-					// Set final sampling rate after integer rounding
-					// Multiply up the sampling rate
-					for _, series := range partial.Series {
-						for i, sample := range series.Samples {
-							sample.Value *= 1.0 / fRate
-							series.Samples[i] = sample
-						}
-					}
-				}
-			}
-
 			combiner.Combine(partial)
 
 			return nil

diff --git a/modules/frontend/config.go b/modules/frontend/config.go
@@ -90,7 +90,6 @@ func (cfg *Config) RegisterFlagsAndApplyDefaults(string, *flag.FlagSet) {
 			ConcurrentRequests:    defaultConcurrentRequests,
 			TargetBytesPerRequest: defaultTargetBytesPerRequest,
 			Interval:              5 * time.Minute,
-			RF1ReadPath:           false,
 			Exemplars:             false, // TODO: Remove?
 			MaxExemplars:          100,
 		},

diff --git a/modules/frontend/metrics_query_range_handler_test.go b/modules/frontend/metrics_query_range_handler_test.go
@@ -47,6 +47,7 @@ func TestQueryRangeHandlerSucceeds(t *testing.T) {
 	}, nil, nil, nil, func(c *Config) {
 		c.Metrics.Sharder.Interval = time.Hour
 	})
+
 	tenant := "foo"
 
 	httpReq := httptest.NewRequest("GET", api.PathMetricsQueryRange, nil)
@@ -66,10 +67,9 @@ func TestQueryRangeHandlerSucceeds(t *testing.T) {
 
 	require.Equal(t, 200, httpResp.Code)
 
-	// for reasons I don't understand, this query turns into 408 jobs.
 	expectedResp := &tempopb.QueryRangeResponse{
 		Metrics: &tempopb.SearchMetrics{
-			CompletedJobs:   4,
+			CompletedJobs:   4, // 2 blocks, each with 2 row groups that take 1 job
 			InspectedTraces: 4,
 			InspectedBytes:  4,
 			TotalJobs:       4,
@@ -101,90 +101,3 @@ func TestQueryRangeHandlerSucceeds(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, expectedResp, actualResp)
 }
-
-func TestQueryRangeHandlerRespectsSamplingRate(t *testing.T) {
-	resp := &tempopb.QueryRangeResponse{
-		Metrics: &tempopb.SearchMetrics{
-			InspectedTraces: 1,
-			InspectedBytes:  1,
-		},
-		Series: []*tempopb.TimeSeries{
-			{
-				PromLabels: "foo",
-				Labels: []v1.KeyValue{
-					{Key: "foo", Value: &v1.AnyValue{Value: &v1.AnyValue_StringValue{StringValue: "bar"}}},
-				},
-				Samples: []tempopb.Sample{
-					{
-						TimestampMs: 1200_000,
-						Value:       2,
-					},
-					{
-						TimestampMs: 1100_000,
-						Value:       1,
-					},
-				},
-			},
-		},
-	}
-
-	f := frontendWithSettings(t, &mockRoundTripper{
-		responseFn: func() proto.Message {
-			return resp
-		},
-	}, nil, nil, nil, func(c *Config) {
-		c.Metrics.Sharder.Interval = time.Hour
-	})
-	tenant := "foo"
-
-	httpReq := httptest.NewRequest("GET", api.PathMetricsQueryRange, nil)
-	httpReq = api.BuildQueryRangeRequest(httpReq, &tempopb.QueryRangeRequest{
-		Query: "{} | rate() with (sample=.2)",
-		Start: uint64(1100 * time.Second),
-		End:   uint64(1200 * time.Second),
-		Step:  uint64(100 * time.Second),
-	})
-
-	ctx := user.InjectOrgID(httpReq.Context(), tenant)
-	httpReq = httpReq.WithContext(ctx)
-
-	httpResp := httptest.NewRecorder()
-
-	f.MetricsQueryRangeHandler.ServeHTTP(httpResp, httpReq)
-
-	require.Equal(t, 200, httpResp.Code)
-
-	expectedResp := &tempopb.QueryRangeResponse{
-		Metrics: &tempopb.SearchMetrics{
-			CompletedJobs:   1,
-			InspectedTraces: 1,
-			InspectedBytes:  1,
-			TotalJobs:       1,
-			TotalBlocks:     2,
-			TotalBlockBytes: 419430400,
-		},
-		Series: []*tempopb.TimeSeries{
-			{
-				PromLabels: "foo",
-				Labels: []v1.KeyValue{
-					{Key: "foo", Value: &v1.AnyValue{Value: &v1.AnyValue_StringValue{StringValue: "bar"}}},
-				},
-				Samples: []tempopb.Sample{
-					{
-						TimestampMs: 1100_000,
-						Value:       5,
-					},
-					{
-						TimestampMs: 1200_000,
-						Value:       10,
-					},
-				},
-			},
-		},
-	}
-
-	actualResp := &tempopb.QueryRangeResponse{}
-	err := jsonpb.Unmarshal(httpResp.Body, actualResp)
-	require.NoError(t, err)
-	require.Equal(t, expectedResp, actualResp)
-}