Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TraceQL Metrics] Remove all obsolete settings and code for RF3 metrics #3995

Merged
merged 5 commits into from
Aug 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +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] TraceQL support for instrumentation scope [#3967](https://github.com/grafana/tempo/pull/3967) (@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
Loading