Skip to content

Commit

Permalink
[TraceQL Metrics] Remove all obsolete settings and code for RF3 metri…
Browse files Browse the repository at this point in the history
…cs (#3995)

* 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)
  • Loading branch information
mdisibio authored and joe-elliott committed Aug 26, 2024
1 parent caebd39 commit ed864b2
Show file tree
Hide file tree
Showing 26 changed files with 241 additions and 1,477 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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)
Expand Down
3 changes: 0 additions & 3 deletions docs/sources/tempo/configuration/_index.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion integration/e2e/config-query-range.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ query_frontend:
query_ingesters_until: 0
metrics:
exemplars: true
rf1_read_path: true

distributor:
receivers:
Expand Down
16 changes: 0 additions & 16 deletions modules/frontend/combiner/metrics_query_range.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion modules/frontend/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
91 changes: 2 additions & 89 deletions modules/frontend/metrics_query_range_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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,
Expand Down Expand Up @@ -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)
}
Loading

0 comments on commit ed864b2

Please sign in to comment.