From 0d74172911f2fe5e6e0719a415b916ac80a947b1 Mon Sep 17 00:00:00 2001 From: Kemal Akkoyun Date: Wed, 9 Sep 2020 18:54:05 +0300 Subject: [PATCH 1/7] Limit the default time window for label name and label value APIs Signed-off-by: Kemal Akkoyun --- CHANGELOG.md | 1 + cmd/thanos/query.go | 6 ++++++ docs/components/query.md | 4 ++++ pkg/api/query/v1.go | 20 ++++++++++++++------ 4 files changed, 25 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ac497f176c..a4e83f1f21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re - [#3133](https://github.com/thanos-io/thanos/pull/3133) Query: Allow passing a `storeMatch[]` to Labels APIs. Also time range metadata based store filtering is supported on Labels APIs. - [#3154](https://github.com/thanos-io/thanos/pull/3154) Query Frontend: Add metric `thanos_memcached_getmulti_gate_queries_max`. - [#3146](https://github.com/thanos-io/thanos/pull/3146) Sidecar: Add `thanos_sidecar_prometheus_store_received_frames` histogram metric. +- [#3147](https://github.com/thanos-io/thanos/pull/3147) Querier: Add `query.labels.lookback-delta` flag to specify the default lookback duration for retrieving labels through Labels APIs when time range parameters are not specified. ### Changed diff --git a/cmd/thanos/query.go b/cmd/thanos/query.go index f3999e350e..823e175fa5 100644 --- a/cmd/thanos/query.go +++ b/cmd/thanos/query.go @@ -23,6 +23,7 @@ import ( "github.com/prometheus/prometheus/discovery/targetgroup" "github.com/prometheus/prometheus/pkg/labels" "github.com/prometheus/prometheus/promql" + "github.com/thanos-io/thanos/pkg/extkingpin" v1 "github.com/thanos-io/thanos/pkg/api/query" @@ -81,6 +82,8 @@ func registerQuery(app *extkingpin.App) { instantDefaultMaxSourceResolution := modelDuration(cmd.Flag("query.instant.default.max_source_resolution", "default value for max_source_resolution for instant queries. If not set, defaults to 0s only taking raw resolution into account. 1h can be a good value if you use instant queries over time ranges that incorporate times outside of your raw-retention.").Default("0s").Hidden()) + defaultLabelLookbackDelta := cmd.Flag("query.labels.lookback-delta", "The default lookback duration for retrieving labels through Labels API when the range parameters are not specified.").Default("2h").Duration() + selectorLabels := cmd.Flag("selector-label", "Query selector labels that will be exposed in info endpoint (repeated)."). PlaceHolder("=\"\"").Strings() @@ -193,6 +196,7 @@ func registerQuery(app *extkingpin.App) { *dnsSDResolver, time.Duration(*unhealthyStoreTimeout), time.Duration(*instantDefaultMaxSourceResolution), + time.Duration(*defaultLabelLookbackDelta), *strictStores, component.Query, ) @@ -241,6 +245,7 @@ func runQuery( dnsSDResolver string, unhealthyStoreTimeout time.Duration, instantDefaultMaxSourceResolution time.Duration, + defaultLabelLookbackDelta time.Duration, strictStores []string, comp component.Component, ) error { @@ -442,6 +447,7 @@ func runQuery( queryReplicaLabels, flagsMap, instantDefaultMaxSourceResolution, + defaultLabelLookbackDelta, gate.New( extprom.WrapRegistererWithPrefix("thanos_query_concurrent_", reg), maxConcurrentQueries, diff --git a/docs/components/query.md b/docs/components/query.md index 8a780fee78..95ad729c2c 100644 --- a/docs/components/query.md +++ b/docs/components/query.md @@ -389,6 +389,10 @@ Flags: able to query without deduplication using 'dedup=false' parameter. Data includes time series, recording rules, and alerting rules. + --query.labels.lookback-delta=2h + The default lookback duration for retrieving + labels through Labels API when the range + parameters are not specified. --selector-label=="" ... Query selector labels that will be exposed in info endpoint (repeated). diff --git a/pkg/api/query/v1.go b/pkg/api/query/v1.go index b674898ad6..3316983585 100644 --- a/pkg/api/query/v1.go +++ b/pkg/api/query/v1.go @@ -70,10 +70,12 @@ type QueryAPI struct { enableAutodownsampling bool enableQueryPartialResponse bool enableRulePartialResponse bool - replicaLabels []string - storeSet *query.StoreSet + replicaLabels []string + storeSet *query.StoreSet + defaultInstantQueryMaxSourceResolution time.Duration + defaultLabelLookbackDelta time.Duration } // NewQueryAPI returns an initialized QueryAPI type. @@ -89,6 +91,7 @@ func NewQueryAPI( replicaLabels []string, flagsMap map[string]string, defaultInstantQueryMaxSourceResolution time.Duration, + defaultLabelLookbackDelta time.Duration, gate gate.Gate, ) *QueryAPI { return &QueryAPI{ @@ -105,6 +108,7 @@ func NewQueryAPI( replicaLabels: replicaLabels, storeSet: storeSet, defaultInstantQueryMaxSourceResolution: defaultInstantQueryMaxSourceResolution, + defaultLabelLookbackDelta: defaultLabelLookbackDelta, } } @@ -418,11 +422,13 @@ func (qapi *QueryAPI) labelValues(r *http.Request) (interface{}, []error, *api.A return nil, nil, &api.ApiError{Typ: api.ErrorBadData, Err: errors.Errorf("invalid label name: %q", name)} } - start, err := parseTimeParam(r, "start", minTime) + // If start and end time not specified, we get the last 2h window by default. + now := time.Now() + start, err := parseTimeParam(r, "start", now.Add(-qapi.defaultLabelLookbackDelta)) if err != nil { return nil, nil, &api.ApiError{Typ: api.ErrorBadData, Err: err} } - end, err := parseTimeParam(r, "end", maxTime) + end, err := parseTimeParam(r, "end", now) if err != nil { return nil, nil, &api.ApiError{Typ: api.ErrorBadData, Err: err} } @@ -584,11 +590,13 @@ func parseDuration(s string) (time.Duration, error) { func (qapi *QueryAPI) labelNames(r *http.Request) (interface{}, []error, *api.ApiError) { ctx := r.Context() - start, err := parseTimeParam(r, "start", minTime) + // If start and end time not specified, we get the last 2h window by default. + now := time.Now() + start, err := parseTimeParam(r, "start", now.Add(-qapi.defaultLabelLookbackDelta)) if err != nil { return nil, nil, &api.ApiError{Typ: api.ErrorBadData, Err: err} } - end, err := parseTimeParam(r, "end", maxTime) + end, err := parseTimeParam(r, "end", now) if err != nil { return nil, nil, &api.ApiError{Typ: api.ErrorBadData, Err: err} } From 9ba0e09b3bed20871733430757983bbc6708ff85 Mon Sep 17 00:00:00 2001 From: Kemal Akkoyun Date: Mon, 14 Sep 2020 17:33:56 +0300 Subject: [PATCH 2/7] Keep the existing behaviour as default Signed-off-by: Kemal Akkoyun --- cmd/thanos/query.go | 2 +- docs/components/query.md | 6 +- pkg/api/query/v1.go | 143 ++++++++++++++++++++------------------- 3 files changed, 80 insertions(+), 71 deletions(-) diff --git a/cmd/thanos/query.go b/cmd/thanos/query.go index 823e175fa5..a8526b0f2c 100644 --- a/cmd/thanos/query.go +++ b/cmd/thanos/query.go @@ -82,7 +82,7 @@ func registerQuery(app *extkingpin.App) { instantDefaultMaxSourceResolution := modelDuration(cmd.Flag("query.instant.default.max_source_resolution", "default value for max_source_resolution for instant queries. If not set, defaults to 0s only taking raw resolution into account. 1h can be a good value if you use instant queries over time ranges that incorporate times outside of your raw-retention.").Default("0s").Hidden()) - defaultLabelLookbackDelta := cmd.Flag("query.labels.lookback-delta", "The default lookback duration for retrieving labels through Labels API when the range parameters are not specified.").Default("2h").Duration() + defaultLabelLookbackDelta := cmd.Flag("query.labels.api-lookback-delta", "The default lookback duration for retrieving labels through Labels API when the range parameters are not specified. The zero value means range covers the time since the beginning.").Default("0s").Duration() selectorLabels := cmd.Flag("selector-label", "Query selector labels that will be exposed in info endpoint (repeated)."). PlaceHolder("=\"\"").Strings() diff --git a/docs/components/query.md b/docs/components/query.md index 95ad729c2c..29425f1040 100644 --- a/docs/components/query.md +++ b/docs/components/query.md @@ -389,10 +389,12 @@ Flags: able to query without deduplication using 'dedup=false' parameter. Data includes time series, recording rules, and alerting rules. - --query.labels.lookback-delta=2h + --query.labels.api-lookback-delta=0s The default lookback duration for retrieving labels through Labels API when the range - parameters are not specified. + parameters are not specified. The zero value + means range covers the time since the + beginning. --selector-label=="" ... Query selector labels that will be exposed in info endpoint (repeated). diff --git a/pkg/api/query/v1.go b/pkg/api/query/v1.go index 3316983585..a581bf9da7 100644 --- a/pkg/api/query/v1.go +++ b/pkg/api/query/v1.go @@ -422,20 +422,10 @@ func (qapi *QueryAPI) labelValues(r *http.Request) (interface{}, []error, *api.A return nil, nil, &api.ApiError{Typ: api.ErrorBadData, Err: errors.Errorf("invalid label name: %q", name)} } - // If start and end time not specified, we get the last 2h window by default. - now := time.Now() - start, err := parseTimeParam(r, "start", now.Add(-qapi.defaultLabelLookbackDelta)) + start, end, err := parseLabelTimeRange(r, qapi.defaultLabelLookbackDelta) if err != nil { return nil, nil, &api.ApiError{Typ: api.ErrorBadData, Err: err} } - end, err := parseTimeParam(r, "end", now) - if err != nil { - return nil, nil, &api.ApiError{Typ: api.ErrorBadData, Err: err} - } - if end.Before(start) { - err := errors.New("end timestamp must not be before start time") - return nil, nil, &api.ApiError{Typ: api.ErrorBadData, Err: err} - } enablePartialResponse, apiErr := qapi.parsePartialResponseParam(r, qapi.enableQueryPartialResponse) if apiErr != nil { @@ -468,11 +458,6 @@ func (qapi *QueryAPI) labelValues(r *http.Request) (interface{}, []error, *api.A return vals, warnings, nil } -var ( - minTime = time.Unix(math.MinInt64/1000+62135596801, 0) - maxTime = time.Unix(math.MaxInt64/1000-62135596801, 999999999) -) - func (qapi *QueryAPI) series(r *http.Request) (interface{}, []error, *api.ApiError) { if err := r.ParseForm(); err != nil { return nil, nil, &api.ApiError{Typ: api.ErrorInternal, Err: errors.Wrap(err, "parse form")} @@ -549,61 +534,11 @@ func (qapi *QueryAPI) series(r *http.Request) (interface{}, []error, *api.ApiErr return metrics, set.Warnings(), nil } -func parseTimeParam(r *http.Request, paramName string, defaultValue time.Time) (time.Time, error) { - val := r.FormValue(paramName) - if val == "" { - return defaultValue, nil - } - result, err := parseTime(val) - if err != nil { - return time.Time{}, errors.Wrapf(err, "Invalid time value for '%s'", paramName) - } - return result, nil -} - -func parseTime(s string) (time.Time, error) { - if t, err := strconv.ParseFloat(s, 64); err == nil { - s, ns := math.Modf(t) - ns = math.Round(ns*1000) / 1000 - return time.Unix(int64(s), int64(ns*float64(time.Second))), nil - } - if t, err := time.Parse(time.RFC3339Nano, s); err == nil { - return t, nil - } - return time.Time{}, errors.Errorf("cannot parse %q to a valid timestamp", s) -} - -func parseDuration(s string) (time.Duration, error) { - if d, err := strconv.ParseFloat(s, 64); err == nil { - ts := d * float64(time.Second) - if ts > float64(math.MaxInt64) || ts < float64(math.MinInt64) { - return 0, errors.Errorf("cannot parse %q to a valid duration. It overflows int64", s) - } - return time.Duration(ts), nil - } - if d, err := model.ParseDuration(s); err == nil { - return time.Duration(d), nil - } - return 0, errors.Errorf("cannot parse %q to a valid duration", s) -} - func (qapi *QueryAPI) labelNames(r *http.Request) (interface{}, []error, *api.ApiError) { - ctx := r.Context() - - // If start and end time not specified, we get the last 2h window by default. - now := time.Now() - start, err := parseTimeParam(r, "start", now.Add(-qapi.defaultLabelLookbackDelta)) - if err != nil { - return nil, nil, &api.ApiError{Typ: api.ErrorBadData, Err: err} - } - end, err := parseTimeParam(r, "end", now) + start, end, err := parseLabelTimeRange(r, qapi.defaultLabelLookbackDelta) if err != nil { return nil, nil, &api.ApiError{Typ: api.ErrorBadData, Err: err} } - if end.Before(start) { - err := errors.New("end timestamp must not be before start time") - return nil, nil, &api.ApiError{Typ: api.ErrorBadData, Err: err} - } enablePartialResponse, apiErr := qapi.parsePartialResponseParam(r, qapi.enableQueryPartialResponse) if apiErr != nil { @@ -615,7 +550,8 @@ func (qapi *QueryAPI) labelNames(r *http.Request) (interface{}, []error, *api.Ap return nil, nil, apiErr } - q, err := qapi.queryableCreate(true, nil, storeMatchers, 0, enablePartialResponse, false).Querier(ctx, timestamp.FromTime(start), timestamp.FromTime(end)) + q, err := qapi.queryableCreate(true, nil, storeMatchers, 0, enablePartialResponse, false). + Querier(r.Context(), timestamp.FromTime(start), timestamp.FromTime(end)) if err != nil { return nil, nil, &api.ApiError{Typ: api.ErrorExec, Err: err} } @@ -667,3 +603,74 @@ func NewRulesHandler(client rules.UnaryClient, enablePartialResponse bool) func( return groups, warnings, nil } } + +var ( + minTime = time.Unix(math.MinInt64/1000+62135596801, 0) + maxTime = time.Unix(math.MaxInt64/1000-62135596801, 999999999) +) + +func parseLabelTimeRange(r *http.Request, defaultLabelLookbackDelta time.Duration) (time.Time, time.Time, error) { + // If start and end time not specified as query parameter, we get the range from the beginning of time by default. + var defaultStart, defaultEnd time.Time + if defaultLabelLookbackDelta == 0 { + defaultStart = minTime + defaultEnd = maxTime + } else { + now := time.Now() + defaultStart = now.Add(-defaultLabelLookbackDelta) + defaultEnd = now + } + + start, err := parseTimeParam(r, "start", defaultStart) + if err != nil { + return time.Time{}, time.Time{}, &api.ApiError{Typ: api.ErrorBadData, Err: err} + } + end, err := parseTimeParam(r, "end", defaultEnd) + if err != nil { + return time.Time{}, time.Time{}, &api.ApiError{Typ: api.ErrorBadData, Err: err} + } + if end.Before(start) { + err := errors.New("end timestamp must not be before start time") + return time.Time{}, time.Time{}, &api.ApiError{Typ: api.ErrorBadData, Err: err} + } + + return start, end, nil +} + +func parseTimeParam(r *http.Request, paramName string, defaultValue time.Time) (time.Time, error) { + val := r.FormValue(paramName) + if val == "" { + return defaultValue, nil + } + result, err := parseTime(val) + if err != nil { + return time.Time{}, errors.Wrapf(err, "Invalid time value for '%s'", paramName) + } + return result, nil +} + +func parseTime(s string) (time.Time, error) { + if t, err := strconv.ParseFloat(s, 64); err == nil { + s, ns := math.Modf(t) + ns = math.Round(ns*1000) / 1000 + return time.Unix(int64(s), int64(ns*float64(time.Second))), nil + } + if t, err := time.Parse(time.RFC3339Nano, s); err == nil { + return t, nil + } + return time.Time{}, errors.Errorf("cannot parse %q to a valid timestamp", s) +} + +func parseDuration(s string) (time.Duration, error) { + if d, err := strconv.ParseFloat(s, 64); err == nil { + ts := d * float64(time.Second) + if ts > float64(math.MaxInt64) || ts < float64(math.MinInt64) { + return 0, errors.Errorf("cannot parse %q to a valid duration. It overflows int64", s) + } + return time.Duration(ts), nil + } + if d, err := model.ParseDuration(s); err == nil { + return time.Duration(d), nil + } + return 0, errors.Errorf("cannot parse %q to a valid duration", s) +} From c3f31938990fec0e8004b35d15922443db034a5b Mon Sep 17 00:00:00 2001 From: Kemal Akkoyun Date: Thu, 17 Sep 2020 18:55:14 +0300 Subject: [PATCH 3/7] Add label API endpoint tests Signed-off-by: Kemal Akkoyun --- pkg/api/query/v1_test.go | 452 +++++++++++++++++++++-------- pkg/testutil/e2eutil/prometheus.go | 3 +- 2 files changed, 332 insertions(+), 123 deletions(-) diff --git a/pkg/api/query/v1_test.go b/pkg/api/query/v1_test.go index d3ae8dc0d7..324f0dea5b 100644 --- a/pkg/api/query/v1_test.go +++ b/pkg/api/query/v1_test.go @@ -21,6 +21,8 @@ import ( "encoding/json" "fmt" "io" + "io/ioutil" + "math" "math/rand" "net/http" "net/url" @@ -37,6 +39,7 @@ import ( "github.com/prometheus/prometheus/promql/parser" "github.com/prometheus/prometheus/rules" "github.com/prometheus/prometheus/storage" + "github.com/prometheus/prometheus/tsdb" baseAPI "github.com/thanos-io/thanos/pkg/api" "github.com/thanos-io/thanos/pkg/compact" @@ -55,6 +58,63 @@ func TestMain(m *testing.M) { testutil.TolerantVerifyLeakMain(m) } +type endpointTestCase struct { + endpoint baseAPI.ApiFunc + params map[string]string + query url.Values + method string + response interface{} + errType baseAPI.ErrorType +} + +func testEndpoint(t *testing.T, test endpointTestCase, name string) bool { + return t.Run(name, func(t *testing.T) { + // Build a context with the correct request params. + ctx := context.Background() + for p, v := range test.params { + ctx = route.WithParam(ctx, p, v) + } + + reqURL := "http://example.com" + params := test.query.Encode() + + var body io.Reader + if test.method == http.MethodPost { + body = strings.NewReader(params) + } else if test.method == "" { + test.method = "ANY" + reqURL += "?" + params + } + + req, err := http.NewRequest(test.method, reqURL, body) + if err != nil { + t.Fatal(err) + } + + if body != nil { + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + } + + resp, _, apiErr := test.endpoint(req.WithContext(ctx)) + if apiErr != nil { + if test.errType == baseAPI.ErrorNone { + t.Fatalf("Unexpected error: %s", apiErr) + } + if test.errType != apiErr.Typ { + t.Fatalf("Expected error of type %q but got type %q", test.errType, apiErr.Typ) + } + return + } + if test.errType != baseAPI.ErrorNone { + t.Fatalf("Expected error of type %q but got none", test.errType) + } + + if !reflect.DeepEqual(resp, test.response) { + t.Fatalf("Response does not match, expected:\n%+v\ngot:\n%+v", test.response, resp) + } + }) +} + func TestEndpoints(t *testing.T) { lbls := []labels.Labels{ { @@ -122,14 +182,7 @@ func TestEndpoints(t *testing.T) { start := time.Unix(0, 0) - var tests = []struct { - endpoint baseAPI.ApiFunc - params map[string]string - query url.Values - method string - response interface{} - errType baseAPI.ErrorType - }{ + var tests = []endpointTestCase{ { endpoint: api.query, query: url.Values{ @@ -513,35 +566,6 @@ func TestEndpoints(t *testing.T) { }, errType: baseAPI.ErrorBadData, }, - { - endpoint: api.labelValues, - params: map[string]string{ - "name": "__name__", - }, - response: []string{ - "test_metric1", - "test_metric2", - "test_metric_replica1", - }, - }, - { - endpoint: api.labelValues, - params: map[string]string{ - "name": "foo", - }, - response: []string{ - "bar", - "boo", - }, - }, - // Bad name parameter. - { - endpoint: api.labelValues, - params: map[string]string{ - "name": "not!!!allowed", - }, - errType: baseAPI.ErrorBadData, - }, { endpoint: api.series, query: url.Values{ @@ -796,55 +820,239 @@ func TestEndpoints(t *testing.T) { }, } - for _, test := range tests { - if ok := t.Run(test.query.Encode(), func(t *testing.T) { - // Build a context with the correct request params. - ctx := context.Background() - for p, v := range test.params { - ctx = route.WithParam(ctx, p, v) - } + for i, test := range tests { + if ok := testEndpoint(t, test, fmt.Sprintf("#%d %s", i, test.query.Encode())); !ok { + return + } + } +} - reqURL := "http://example.com" - params := test.query.Encode() +func TestLabelEndpoints(t *testing.T) { + var old = []labels.Labels{ + { + labels.Label{Name: "__name__", Value: "test_metric1"}, + labels.Label{Name: "foo", Value: "bar"}, + }, + { + labels.Label{Name: "__name__", Value: "test_metric1"}, + labels.Label{Name: "foo", Value: "boo"}, + }, + { + labels.Label{Name: "__name__", Value: "test_metric2"}, + labels.Label{Name: "foo", Value: "boo"}, + }, + } - var body io.Reader - if test.method == http.MethodPost { - body = strings.NewReader(params) - } else if test.method == "" { - test.method = "ANY" - reqURL += "?" + params - } + var recent = []labels.Labels{ + { + labels.Label{Name: "__name__", Value: "test_metric_replica1"}, + labels.Label{Name: "foo", Value: "bar"}, + labels.Label{Name: "replica", Value: "a"}, + }, + { + labels.Label{Name: "__name__", Value: "test_metric_replica1"}, + labels.Label{Name: "foo", Value: "boo"}, + labels.Label{Name: "replica", Value: "a"}, + }, + { + labels.Label{Name: "__name__", Value: "test_metric_replica1"}, + labels.Label{Name: "foo", Value: "boo"}, + labels.Label{Name: "replica", Value: "b"}, + }, + { + labels.Label{Name: "__name__", Value: "test_metric_replica2"}, + labels.Label{Name: "foo", Value: "boo"}, + labels.Label{Name: "replica1", Value: "a"}, + }, + } - req, err := http.NewRequest(test.method, reqURL, body) - if err != nil { - t.Fatal(err) - } + dir, err := ioutil.TempDir("", "prometheus-test") + testutil.Ok(t, err) - if body != nil { - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - } + var ( + mint int64 = 0 + maxt int64 = 600_000 + ) + var metricSamples []*tsdb.MetricSample + for _, lbl := range old { + for i := int64(0); i < 10; i++ { + metricSamples = append(metricSamples, &tsdb.MetricSample{ + TimestampMs: i * 60_000, + Value: float64(i), + Labels: lbl, + }) + } + } - resp, _, apiErr := test.endpoint(req.WithContext(ctx)) - if apiErr != nil { - if test.errType == baseAPI.ErrorNone { - t.Fatalf("Unexpected error: %s", apiErr) - } - if test.errType != apiErr.Typ { - t.Fatalf("Expected error of type %q but got type %q", test.errType, apiErr.Typ) - } - return - } - if test.errType != baseAPI.ErrorNone { - t.Fatalf("Expected error of type %q but got none", test.errType) - } + _, err = tsdb.CreateBlock(metricSamples, dir, mint, maxt, nil) + testutil.Ok(t, err) - if !reflect.DeepEqual(resp, test.response) { - t.Fatalf("Response does not match, expected:\n%+v\ngot:\n%+v", test.response, resp) - } - }); !ok { - return + opts := tsdb.DefaultOptions() + opts.RetentionDuration = math.MaxInt64 + db, err := tsdb.Open(dir, nil, nil, opts) + defer func() { testutil.Ok(t, db.Close()) }() + testutil.Ok(t, err) + + var ( + apiLookbackDelta = 2 * time.Hour + start = time.Now().Add(-apiLookbackDelta).Unix() * 1000 + app = db.Appender(context.Background()) + ) + for _, lbl := range recent { + for i := int64(0); i < 10; i++ { + _, err := app.Add(lbl, start+(i*60_000), float64(i)) // ms + testutil.Ok(t, err) } + } + testutil.Ok(t, app.Commit()) + now := time.Now() + timeout := 100 * time.Second + api := &QueryAPI{ + baseAPI: &baseAPI.BaseAPI{ + Now: func() time.Time { return now }, + }, + queryableCreate: query.NewQueryableCreator(nil, nil, store.NewTSDBStore(nil, nil, db, component.Query, nil), 2, timeout), + queryEngine: promql.NewEngine(promql.EngineOpts{ + Logger: nil, + Reg: nil, + MaxSamples: 10000, + Timeout: timeout, + }), + gate: gate.New(nil, 4), + } + apiWithLabelLookback := &QueryAPI{ + baseAPI: &baseAPI.BaseAPI{ + Now: func() time.Time { return now }, + }, + queryableCreate: query.NewQueryableCreator(nil, nil, store.NewTSDBStore(nil, nil, db, component.Query, nil), 2, timeout), + queryEngine: promql.NewEngine(promql.EngineOpts{ + Logger: nil, + Reg: nil, + MaxSamples: 10000, + Timeout: timeout, + }), + gate: gate.New(nil, 4), + defaultLabelLookbackDelta: apiLookbackDelta, + } + + var tests = []endpointTestCase{ + { + endpoint: api.labelValues, + params: map[string]string{ + "name": "__name__", + }, + response: []string{ + "test_metric1", + "test_metric2", + "test_metric_replica1", + "test_metric_replica2", + }, + }, + { + endpoint: apiWithLabelLookback.labelValues, + params: map[string]string{ + "name": "__name__", + }, + response: []string{ + "test_metric_replica1", + "test_metric_replica2", + }, + }, + { + endpoint: api.labelValues, + query: url.Values{ + "start": []string{"1970-01-01T00:00:00Z"}, + "end": []string{"1970-01-01T00:09:00Z"}, + }, + params: map[string]string{ + "name": "__name__", + }, + response: []string{ + "test_metric1", + "test_metric2", + }, + }, + { + endpoint: apiWithLabelLookback.labelValues, + query: url.Values{ + "start": []string{"1970-01-01T00:00:00Z"}, + "end": []string{"1970-01-01T00:09:00Z"}, + }, + params: map[string]string{ + "name": "__name__", + }, + response: []string{ + "test_metric1", + "test_metric2", + }, + }, + { + endpoint: api.labelNames, + params: map[string]string{ + "name": "__name__", + }, + response: []string{ + "__name__", + "foo", + "replica", + "replica1", + }, + }, + { + endpoint: apiWithLabelLookback.labelNames, + params: map[string]string{ + "name": "foo", + }, + response: []string{ + "__name__", + "foo", + "replica", + "replica1", + }, + }, + { + endpoint: api.labelNames, + query: url.Values{ + "start": []string{"1970-01-01T00:00:00Z"}, + "end": []string{"1970-01-01T00:09:00Z"}, + }, + params: map[string]string{ + "name": "foo", + }, + response: []string{ + "__name__", + "foo", + }, + }, + { + endpoint: apiWithLabelLookback.labelNames, + query: url.Values{ + "start": []string{"1970-01-01T00:00:00Z"}, + "end": []string{"1970-01-01T00:09:00Z"}, + }, + params: map[string]string{ + "name": "foo", + }, + response: []string{ + "__name__", + "foo", + }, + }, + // Bad name parameter. + { + endpoint: api.labelValues, + params: map[string]string{ + "name": "not!!!allowed", + }, + errType: baseAPI.ErrorBadData, + }, + } + + for i, test := range tests { + if ok := testEndpoint(t, test, fmt.Sprintf("#%d %s", i, test.query.Encode())); !ok { + return + } } } @@ -957,40 +1165,6 @@ func TestParseDuration(t *testing.T) { } } -func BenchmarkQueryResultEncoding(b *testing.B) { - var mat promql.Matrix - for i := 0; i < 1000; i++ { - lset := labels.FromStrings( - "__name__", "my_test_metric_name", - "instance", fmt.Sprintf("abcdefghijklmnopqrstuvxyz-%d", i), - "job", "test-test", - "method", "ABCD", - "status", "199", - "namespace", "something", - "long-label", "34grnt83j0qxj309je9rgt9jf2jd-92jd-92jf9wrfjre", - ) - var points []promql.Point - for j := 0; j < b.N/1000; j++ { - points = append(points, promql.Point{ - T: int64(j * 10000), - V: rand.Float64(), - }) - } - mat = append(mat, promql.Series{ - Metric: lset, - Points: points, - }) - } - input := &queryData{ - ResultType: parser.ValueTypeMatrix, - Result: mat, - } - b.ResetTimer() - - _, err := json.Marshal(&input) - testutil.Ok(b, err) -} - func TestParseDownsamplingParamMillis(t *testing.T) { var tests = []struct { maxSourceResolutionParam string @@ -1119,16 +1293,6 @@ func TestParseStoreMatchersParam(t *testing.T) { } } -type mockedRulesClient struct { - g map[rulespb.RulesRequest_Type][]*rulespb.RuleGroup - w storage.Warnings - err error -} - -func (c mockedRulesClient) Rules(_ context.Context, req *rulespb.RulesRequest) (*rulespb.RuleGroups, storage.Warnings, error) { - return &rulespb.RuleGroups{Groups: c.g[req.Type]}, c.w, c.err -} - func TestRulesHandler(t *testing.T) { twoHAgo := time.Now().Add(-2 * time.Hour) all := []*rulespb.Rule{ @@ -1399,3 +1563,47 @@ func TestRulesHandler(t *testing.T) { }) } } + +func BenchmarkQueryResultEncoding(b *testing.B) { + var mat promql.Matrix + for i := 0; i < 1000; i++ { + lset := labels.FromStrings( + "__name__", "my_test_metric_name", + "instance", fmt.Sprintf("abcdefghijklmnopqrstuvxyz-%d", i), + "job", "test-test", + "method", "ABCD", + "status", "199", + "namespace", "something", + "long-label", "34grnt83j0qxj309je9rgt9jf2jd-92jd-92jf9wrfjre", + ) + var points []promql.Point + for j := 0; j < b.N/1000; j++ { + points = append(points, promql.Point{ + T: int64(j * 10000), + V: rand.Float64(), + }) + } + mat = append(mat, promql.Series{ + Metric: lset, + Points: points, + }) + } + input := &queryData{ + ResultType: parser.ValueTypeMatrix, + Result: mat, + } + b.ResetTimer() + + _, err := json.Marshal(&input) + testutil.Ok(b, err) +} + +type mockedRulesClient struct { + g map[rulespb.RulesRequest_Type][]*rulespb.RuleGroup + w storage.Warnings + err error +} + +func (c mockedRulesClient) Rules(_ context.Context, req *rulespb.RulesRequest) (*rulespb.RuleGroups, storage.Warnings, error) { + return &rulespb.RuleGroups{Groups: c.g[req.Type]}, c.w, c.err +} diff --git a/pkg/testutil/e2eutil/prometheus.go b/pkg/testutil/e2eutil/prometheus.go index acc4482d0d..447d65425f 100644 --- a/pkg/testutil/e2eutil/prometheus.go +++ b/pkg/testutil/e2eutil/prometheus.go @@ -30,10 +30,11 @@ import ( "github.com/prometheus/prometheus/storage" "github.com/prometheus/prometheus/tsdb" "github.com/prometheus/prometheus/tsdb/index" + "golang.org/x/sync/errgroup" + "github.com/thanos-io/thanos/pkg/block/metadata" "github.com/thanos-io/thanos/pkg/runutil" "github.com/thanos-io/thanos/pkg/testutil" - "golang.org/x/sync/errgroup" ) const ( From 1ebbb5c0a07ec76360330ed87f86fb4653696adb Mon Sep 17 00:00:00 2001 From: Kemal Akkoyun Date: Fri, 18 Sep 2020 14:19:40 +0300 Subject: [PATCH 4/7] Fix changelog entry Signed-off-by: Kemal Akkoyun --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a4e83f1f21..7cce5f93cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,7 +25,7 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re ### Changed - [#3136](https://github.com/thanos-io/thanos/pull/3136) Sidecar: Add metric `thanos_sidecar_reloader_config_apply_operations_total` and rename metric `thanos_sidecar_reloader_config_apply_errors_total` to `thanos_sidecar_reloader_config_apply_operations_failed_total`. -- [#3154](https://github.com/thanos-io/thanos/pull/3154) Query: Add metric `thanos_query_gate_queries_max`. Remove metric `thanos_query_concurrent_selects_gate_queries_in_flight`. +- [#3154](https://github.com/thanos-io/thanos/pull/3154) Querier: Add metric `thanos_query_gate_queries_max`. Remove metric `thanos_query_concurrent_selects_gate_queries_in_flight`. - [#3154](https://github.com/thanos-io/thanos/pull/3154) Store: Rename metric `thanos_bucket_store_queries_concurrent_max` to `thanos_bucket_store_series_gate_queries_max`. ## [v0.15.0](https://github.com/thanos-io/thanos/releases) - 2020.09.07 From de92bdc3aa06c2fa540ec14e8816c70874d660d4 Mon Sep 17 00:00:00 2001 From: Kemal Akkoyun Date: Mon, 21 Sep 2020 17:42:38 +0300 Subject: [PATCH 5/7] Address review issues Implement time ranges for series API Signed-off-by: Kemal Akkoyun --- cmd/thanos/query.go | 8 +- .../{query-frontend.go => query_frontend.go} | 3 +- docs/components/query.md | 12 +- pkg/api/query/v1.go | 48 +- pkg/api/query/v1_test.go | 556 ++++++++++-------- 5 files changed, 332 insertions(+), 295 deletions(-) rename cmd/thanos/{query-frontend.go => query_frontend.go} (99%) diff --git a/cmd/thanos/query.go b/cmd/thanos/query.go index a8526b0f2c..b8c7ec3127 100644 --- a/cmd/thanos/query.go +++ b/cmd/thanos/query.go @@ -82,7 +82,7 @@ func registerQuery(app *extkingpin.App) { instantDefaultMaxSourceResolution := modelDuration(cmd.Flag("query.instant.default.max_source_resolution", "default value for max_source_resolution for instant queries. If not set, defaults to 0s only taking raw resolution into account. 1h can be a good value if you use instant queries over time ranges that incorporate times outside of your raw-retention.").Default("0s").Hidden()) - defaultLabelLookbackDelta := cmd.Flag("query.labels.api-lookback-delta", "The default lookback duration for retrieving labels through Labels API when the range parameters are not specified. The zero value means range covers the time since the beginning.").Default("0s").Duration() + defaultMetadataTimeRange := cmd.Flag("query.metadata.default-time-range", "The default metadata time range duration for retrieving labels through Labels and Series API when the range parameters are not specified. The zero value means range covers the time since the beginning.").Default("0s").Duration() selectorLabels := cmd.Flag("selector-label", "Query selector labels that will be exposed in info endpoint (repeated)."). PlaceHolder("=\"\"").Strings() @@ -196,7 +196,7 @@ func registerQuery(app *extkingpin.App) { *dnsSDResolver, time.Duration(*unhealthyStoreTimeout), time.Duration(*instantDefaultMaxSourceResolution), - time.Duration(*defaultLabelLookbackDelta), + *defaultMetadataTimeRange, *strictStores, component.Query, ) @@ -245,7 +245,7 @@ func runQuery( dnsSDResolver string, unhealthyStoreTimeout time.Duration, instantDefaultMaxSourceResolution time.Duration, - defaultLabelLookbackDelta time.Duration, + defaultMetadataTimeRange time.Duration, strictStores []string, comp component.Component, ) error { @@ -447,7 +447,7 @@ func runQuery( queryReplicaLabels, flagsMap, instantDefaultMaxSourceResolution, - defaultLabelLookbackDelta, + defaultMetadataTimeRange, gate.New( extprom.WrapRegistererWithPrefix("thanos_query_concurrent_", reg), maxConcurrentQueries, diff --git a/cmd/thanos/query-frontend.go b/cmd/thanos/query_frontend.go similarity index 99% rename from cmd/thanos/query-frontend.go rename to cmd/thanos/query_frontend.go index fadc14053e..44d83036fa 100644 --- a/cmd/thanos/query-frontend.go +++ b/cmd/thanos/query_frontend.go @@ -17,6 +17,8 @@ import ( "github.com/opentracing/opentracing-go" "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" + "github.com/weaveworks/common/user" + "github.com/thanos-io/thanos/pkg/component" "github.com/thanos-io/thanos/pkg/extflag" "github.com/thanos-io/thanos/pkg/extkingpin" @@ -28,7 +30,6 @@ import ( httpserver "github.com/thanos-io/thanos/pkg/server/http" "github.com/thanos-io/thanos/pkg/server/http/middleware" "github.com/thanos-io/thanos/pkg/tracing" - "github.com/weaveworks/common/user" ) type queryFrontendConfig struct { diff --git a/docs/components/query.md b/docs/components/query.md index 29425f1040..5dfacab116 100644 --- a/docs/components/query.md +++ b/docs/components/query.md @@ -389,12 +389,12 @@ Flags: able to query without deduplication using 'dedup=false' parameter. Data includes time series, recording rules, and alerting rules. - --query.labels.api-lookback-delta=0s - The default lookback duration for retrieving - labels through Labels API when the range - parameters are not specified. The zero value - means range covers the time since the - beginning. + --query.metadata.default-time-range=0s + The default metadata time range duration for + retrieving labels through Labels and Series API + when the range parameters are not specified. + The zero value means range covers the time + since the beginning. --selector-label=="" ... Query selector labels that will be exposed in info endpoint (repeated). diff --git a/pkg/api/query/v1.go b/pkg/api/query/v1.go index a581bf9da7..47d4e8e8f5 100644 --- a/pkg/api/query/v1.go +++ b/pkg/api/query/v1.go @@ -75,7 +75,7 @@ type QueryAPI struct { storeSet *query.StoreSet defaultInstantQueryMaxSourceResolution time.Duration - defaultLabelLookbackDelta time.Duration + defaultMetadataTimeRange time.Duration } // NewQueryAPI returns an initialized QueryAPI type. @@ -91,7 +91,7 @@ func NewQueryAPI( replicaLabels []string, flagsMap map[string]string, defaultInstantQueryMaxSourceResolution time.Duration, - defaultLabelLookbackDelta time.Duration, + defaultMetadataTimeRange time.Duration, gate gate.Gate, ) *QueryAPI { return &QueryAPI{ @@ -108,7 +108,7 @@ func NewQueryAPI( replicaLabels: replicaLabels, storeSet: storeSet, defaultInstantQueryMaxSourceResolution: defaultInstantQueryMaxSourceResolution, - defaultLabelLookbackDelta: defaultLabelLookbackDelta, + defaultMetadataTimeRange: defaultMetadataTimeRange, } } @@ -422,7 +422,7 @@ func (qapi *QueryAPI) labelValues(r *http.Request) (interface{}, []error, *api.A return nil, nil, &api.ApiError{Typ: api.ErrorBadData, Err: errors.Errorf("invalid label name: %q", name)} } - start, end, err := parseLabelTimeRange(r, qapi.defaultLabelLookbackDelta) + start, end, err := parseMetadataTimeRange(r, qapi.defaultMetadataTimeRange) if err != nil { return nil, nil, &api.ApiError{Typ: api.ErrorBadData, Err: err} } @@ -467,18 +467,10 @@ func (qapi *QueryAPI) series(r *http.Request) (interface{}, []error, *api.ApiErr return nil, nil, &api.ApiError{Typ: api.ErrorBadData, Err: errors.New("no match[] parameter provided")} } - start, err := parseTimeParam(r, "start", minTime) + start, end, err := parseMetadataTimeRange(r, qapi.defaultMetadataTimeRange) if err != nil { return nil, nil, &api.ApiError{Typ: api.ErrorBadData, Err: err} } - end, err := parseTimeParam(r, "end", maxTime) - if err != nil { - return nil, nil, &api.ApiError{Typ: api.ErrorBadData, Err: err} - } - if end.Before(start) { - err := errors.New("end timestamp must not be before start time") - return nil, nil, &api.ApiError{Typ: api.ErrorBadData, Err: err} - } var matcherSets [][]*labels.Matcher for _, s := range r.Form["match[]"] { @@ -535,7 +527,7 @@ func (qapi *QueryAPI) series(r *http.Request) (interface{}, []error, *api.ApiErr } func (qapi *QueryAPI) labelNames(r *http.Request) (interface{}, []error, *api.ApiError) { - start, end, err := parseLabelTimeRange(r, qapi.defaultLabelLookbackDelta) + start, end, err := parseMetadataTimeRange(r, qapi.defaultMetadataTimeRange) if err != nil { return nil, nil, &api.ApiError{Typ: api.ErrorBadData, Err: err} } @@ -605,33 +597,35 @@ func NewRulesHandler(client rules.UnaryClient, enablePartialResponse bool) func( } var ( - minTime = time.Unix(math.MinInt64/1000+62135596801, 0) - maxTime = time.Unix(math.MaxInt64/1000-62135596801, 999999999) + infMinTime = time.Unix(math.MinInt64/1000+62135596801, 0) + infMaxTime = time.Unix(math.MaxInt64/1000-62135596801, 999999999) ) -func parseLabelTimeRange(r *http.Request, defaultLabelLookbackDelta time.Duration) (time.Time, time.Time, error) { +func parseMetadataTimeRange(r *http.Request, defaultMetadataTimeRange time.Duration) (time.Time, time.Time, error) { // If start and end time not specified as query parameter, we get the range from the beginning of time by default. - var defaultStart, defaultEnd time.Time - if defaultLabelLookbackDelta == 0 { - defaultStart = minTime - defaultEnd = maxTime + var defaultStartTime, defaultEndTime time.Time + if defaultMetadataTimeRange == 0 { + defaultStartTime = infMinTime + defaultEndTime = infMaxTime } else { now := time.Now() - defaultStart = now.Add(-defaultLabelLookbackDelta) - defaultEnd = now + defaultStartTime = now.Add(-defaultMetadataTimeRange) + defaultEndTime = now } - start, err := parseTimeParam(r, "start", defaultStart) + start, err := parseTimeParam(r, "start", defaultStartTime) if err != nil { return time.Time{}, time.Time{}, &api.ApiError{Typ: api.ErrorBadData, Err: err} } - end, err := parseTimeParam(r, "end", defaultEnd) + end, err := parseTimeParam(r, "end", defaultEndTime) if err != nil { return time.Time{}, time.Time{}, &api.ApiError{Typ: api.ErrorBadData, Err: err} } if end.Before(start) { - err := errors.New("end timestamp must not be before start time") - return time.Time{}, time.Time{}, &api.ApiError{Typ: api.ErrorBadData, Err: err} + return time.Time{}, time.Time{}, &api.ApiError{ + Typ: api.ErrorBadData, + Err: errors.New("end timestamp must not be before start time"), + } } return start, end, nil diff --git a/pkg/api/query/v1_test.go b/pkg/api/query/v1_test.go index 324f0dea5b..7102b9f28c 100644 --- a/pkg/api/query/v1_test.go +++ b/pkg/api/query/v1_test.go @@ -115,7 +115,7 @@ func testEndpoint(t *testing.T, test endpointTestCase, name string) bool { }) } -func TestEndpoints(t *testing.T) { +func TestQueryEndpoints(t *testing.T) { lbls := []labels.Labels{ { labels.Label{Name: "__name__", Value: "test_metric1"}, @@ -566,258 +566,6 @@ func TestEndpoints(t *testing.T) { }, errType: baseAPI.ErrorBadData, }, - { - endpoint: api.series, - query: url.Values{ - "match[]": []string{`test_metric2`}, - }, - response: []labels.Labels{ - labels.FromStrings("__name__", "test_metric2", "foo", "boo"), - }, - }, - // Series that does not exist should return an empty array. - { - endpoint: api.series, - query: url.Values{ - "match[]": []string{`foobar`}, - }, - response: []labels.Labels{}, - }, - { - endpoint: api.series, - query: url.Values{ - "match[]": []string{`test_metric1{foo=~".+o"}`}, - }, - response: []labels.Labels{ - labels.FromStrings("__name__", "test_metric1", "foo", "boo"), - }, - }, - { - endpoint: api.series, - query: url.Values{ - "match[]": []string{`test_metric1{foo=~".+o$"}`, `test_metric1{foo=~".+o"}`}, - }, - response: []labels.Labels{ - labels.FromStrings("__name__", "test_metric1", "foo", "boo"), - }, - }, - { - endpoint: api.series, - query: url.Values{ - "match[]": []string{`test_metric1{foo=~".+o"}`, `none`}, - }, - response: []labels.Labels{ - labels.FromStrings("__name__", "test_metric1", "foo", "boo"), - }, - }, - // Start and end before series starts. - { - endpoint: api.series, - query: url.Values{ - "match[]": []string{`test_metric2`}, - "start": []string{"-2"}, - "end": []string{"-1"}, - }, - response: []labels.Labels{}, - }, - // Start and end after series ends. - { - endpoint: api.series, - query: url.Values{ - "match[]": []string{`test_metric2`}, - "start": []string{"100000"}, - "end": []string{"100001"}, - }, - response: []labels.Labels{}, - }, - // Start before series starts, end after series ends. - { - endpoint: api.series, - query: url.Values{ - "match[]": []string{`test_metric2`}, - "start": []string{"-1"}, - "end": []string{"100000"}, - }, - response: []labels.Labels{ - labels.FromStrings("__name__", "test_metric2", "foo", "boo"), - }, - }, - // Start and end within series. - { - endpoint: api.series, - query: url.Values{ - "match[]": []string{`test_metric2`}, - "start": []string{"1"}, - "end": []string{"100"}, - }, - response: []labels.Labels{ - labels.FromStrings("__name__", "test_metric2", "foo", "boo"), - }, - }, - // Start within series, end after. - { - endpoint: api.series, - query: url.Values{ - "match[]": []string{`test_metric2`}, - "start": []string{"1"}, - "end": []string{"100000"}, - }, - response: []labels.Labels{ - labels.FromStrings("__name__", "test_metric2", "foo", "boo"), - }, - }, - // Start before series, end within series. - { - endpoint: api.series, - query: url.Values{ - "match[]": []string{`test_metric2`}, - "start": []string{"-1"}, - "end": []string{"1"}, - }, - response: []labels.Labels{ - labels.FromStrings("__name__", "test_metric2", "foo", "boo"), - }, - }, - // Missing match[] query params in series requests. - { - endpoint: api.series, - errType: baseAPI.ErrorBadData, - }, - { - endpoint: api.series, - query: url.Values{ - "match[]": []string{`test_metric2`}, - "dedup": []string{"sdfsf-series"}, - }, - errType: baseAPI.ErrorBadData, - }, - { - endpoint: api.series, - query: url.Values{ - "match[]": []string{`test_metric2`}, - }, - response: []labels.Labels{ - labels.FromStrings("__name__", "test_metric2", "foo", "boo"), - }, - method: http.MethodPost, - }, - { - endpoint: api.series, - query: url.Values{ - "match[]": []string{`test_metric1{foo=~".+o"}`}, - }, - response: []labels.Labels{ - labels.FromStrings("__name__", "test_metric1", "foo", "boo"), - }, - method: http.MethodPost, - }, - { - endpoint: api.series, - query: url.Values{ - "match[]": []string{`test_metric1{foo=~".+o$"}`, `test_metric1{foo=~".+o"}`}, - }, - response: []labels.Labels{ - labels.FromStrings("__name__", "test_metric1", "foo", "boo"), - }, - method: http.MethodPost, - }, - { - endpoint: api.series, - query: url.Values{ - "match[]": []string{`test_metric1{foo=~".+o"}`, `none`}, - }, - response: []labels.Labels{ - labels.FromStrings("__name__", "test_metric1", "foo", "boo"), - }, - method: http.MethodPost, - }, - // Start and end before series starts. - { - endpoint: api.series, - query: url.Values{ - "match[]": []string{`test_metric2`}, - "start": []string{"-2"}, - "end": []string{"-1"}, - }, - response: []labels.Labels{}, - }, - // Start and end after series ends. - { - endpoint: api.series, - query: url.Values{ - "match[]": []string{`test_metric2`}, - "start": []string{"100000"}, - "end": []string{"100001"}, - }, - response: []labels.Labels{}, - }, - // Start before series starts, end after series ends. - { - endpoint: api.series, - query: url.Values{ - "match[]": []string{`test_metric2`}, - "start": []string{"-1"}, - "end": []string{"100000"}, - }, - response: []labels.Labels{ - labels.FromStrings("__name__", "test_metric2", "foo", "boo"), - }, - method: http.MethodPost, - }, - // Start and end within series. - { - endpoint: api.series, - query: url.Values{ - "match[]": []string{`test_metric2`}, - "start": []string{"1"}, - "end": []string{"100"}, - }, - response: []labels.Labels{ - labels.FromStrings("__name__", "test_metric2", "foo", "boo"), - }, - method: http.MethodPost, - }, - // Start within series, end after. - { - endpoint: api.series, - query: url.Values{ - "match[]": []string{`test_metric2`}, - "start": []string{"1"}, - "end": []string{"100000"}, - }, - response: []labels.Labels{ - labels.FromStrings("__name__", "test_metric2", "foo", "boo"), - }, - method: http.MethodPost, - }, - // Start before series, end within series. - { - endpoint: api.series, - query: url.Values{ - "match[]": []string{`test_metric2`}, - "start": []string{"-1"}, - "end": []string{"1"}, - }, - response: []labels.Labels{ - labels.FromStrings("__name__", "test_metric2", "foo", "boo"), - }, - method: http.MethodPost, - }, - // Missing match[] query params in series requests. - { - endpoint: api.series, - errType: baseAPI.ErrorBadData, - method: http.MethodPost, - }, - { - endpoint: api.series, - query: url.Values{ - "match[]": []string{`test_metric2`}, - "dedup": []string{"sdfsf-series"}, - }, - errType: baseAPI.ErrorBadData, - method: http.MethodPost, - }, } for i, test := range tests { @@ -827,7 +575,7 @@ func TestEndpoints(t *testing.T) { } } -func TestLabelEndpoints(t *testing.T) { +func TestMetadataEndpoints(t *testing.T) { var old = []labels.Labels{ { labels.Label{Name: "__name__", Value: "test_metric1"}, @@ -932,8 +680,8 @@ func TestLabelEndpoints(t *testing.T) { MaxSamples: 10000, Timeout: timeout, }), - gate: gate.New(nil, 4), - defaultLabelLookbackDelta: apiLookbackDelta, + gate: gate.New(nil, 4), + defaultMetadataTimeRange: apiLookbackDelta, } var tests = []endpointTestCase{ @@ -1047,10 +795,304 @@ func TestLabelEndpoints(t *testing.T) { }, errType: baseAPI.ErrorBadData, }, + { + endpoint: api.series, + query: url.Values{ + "match[]": []string{`test_metric2`}, + }, + response: []labels.Labels{ + labels.FromStrings("__name__", "test_metric2", "foo", "boo"), + }, + }, + { + endpoint: apiWithLabelLookback.series, + query: url.Values{ + "match[]": []string{`test_metric2`}, + }, + response: []labels.Labels{}, + }, + { + endpoint: apiWithLabelLookback.series, + query: url.Values{ + "match[]": []string{`test_metric_replica1`}, + }, + response: []labels.Labels{ + labels.FromStrings("__name__", "test_metric_replica1", "foo", "bar", "replica", "a"), + labels.FromStrings("__name__", "test_metric_replica1", "foo", "boo", "replica", "a"), + labels.FromStrings("__name__", "test_metric_replica1", "foo", "boo", "replica", "b"), + }, + }, + // Series that does not exist should return an empty array. + { + endpoint: + api.series, + query: url.Values{ + "match[]": []string{`foobar`}, + }, + response: []labels.Labels{}, + }, + { + endpoint: + api.series, + query: url.Values{ + "match[]": []string{`test_metric1{foo=~".+o"}`}, + }, + response: []labels.Labels{ + labels.FromStrings("__name__", "test_metric1", "foo", "boo"), + }, + }, + { + endpoint: + api.series, + query: url.Values{ + "match[]": []string{`test_metric1{foo=~".+o$"}`, `test_metric1{foo=~".+o"}`}, + }, + response: []labels.Labels{ + labels.FromStrings("__name__", "test_metric1", "foo", "boo"), + }, + }, + { + endpoint: + api.series, + query: url.Values{ + "match[]": []string{`test_metric1{foo=~".+o"}`, `none`}, + }, + response: []labels.Labels{ + labels.FromStrings("__name__", "test_metric1", "foo", "boo"), + }, + }, + // Start and end before series starts. + { + endpoint: + api.series, + query: url.Values{ + "match[]": []string{`test_metric2`}, + "start": []string{"-2"}, + "end": []string{"-1"}, + }, + response: []labels.Labels{}, + }, + // Start and end after series ends. + { + endpoint: + api.series, + query: url.Values{ + "match[]": []string{`test_metric2`}, + "start": []string{"100000"}, + "end": []string{"100001"}, + }, + response: []labels.Labels{}, + }, + // Start before series starts, end after series ends. + { + endpoint: + api.series, + query: url.Values{ + "match[]": []string{`test_metric2`}, + "start": []string{"-1"}, + "end": []string{"100000"}, + }, + response: []labels.Labels{ + labels.FromStrings("__name__", "test_metric2", "foo", "boo"), + }, + }, + // Start and end within series. + { + endpoint: + api.series, + query: url.Values{ + "match[]": []string{`test_metric2`}, + "start": []string{"1"}, + "end": []string{"100"}, + }, + response: []labels.Labels{ + labels.FromStrings("__name__", "test_metric2", "foo", "boo"), + }, + }, + // Start within series, end after. + { + endpoint: + api.series, + query: url.Values{ + "match[]": []string{`test_metric2`}, + "start": []string{"1"}, + "end": []string{"100000"}, + }, + response: []labels.Labels{ + labels.FromStrings("__name__", "test_metric2", "foo", "boo"), + }, + }, + // Start before series, end within series. + { + endpoint: + api.series, + query: url.Values{ + "match[]": []string{`test_metric2`}, + "start": []string{"-1"}, + "end": []string{"1"}, + }, + response: []labels.Labels{ + labels.FromStrings("__name__", "test_metric2", "foo", "boo"), + }, + }, + // Missing match[] query params in series requests. + { + endpoint: + api.series, + errType: baseAPI.ErrorBadData, + }, + { + endpoint: + api.series, + query: url.Values{ + "match[]": []string{`test_metric2`}, + "dedup": []string{"sdfsf-series"}, + }, + errType: baseAPI.ErrorBadData, + }, + { + endpoint: + api.series, + query: url.Values{ + "match[]": []string{`test_metric2`}, + }, + response: []labels.Labels{ + labels.FromStrings("__name__", "test_metric2", "foo", "boo"), + }, + method: http.MethodPost, + }, + { + endpoint: + api.series, + query: url.Values{ + "match[]": []string{`test_metric1{foo=~".+o"}`}, + }, + response: []labels.Labels{ + labels.FromStrings("__name__", "test_metric1", "foo", "boo"), + }, + method: http.MethodPost, + }, + { + endpoint: + api.series, + query: url.Values{ + "match[]": []string{`test_metric1{foo=~".+o$"}`, `test_metric1{foo=~".+o"}`}, + }, + response: []labels.Labels{ + labels.FromStrings("__name__", "test_metric1", "foo", "boo"), + }, + method: http.MethodPost, + }, + { + endpoint: + api.series, + query: url.Values{ + "match[]": []string{`test_metric1{foo=~".+o"}`, `none`}, + }, + response: []labels.Labels{ + labels.FromStrings("__name__", "test_metric1", "foo", "boo"), + }, + method: http.MethodPost, + }, + // Start and end before series starts. + { + endpoint: + api.series, + query: url.Values{ + "match[]": []string{`test_metric2`}, + "start": []string{"-2"}, + "end": []string{"-1"}, + }, + response: []labels.Labels{}, + }, + // Start and end after series ends. + { + endpoint: + api.series, + query: url.Values{ + "match[]": []string{`test_metric2`}, + "start": []string{"100000"}, + "end": []string{"100001"}, + }, + response: []labels.Labels{}, + }, + // Start before series starts, end after series ends. + { + endpoint: + api.series, + query: url.Values{ + "match[]": []string{`test_metric2`}, + "start": []string{"-1"}, + "end": []string{"100000"}, + }, + response: []labels.Labels{ + labels.FromStrings("__name__", "test_metric2", "foo", "boo"), + }, + method: http.MethodPost, + }, + // Start and end within series. + { + endpoint: + api.series, + query: url.Values{ + "match[]": []string{`test_metric2`}, + "start": []string{"1"}, + "end": []string{"100"}, + }, + response: []labels.Labels{ + labels.FromStrings("__name__", "test_metric2", "foo", "boo"), + }, + method: http.MethodPost, + }, + // Start within series, end after. + { + endpoint: + api.series, + query: url.Values{ + "match[]": []string{`test_metric2`}, + "start": []string{"1"}, + "end": []string{"100000"}, + }, + response: []labels.Labels{ + labels.FromStrings("__name__", "test_metric2", "foo", "boo"), + }, + method: http.MethodPost, + }, + // Start before series, end within series. + { + endpoint: + api.series, + query: url.Values{ + "match[]": []string{`test_metric2`}, + "start": []string{"-1"}, + "end": []string{"1"}, + }, + response: []labels.Labels{ + labels.FromStrings("__name__", "test_metric2", "foo", "boo"), + }, + method: http.MethodPost, + }, + // Missing match[] query params in series requests. + { + endpoint: + api.series, + errType: baseAPI.ErrorBadData, + method: http.MethodPost, + }, + { + endpoint: + api.series, + query: url.Values{ + "match[]": []string{`test_metric2`}, + "dedup": []string{"sdfsf-series"}, + }, + errType: baseAPI.ErrorBadData, + method: http.MethodPost, + }, } for i, test := range tests { - if ok := testEndpoint(t, test, fmt.Sprintf("#%d %s", i, test.query.Encode())); !ok { + if ok := testEndpoint(t, test, strings.TrimSpace(fmt.Sprintf("#%d %s", i, test.query.Encode()))); !ok { return } } From cca1b5e860fe4628075a8530566cc1b3d64e2ec7 Mon Sep 17 00:00:00 2001 From: Kemal Akkoyun Date: Mon, 21 Sep 2020 17:44:47 +0300 Subject: [PATCH 6/7] Update changelog Signed-off-by: Kemal Akkoyun --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7cce5f93cb..09d747c33d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,7 +20,7 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re - [#3133](https://github.com/thanos-io/thanos/pull/3133) Query: Allow passing a `storeMatch[]` to Labels APIs. Also time range metadata based store filtering is supported on Labels APIs. - [#3154](https://github.com/thanos-io/thanos/pull/3154) Query Frontend: Add metric `thanos_memcached_getmulti_gate_queries_max`. - [#3146](https://github.com/thanos-io/thanos/pull/3146) Sidecar: Add `thanos_sidecar_prometheus_store_received_frames` histogram metric. -- [#3147](https://github.com/thanos-io/thanos/pull/3147) Querier: Add `query.labels.lookback-delta` flag to specify the default lookback duration for retrieving labels through Labels APIs when time range parameters are not specified. +- [#3147](https://github.com/thanos-io/thanos/pull/3147) Querier: Add `query.metadata.default-time-range` flag to specify the default metadata time range duration for retrieving labels through Labels and Series API when the range parameters are not specified. The zero value means range covers the time since the beginning. ### Changed From a3862c114c90fc3c4835e4c4930663cce60e67dc Mon Sep 17 00:00:00 2001 From: Kemal Akkoyun Date: Mon, 21 Sep 2020 17:57:22 +0300 Subject: [PATCH 7/7] Fix formatting Signed-off-by: Kemal Akkoyun --- pkg/api/query/v1_test.go | 78 ++++++++++++++-------------------------- 1 file changed, 27 insertions(+), 51 deletions(-) diff --git a/pkg/api/query/v1_test.go b/pkg/api/query/v1_test.go index 7102b9f28c..5dba268c58 100644 --- a/pkg/api/query/v1_test.go +++ b/pkg/api/query/v1_test.go @@ -824,16 +824,14 @@ func TestMetadataEndpoints(t *testing.T) { }, // Series that does not exist should return an empty array. { - endpoint: - api.series, + endpoint: api.series, query: url.Values{ "match[]": []string{`foobar`}, }, response: []labels.Labels{}, }, { - endpoint: - api.series, + endpoint: api.series, query: url.Values{ "match[]": []string{`test_metric1{foo=~".+o"}`}, }, @@ -842,8 +840,7 @@ func TestMetadataEndpoints(t *testing.T) { }, }, { - endpoint: - api.series, + endpoint: api.series, query: url.Values{ "match[]": []string{`test_metric1{foo=~".+o$"}`, `test_metric1{foo=~".+o"}`}, }, @@ -852,8 +849,7 @@ func TestMetadataEndpoints(t *testing.T) { }, }, { - endpoint: - api.series, + endpoint: api.series, query: url.Values{ "match[]": []string{`test_metric1{foo=~".+o"}`, `none`}, }, @@ -863,8 +859,7 @@ func TestMetadataEndpoints(t *testing.T) { }, // Start and end before series starts. { - endpoint: - api.series, + endpoint: api.series, query: url.Values{ "match[]": []string{`test_metric2`}, "start": []string{"-2"}, @@ -874,8 +869,7 @@ func TestMetadataEndpoints(t *testing.T) { }, // Start and end after series ends. { - endpoint: - api.series, + endpoint: api.series, query: url.Values{ "match[]": []string{`test_metric2`}, "start": []string{"100000"}, @@ -885,8 +879,7 @@ func TestMetadataEndpoints(t *testing.T) { }, // Start before series starts, end after series ends. { - endpoint: - api.series, + endpoint: api.series, query: url.Values{ "match[]": []string{`test_metric2`}, "start": []string{"-1"}, @@ -898,8 +891,7 @@ func TestMetadataEndpoints(t *testing.T) { }, // Start and end within series. { - endpoint: - api.series, + endpoint: api.series, query: url.Values{ "match[]": []string{`test_metric2`}, "start": []string{"1"}, @@ -911,8 +903,7 @@ func TestMetadataEndpoints(t *testing.T) { }, // Start within series, end after. { - endpoint: - api.series, + endpoint: api.series, query: url.Values{ "match[]": []string{`test_metric2`}, "start": []string{"1"}, @@ -924,8 +915,7 @@ func TestMetadataEndpoints(t *testing.T) { }, // Start before series, end within series. { - endpoint: - api.series, + endpoint: api.series, query: url.Values{ "match[]": []string{`test_metric2`}, "start": []string{"-1"}, @@ -937,13 +927,11 @@ func TestMetadataEndpoints(t *testing.T) { }, // Missing match[] query params in series requests. { - endpoint: - api.series, - errType: baseAPI.ErrorBadData, + endpoint: api.series, + errType: baseAPI.ErrorBadData, }, { - endpoint: - api.series, + endpoint: api.series, query: url.Values{ "match[]": []string{`test_metric2`}, "dedup": []string{"sdfsf-series"}, @@ -951,8 +939,7 @@ func TestMetadataEndpoints(t *testing.T) { errType: baseAPI.ErrorBadData, }, { - endpoint: - api.series, + endpoint: api.series, query: url.Values{ "match[]": []string{`test_metric2`}, }, @@ -962,8 +949,7 @@ func TestMetadataEndpoints(t *testing.T) { method: http.MethodPost, }, { - endpoint: - api.series, + endpoint: api.series, query: url.Values{ "match[]": []string{`test_metric1{foo=~".+o"}`}, }, @@ -973,8 +959,7 @@ func TestMetadataEndpoints(t *testing.T) { method: http.MethodPost, }, { - endpoint: - api.series, + endpoint: api.series, query: url.Values{ "match[]": []string{`test_metric1{foo=~".+o$"}`, `test_metric1{foo=~".+o"}`}, }, @@ -984,8 +969,7 @@ func TestMetadataEndpoints(t *testing.T) { method: http.MethodPost, }, { - endpoint: - api.series, + endpoint: api.series, query: url.Values{ "match[]": []string{`test_metric1{foo=~".+o"}`, `none`}, }, @@ -996,8 +980,7 @@ func TestMetadataEndpoints(t *testing.T) { }, // Start and end before series starts. { - endpoint: - api.series, + endpoint: api.series, query: url.Values{ "match[]": []string{`test_metric2`}, "start": []string{"-2"}, @@ -1007,8 +990,7 @@ func TestMetadataEndpoints(t *testing.T) { }, // Start and end after series ends. { - endpoint: - api.series, + endpoint: api.series, query: url.Values{ "match[]": []string{`test_metric2`}, "start": []string{"100000"}, @@ -1018,8 +1000,7 @@ func TestMetadataEndpoints(t *testing.T) { }, // Start before series starts, end after series ends. { - endpoint: - api.series, + endpoint: api.series, query: url.Values{ "match[]": []string{`test_metric2`}, "start": []string{"-1"}, @@ -1032,8 +1013,7 @@ func TestMetadataEndpoints(t *testing.T) { }, // Start and end within series. { - endpoint: - api.series, + endpoint: api.series, query: url.Values{ "match[]": []string{`test_metric2`}, "start": []string{"1"}, @@ -1046,8 +1026,7 @@ func TestMetadataEndpoints(t *testing.T) { }, // Start within series, end after. { - endpoint: - api.series, + endpoint: api.series, query: url.Values{ "match[]": []string{`test_metric2`}, "start": []string{"1"}, @@ -1060,8 +1039,7 @@ func TestMetadataEndpoints(t *testing.T) { }, // Start before series, end within series. { - endpoint: - api.series, + endpoint: api.series, query: url.Values{ "match[]": []string{`test_metric2`}, "start": []string{"-1"}, @@ -1074,14 +1052,12 @@ func TestMetadataEndpoints(t *testing.T) { }, // Missing match[] query params in series requests. { - endpoint: - api.series, - errType: baseAPI.ErrorBadData, - method: http.MethodPost, + endpoint: api.series, + errType: baseAPI.ErrorBadData, + method: http.MethodPost, }, { - endpoint: - api.series, + endpoint: api.series, query: url.Values{ "match[]": []string{`test_metric2`}, "dedup": []string{"sdfsf-series"},