PR response

m3db · Dec 1, 2019 · f6640a1 · f6640a1
1 parent 405d842
commit f6640a1
Show file tree

Hide file tree

Showing 18 changed files with 1,109 additions and 469 deletions.
diff --git a/src/query/api/v1/handler/fetch_options.go b/src/query/api/v1/handler/fetch_options.go
@@ -21,6 +21,7 @@
 package handler
 
 import (
+	"encoding/json"
 	"fmt"
 	"math"
 	"net/http"
@@ -30,12 +31,8 @@ import (
 
 	"github.com/m3db/m3/src/metrics/policy"
 	"github.com/m3db/m3/src/query/errors"
-	"github.com/m3db/m3/src/query/models"
-	parser "github.com/m3db/m3/src/query/parser/promql"
 	"github.com/m3db/m3/src/query/storage"
 	xhttp "github.com/m3db/m3/src/x/net/http"
-
-	"github.com/prometheus/prometheus/promql"
 )
 
 const (
@@ -118,7 +115,9 @@ func (b fetchOptionsBuilder) NewFetchOptions(
 		}
 
 		fetchOpts.RestrictFetchOptions = newOrExistingRestrictFetchOptions(fetchOpts)
-		fetchOpts.RestrictFetchOptions.MetricsType = mt
+		fetchOpts.RestrictFetchOptions.RestrictByType =
+			newOrExistingRestrictFetchOptionsRestrictByType(fetchOpts)
+		fetchOpts.RestrictFetchOptions.RestrictByType.MetricsType = mt
 	}
 
 	if str := req.Header.Get(MetricsStoragePolicyHeader); str != "" {
@@ -130,23 +129,19 @@ func (b fetchOptionsBuilder) NewFetchOptions(
 		}
 
 		fetchOpts.RestrictFetchOptions = newOrExistingRestrictFetchOptions(fetchOpts)
-		fetchOpts.RestrictFetchOptions.StoragePolicy = sp
+		fetchOpts.RestrictFetchOptions.RestrictByType =
+			newOrExistingRestrictFetchOptionsRestrictByType(fetchOpts)
+		fetchOpts.RestrictFetchOptions.RestrictByType.StoragePolicy = sp
 	}
 
 	if str := req.Header.Get(FetchRestrictLabels); str != "" {
-		promMatchers, err := promql.ParseMetricSelector(str)
-		if err != nil {
+		var opts *storage.RestrictFetchOptions
+		if err := json.Unmarshal([]byte(str), opts); err != nil {
 			return nil, xhttp.NewParseError(err, http.StatusBadRequest)
 		}
 
 		fetchOpts.RestrictFetchOptions = newOrExistingRestrictFetchOptions(fetchOpts)
-		// TODO: filter through TagOptions
-		m3Matchers, err := parser.LabelMatchersToModelMatcher(promMatchers, models.NewTagOptions())
-		if err != nil {
-			return nil, xhttp.NewParseError(err, http.StatusBadRequest)
-		}
-
-		fetchOpts.RestrictFetchOptions.MustApplyMatchers = m3Matchers
+		fetchOpts.RestrictFetchOptions.RestrictByTag = opts.RestrictByTag
 	}
 
 	if restrict := fetchOpts.RestrictFetchOptions; restrict != nil {
@@ -186,6 +181,15 @@ func newOrExistingRestrictFetchOptions(
 	return &storage.RestrictFetchOptions{}
 }
 
+func newOrExistingRestrictFetchOptionsRestrictByType(
+	fetchOpts *storage.FetchOptions,
+) *storage.RestrictByType {
+	if v := fetchOpts.RestrictFetchOptions.RestrictByType; v != nil {
+		return v
+	}
+	return &storage.RestrictByType{}
+}
+
 // ParseStep parses the step duration for an HTTP request.
 func ParseStep(r *http.Request) (time.Duration, bool, error) {
 	step := r.FormValue(StepParam)

diff --git a/src/query/api/v1/handler/fetch_options_test.go b/src/query/api/v1/handler/fetch_options_test.go
@@ -79,7 +79,9 @@ func TestFetchOptionsBuilder(t *testing.T) {
 				MetricsTypeHeader: storage.UnaggregatedMetricsType.String(),
 			},
 			expectedRestrict: &storage.RestrictFetchOptions{
-				MetricsType: storage.UnaggregatedMetricsType,
+				RestrictByType: &storage.RestrictByType{
+					MetricsType: storage.UnaggregatedMetricsType,
+				},
 			},
 		},
 		{
@@ -89,8 +91,10 @@ func TestFetchOptionsBuilder(t *testing.T) {
 				MetricsStoragePolicyHeader: "1m:14d",
 			},
 			expectedRestrict: &storage.RestrictFetchOptions{
-				MetricsType:   storage.AggregatedMetricsType,
-				StoragePolicy: policy.MustParseStoragePolicy("1m:14d"),
+				RestrictByType: &storage.RestrictByType{
+					MetricsType:   storage.AggregatedMetricsType,
+					StoragePolicy: policy.MustParseStoragePolicy("1m:14d"),
+				},
 			},
 		},
 		{

diff --git a/src/query/api/v1/handler/prometheus/common.go b/src/query/api/v1/handler/prometheus/common.go
@@ -639,22 +639,15 @@ func FilterSeriesByOptions(
 	series []*ts.Series,
 	opts *storage.FetchOptions,
 ) []*ts.Series {
-	if opts == nil || opts.RestrictFetchOptions == nil {
+	if opts == nil {
 		return series
 	}
 
-	matchers := opts.RestrictFetchOptions.MustApplyMatchers
-	if len(matchers) == 0 {
-		return series
-	}
-
-	keys := make([][]byte, 0, len(matchers))
-	for _, m := range matchers {
-		keys = append(keys, m.Name)
-	}
-
-	for i, s := range series {
-		series[i].Tags = s.Tags.TagsWithoutKeys(keys)
+	keys := opts.RestrictFetchOptions.GetRestrictByTag().GetFilterByNames()
+	if len(keys) > 0 {
+		for i, s := range series {
+			series[i].Tags = s.Tags.TagsWithoutKeys(keys)
+		}
 	}
 
 	return series

diff --git a/src/query/api/v1/handler/prometheus/native/read.go b/src/query/api/v1/handler/prometheus/native/read.go
@@ -146,7 +146,9 @@ func (h *PromReadHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		QueryContextOptions: models.QueryContextOptions{
 			LimitMaxTimeseries: fetchOpts.Limit,
 		}}
-	if restrictOpts := fetchOpts.RestrictFetchOptions; restrictOpts != nil {
+
+	restrictOpts := fetchOpts.RestrictFetchOptions.GetRestrictByType()
+	if restrictOpts != nil {
 		restrict := &models.RestrictFetchTypeQueryContextOptions{
 			MetricsType:   uint(restrictOpts.MetricsType),
 			StoragePolicy: restrictOpts.StoragePolicy,

diff --git a/src/query/api/v1/handler/prometheus/native/read_instantaneous.go b/src/query/api/v1/handler/prometheus/native/read_instantaneous.go
@@ -96,7 +96,9 @@ func (h *PromReadInstantHandler) ServeHTTP(w http.ResponseWriter, r *http.Reques
 		QueryContextOptions: models.QueryContextOptions{
 			LimitMaxTimeseries: fetchOpts.Limit,
 		}}
-	if restrictOpts := fetchOpts.RestrictFetchOptions; restrictOpts != nil {
+
+	restrictOpts := fetchOpts.RestrictFetchOptions.GetRestrictByType()
+	if restrictOpts != nil {
 		restrict := &models.RestrictFetchTypeQueryContextOptions{
 			MetricsType:   uint(restrictOpts.MetricsType),
 			StoragePolicy: restrictOpts.StoragePolicy,

diff --git a/src/query/api/v1/handler/prometheus/validator/handler_test.go b/src/query/api/v1/handler/prometheus/validator/handler_test.go
@@ -22,7 +22,6 @@ package validator
 
 import (
 	"encoding/json"
-	"fmt"
 	"io"
 	"net/http"
 	"net/http/httptest"
@@ -335,7 +334,6 @@ func TestValidateEndpoint(t *testing.T) {
 	recorder := httptest.NewRecorder()
 	debugHandler.ServeHTTP(recorder, req)
 
-	fmt.Println(recorder.Body.String())
 	var mismatches MismatchesJSON
 	require.NoError(t, json.Unmarshal(recorder.Body.Bytes(), &mismatches))
 	assert.False(t, mismatches.Correct)

diff --git a/src/query/functions/fetch_test.go b/src/query/functions/fetch_test.go
@@ -146,7 +146,8 @@ func TestFetchWithRestrictFetch(t *testing.T) {
 	assert.Equal(t, expected, sink.Values)
 
 	fetchOpts := mockStorage.LastFetchOptions()
-	require.NotNil(t, fetchOpts.RestrictFetchOptions)
-	assert.Equal(t, storage.AggregatedMetricsType, storage.MetricsType(fetchOpts.RestrictFetchOptions.MetricsType))
-	assert.Equal(t, "10s:42d", fetchOpts.RestrictFetchOptions.StoragePolicy.String())
+	restrictByType := fetchOpts.RestrictFetchOptions.GetRestrictByType()
+	require.NotNil(t, restrictByType)
+	assert.Equal(t, storage.AggregatedMetricsType, storage.MetricsType(restrictByType.MetricsType))
+	assert.Equal(t, "10s:42d", restrictByType.StoragePolicy.String())
 }