diff --git a/scripts/docker-integration-tests/carbon/expected/a_dot_starstar_dot_star.json b/scripts/docker-integration-tests/carbon/expected/a_dot_starstar_dot_star.json new file mode 100644 index 0000000000..23ca21621c --- /dev/null +++ b/scripts/docker-integration-tests/carbon/expected/a_dot_starstar_dot_star.json @@ -0,0 +1,23 @@ +[ + { + "id": "a.**.daz", + "text": "daz", + "leaf": 0, + "expandable": 1, + "allowChildren": 1 + }, + { + "id": "a.**.cake", + "text": "cake", + "leaf": 0, + "expandable": 1, + "allowChildren": 1 + }, + { + "id": "a.**.caw", + "text": "caw", + "leaf": 0, + "expandable": 1, + "allowChildren": 1 + } +] \ No newline at end of file diff --git a/scripts/docker-integration-tests/carbon/expected/a_starstar_dot_star.json b/scripts/docker-integration-tests/carbon/expected/a_starstar_dot_star.json new file mode 100644 index 0000000000..9315f8e3ba --- /dev/null +++ b/scripts/docker-integration-tests/carbon/expected/a_starstar_dot_star.json @@ -0,0 +1,44 @@ +[ + { + "id": "a**.caw", + "text": "caw", + "leaf": 0, + "expandable": 1, + "allowChildren": 1 + }, + { + "id": "a**.daz", + "text": "daz", + "leaf": 0, + "expandable": 1, + "allowChildren": 1 + }, + { + "id": "a**.bag", + "text": "bag", + "leaf": 0, + "expandable": 1, + "allowChildren": 1 + }, + { + "id": "a**.bar", + "text": "bar", + "leaf": 0, + "expandable": 1, + "allowChildren": 1 + }, + { + "id": "a**.biz", + "text": "biz", + "leaf": 0, + "expandable": 1, + "allowChildren": 1 + }, + { + "id": "a**.cake", + "text": "cake", + "leaf": 0, + "expandable": 1, + "allowChildren": 1 + } +] \ No newline at end of file diff --git a/scripts/docker-integration-tests/carbon/expected/qux_dot_starstar_dot_star.json b/scripts/docker-integration-tests/carbon/expected/qux_dot_starstar_dot_star.json new file mode 100644 index 0000000000..f96351bdc0 --- /dev/null +++ b/scripts/docker-integration-tests/carbon/expected/qux_dot_starstar_dot_star.json @@ -0,0 +1,16 @@ +[ + { + "id": "qux.**.pos2-1", + "text": "pos2-1", + "leaf": 0, + "expandable": 1, + "allowChildren": 1 + }, + { + "id": "qux.**.pos2-0", + "text": "pos2-0", + "leaf": 0, + "expandable": 1, + "allowChildren": 1 + } +] \ No newline at end of file diff --git a/scripts/docker-integration-tests/carbon/expected/qux_starstar_dot_star.json b/scripts/docker-integration-tests/carbon/expected/qux_starstar_dot_star.json new file mode 100644 index 0000000000..8f3ab219d5 --- /dev/null +++ b/scripts/docker-integration-tests/carbon/expected/qux_starstar_dot_star.json @@ -0,0 +1,37 @@ +[ + { + "id": "qux**.pos2-1", + "text": "pos2-1", + "leaf": 0, + "expandable": 1, + "allowChildren": 1 + }, + { + "id": "qux**.pos1-a", + "text": "pos1-a", + "leaf": 0, + "expandable": 1, + "allowChildren": 1 + }, + { + "id": "qux**.pos1-b", + "text": "pos1-b", + "leaf": 0, + "expandable": 1, + "allowChildren": 1 + }, + { + "id": "qux**.pos1-c", + "text": "pos1-c", + "leaf": 0, + "expandable": 1, + "allowChildren": 1 + }, + { + "id": "qux**.pos2-0", + "text": "pos2-0", + "leaf": 0, + "expandable": 1, + "allowChildren": 1 + } +] \ No newline at end of file diff --git a/scripts/docker-integration-tests/carbon/test.sh b/scripts/docker-integration-tests/carbon/test.sh index a7c68b680e..50b5e0dcd8 100755 --- a/scripts/docker-integration-tests/carbon/test.sh +++ b/scripts/docker-integration-tests/carbon/test.sh @@ -136,6 +136,10 @@ ATTEMPTS=2 TIMEOUT=1 retry_with_backoff "find_carbon 'f.bar.*' fbaz.json" ATTEMPTS=2 TIMEOUT=1 retry_with_backoff "find_carbon 'g.bar.*' gbaz.json" ATTEMPTS=2 TIMEOUT=1 retry_with_backoff "find_carbon 'h.bar*' hbarbaz.json" ATTEMPTS=2 TIMEOUT=1 retry_with_backoff "find_carbon 'i.bar*' ibarbaz.json" +ATTEMPTS=2 TIMEOUT=1 retry_with_backoff "find_carbon 'a**.*' a_starstar_dot_star.json" +ATTEMPTS=2 TIMEOUT=1 retry_with_backoff "find_carbon 'a.**.*' a_dot_starstar_dot_star.json" +ATTEMPTS=2 TIMEOUT=1 retry_with_backoff "find_carbon 'qux**.*' qux_starstar_dot_star.json" +ATTEMPTS=2 TIMEOUT=1 retry_with_backoff "find_carbon 'qux.**.*' qux_dot_starstar_dot_star.json" # Test find limits from config of matching max docs of 200 with: # carbon: diff --git a/src/query/api/v1/handler/graphite/find.go b/src/query/api/v1/handler/graphite/find.go index fb36baea29..f36b99e4ac 100644 --- a/src/query/api/v1/handler/graphite/find.go +++ b/src/query/api/v1/handler/graphite/find.go @@ -73,31 +73,45 @@ func mergeTags( terminatedResult *consolidators.CompleteTagsResult, childResult *consolidators.CompleteTagsResult, ) (map[string]nodeDescriptor, error) { - // sanity check the case. - if terminatedResult.CompleteNameOnly { - return nil, errors.New("terminated result is completing name only") - } + var ( + terminatedResultTags = 0 + childResultTags = 0 + ) - if childResult.CompleteNameOnly { - return nil, errors.New("child result is completing name only") + // Sanity check the queries aren't complete name only queries. + if terminatedResult != nil { + terminatedResultTags = len(terminatedResult.CompletedTags) + if terminatedResult.CompleteNameOnly { + return nil, errors.New("terminated result is completing name only") + } } - mapLength := len(terminatedResult.CompletedTags) + len(childResult.CompletedTags) - tagMap := make(map[string]nodeDescriptor, mapLength) + if childResult != nil { + childResultTags = len(childResult.CompletedTags) + if childResult.CompleteNameOnly { + return nil, errors.New("child result is completing name only") + } + } - for _, tag := range terminatedResult.CompletedTags { - for _, value := range tag.Values { - descriptor := tagMap[string(value)] - descriptor.isLeaf = true - tagMap[string(value)] = descriptor + size := terminatedResultTags + childResultTags + tagMap := make(map[string]nodeDescriptor, size) + if terminatedResult != nil { + for _, tag := range terminatedResult.CompletedTags { + for _, value := range tag.Values { + descriptor := tagMap[string(value)] + descriptor.isLeaf = true + tagMap[string(value)] = descriptor + } } } - for _, tag := range childResult.CompletedTags { - for _, value := range tag.Values { - descriptor := tagMap[string(value)] - descriptor.hasChildren = true - tagMap[string(value)] = descriptor + if childResult != nil { + for _, tag := range childResult.CompletedTags { + for _, value := range tag.Values { + descriptor := tagMap[string(value)] + descriptor.hasChildren = true + tagMap[string(value)] = descriptor + } } } @@ -120,7 +134,7 @@ func (h *grahiteFindHandler) ServeHTTP( // NB: need to run two separate queries, one of which will match only the // provided matchers, and one which will match the provided matchers with at // least one more child node. For further information, refer to the comment - // for parseFindParamsToQueries + // for parseFindParamsToQueries. terminatedQuery, childQuery, raw, err := parseFindParamsToQueries(r) if err != nil { xhttp.WriteError(w, err) @@ -134,11 +148,17 @@ func (h *grahiteFindHandler) ServeHTTP( cErr error wg sync.WaitGroup ) - wg.Add(2) - go func() { - terminatedResult, tErr = h.storage.CompleteTags(ctx, terminatedQuery, opts) - wg.Done() - }() + if terminatedQuery != nil { + // Sometimes we only perform the child query, so only perform + // terminated query if not nil. + wg.Add(1) + go func() { + terminatedResult, tErr = h.storage.CompleteTags(ctx, terminatedQuery, opts) + wg.Done() + }() + } + // Always perform child query. + wg.Add(1) go func() { childResult, cErr = h.storage.CompleteTags(ctx, childQuery, opts) wg.Done() @@ -152,7 +172,11 @@ func (h *grahiteFindHandler) ServeHTTP( return } - meta := terminatedResult.Metadata.CombineMetadata(childResult.Metadata) + meta := childResult.Metadata + if terminatedResult != nil { + meta = terminatedResult.Metadata.CombineMetadata(childResult.Metadata) + } + // NB: merge results from both queries to specify which series have children seenMap, err := mergeTags(terminatedResult, childResult) if err != nil { diff --git a/src/query/api/v1/handler/graphite/find_parser.go b/src/query/api/v1/handler/graphite/find_parser.go index 8d34dedc8d..b34592d3a1 100644 --- a/src/query/api/v1/handler/graphite/find_parser.go +++ b/src/query/api/v1/handler/graphite/find_parser.go @@ -24,6 +24,7 @@ import ( "fmt" "io" "net/http" + "strings" "time" "github.com/m3db/m3/src/query/errors" @@ -96,12 +97,46 @@ func parseFindParamsToQueries(r *http.Request) ( xerrors.NewInvalidParamsError(fmt.Errorf("invalid 'until': %s", untilString)) } - matchers, err := graphitestorage.TranslateQueryToMatchersWithTerminator(query) + matchers, queryType, err := graphitestorage.TranslateQueryToMatchersWithTerminator(query) if err != nil { return nil, nil, "", xerrors.NewInvalidParamsError(fmt.Errorf("invalid 'query': %s", query)) } + switch queryType { + case graphitestorage.StarStarUnterminatedTranslatedQuery: + // Translated query for "**" has unterminated search for children + // terms, we are only going to do a single search and assume all + // results that come back have also children in the graphite path + // tree (since it's very expensive to check if each result that comes + // back is a child or leaf node and "**" in a find query is typically + // only used for template variables rather than searching for metric + // results, which is the only use case isLeaf/hasChildren is useful). + // Note: Filter to all graphite tags that appears at the last node + // or greater than that (we use 100 as an arbitrary upper bound). + maxPathIndexes := 100 + filter := make([][]byte, 0, maxPathIndexes) + parts := 1 + strings.Count(query, ".") + firstPathIndex := parts - 1 + for i := firstPathIndex; i < firstPathIndex+maxPathIndexes; i++ { + filter = append(filter, graphite.TagName(i)) + } + childQuery := &storage.CompleteTagsQuery{ + CompleteNameOnly: false, + FilterNameTags: filter, + TagMatchers: matchers, + Start: xtime.ToUnixNano(from), + End: xtime.ToUnixNano(until), + } + return nil, childQuery, query, nil + case graphitestorage.TerminatedTranslatedQuery: + // Default type of translated query, explicitly craft queries for + // a terminated part of the query and a child part of the query. + break + default: + return nil, nil, "", fmt.Errorf("unknown query type: %v", queryType) + } + // NB: Filter will always be the second last term in the matchers, and the // matchers should always have a length of at least 2 (term + terminator) // so this is a sanity check and unexpected in actual execution. diff --git a/src/query/api/v1/handler/graphite/find_test.go b/src/query/api/v1/handler/graphite/find_test.go index bc64daf1b9..1ab85e84a8 100644 --- a/src/query/api/v1/handler/graphite/find_test.go +++ b/src/query/api/v1/handler/graphite/find_test.go @@ -31,18 +31,21 @@ import ( "testing" "time" + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/m3db/m3/src/m3ninx/doc" "github.com/m3db/m3/src/query/api/v1/handler/prometheus/handleroptions" "github.com/m3db/m3/src/query/api/v1/options" "github.com/m3db/m3/src/query/block" + "github.com/m3db/m3/src/query/graphite/graphite" "github.com/m3db/m3/src/query/models" "github.com/m3db/m3/src/query/storage" "github.com/m3db/m3/src/query/storage/m3/consolidators" "github.com/m3db/m3/src/x/headers" + xtest "github.com/m3db/m3/src/x/test" xtime "github.com/m3db/m3/src/x/time" - - "github.com/golang/mock/gomock" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) // dates is a tuple of a date with a valid string representation @@ -63,10 +66,18 @@ var ( ) type completeTagQueryMatcher struct { - matchers []models.Matcher + matchers []models.Matcher + filterNameTagsIndexStart int + filterNameTagsIndexEnd int +} + +func (m *completeTagQueryMatcher) String() string { + q := storage.CompleteTagsQuery{ + TagMatchers: m.matchers, + } + return q.String() } -func (m *completeTagQueryMatcher) String() string { return "complete tag query" } func (m *completeTagQueryMatcher) Matches(x interface{}) bool { q, ok := x.(*storage.CompleteTagsQuery) if !ok { @@ -85,13 +96,33 @@ func (m *completeTagQueryMatcher) Matches(x interface{}) bool { return false } - if len(q.FilterNameTags) != 1 { - return false - } + if m.filterNameTagsIndexStart == 0 && m.filterNameTagsIndexEnd == 0 { + // Default query completing single graphite path index value. + if len(q.FilterNameTags) != 1 { + return false + } - // both queries should filter on __g1__ - if !bytes.Equal(q.FilterNameTags[0], []byte("__g1__")) { - return false + // Both queries should filter on __g1__. + if !bytes.Equal(q.FilterNameTags[0], []byte("__g1__")) { + return false + } + } else { + // Unterminated query completing many grapth path index values. + n := m.filterNameTagsIndexEnd + expected := make([][]byte, 0, n) + for i := m.filterNameTagsIndexStart; i < m.filterNameTagsIndexEnd; i++ { + expected = append(expected, graphite.TagName(i)) + } + + if len(q.FilterNameTags) != len(expected) { + return false + } + + for i := range expected { + if !bytes.Equal(q.FilterNameTags[i], expected[i]) { + return false + } + } } if len(q.TagMatchers) != len(m.matchers) { @@ -121,65 +152,9 @@ func bs(ss ...string) [][]byte { for i, s := range ss { bb[i] = b(s) } - return bb } -func setupStorage(ctrl *gomock.Controller, ex, ex2 bool) storage.Storage { - store := storage.NewMockStorage(ctrl) - // set up no children case - noChildrenMatcher := &completeTagQueryMatcher{ - matchers: []models.Matcher{ - {Type: models.MatchEqual, Name: b("__g0__"), Value: b("foo")}, - {Type: models.MatchRegexp, Name: b("__g1__"), Value: b(`b[^\.]*`)}, - {Type: models.MatchNotField, Name: b("__g2__")}, - }, - } - - noChildrenResult := &consolidators.CompleteTagsResult{ - CompleteNameOnly: false, - CompletedTags: []consolidators.CompletedTag{ - {Name: b("__g1__"), Values: bs("bug", "bar", "baz")}, - }, - Metadata: block.ResultMetadata{ - LocalOnly: true, - Exhaustive: ex, - }, - } - - store.EXPECT().CompleteTags(gomock.Any(), noChildrenMatcher, gomock.Any()). - Return(noChildrenResult, nil) - - // set up children case - childrenMatcher := &completeTagQueryMatcher{ - matchers: []models.Matcher{ - {Type: models.MatchEqual, Name: b("__g0__"), Value: b("foo")}, - {Type: models.MatchRegexp, Name: b("__g1__"), Value: b(`b[^\.]*`)}, - {Type: models.MatchField, Name: b("__g2__")}, - }, - } - - childrenResult := &consolidators.CompleteTagsResult{ - CompleteNameOnly: false, - CompletedTags: []consolidators.CompletedTag{ - {Name: b("__g1__"), Values: bs("baz", "bix", "bug")}, - }, - Metadata: block.ResultMetadata{ - LocalOnly: false, - Exhaustive: true, - }, - } - - if !ex2 { - childrenResult.Metadata.AddWarning("foo", "bar") - } - - store.EXPECT().CompleteTags(gomock.Any(), childrenMatcher, gomock.Any()). - Return(childrenResult, nil) - - return store -} - type writer struct { results []string header http.Header @@ -221,94 +196,254 @@ func (r results) Less(i, j int) bool { return strings.Compare(r[i].ID, r[j].ID) == -1 } -func testFind(t *testing.T, httpMethod string, ex bool, ex2 bool, header string) { - ctrl := gomock.NewController(t) - defer ctrl.Finish() - - // setup storage and handler - store := setupStorage(ctrl, ex, ex2) - - builder, err := handleroptions.NewFetchOptionsBuilder( - handleroptions.FetchOptionsBuilderOptions{ - Timeout: 15 * time.Second, - }) - require.NoError(t, err) - opts := options.EmptyHandlerOptions(). - SetGraphiteFindFetchOptionsBuilder(builder). - SetStorage(store) - h := NewFindHandler(opts) - - // execute the query - params := make(url.Values) - params.Set("query", "foo.b*") - params.Set("from", from.s) - params.Set("until", until.s) - - w := &writer{} - req := &http.Request{ - Method: httpMethod, - } - switch httpMethod { - case http.MethodGet: - req.URL = &url.URL{ - RawQuery: params.Encode(), - } - case http.MethodPost: - req.Form = params - } - - h.ServeHTTP(w, req) - - // convert results to comparable format - require.Equal(t, 1, len(w.results)) - r := make(results, 0) - decoder := json.NewDecoder(bytes.NewBufferString((w.results[0]))) - require.NoError(t, decoder.Decode(&r)) - sort.Sort(r) - - makeNoChildrenResult := func(t string) result { - return result{ID: fmt.Sprintf("foo.%s", t), Text: t, Leaf: 1, - Expandable: 0, AllowChildren: 0} - } - - makeWithChildrenResult := func(t string) result { - return result{ID: fmt.Sprintf("foo.%s", t), Text: t, Leaf: 0, - Expandable: 1, AllowChildren: 1} +func makeNoChildrenResult(id, text string) result { + return result{ + ID: id, + Text: text, + Leaf: 1, + Expandable: 0, + AllowChildren: 0, } +} - expected := results{ - makeNoChildrenResult("bar"), - makeNoChildrenResult("baz"), - makeWithChildrenResult("baz"), - makeWithChildrenResult("bix"), - makeNoChildrenResult("bug"), - makeWithChildrenResult("bug"), +func makeWithChildrenResult(id, text string) result { + return result{ + ID: id, + Text: text, + Leaf: 0, + Expandable: 1, + AllowChildren: 1, } - - require.Equal(t, expected, r) - actual := w.Header().Get(headers.LimitHeader) - assert.Equal(t, header, actual) } -var limitTests = []struct { +type limitTest struct { name string ex, ex2 bool header string -}{ - {"both incomplete", false, false, fmt.Sprintf( - "%s,%s_%s", headers.LimitHeaderSeriesLimitApplied, "foo", "bar")}, - {"with terminator incomplete", true, false, "foo_bar"}, - {"with children incomplete", false, true, - headers.LimitHeaderSeriesLimitApplied}, - {"both complete", true, true, ""}, } +var ( + bothCompleteLimitTest = limitTest{"both complete", true, true, ""} + limitTests = []limitTest{ + bothCompleteLimitTest, + { + "both incomplete", false, false, + fmt.Sprintf("%s,%s_%s", headers.LimitHeaderSeriesLimitApplied, "foo", "bar"), + }, + { + "with terminator incomplete", true, false, + "foo_bar", + }, + { + "with children incomplete", false, true, + headers.LimitHeaderSeriesLimitApplied, + }, + } +) + func TestFind(t *testing.T) { - for _, tt := range limitTests { - t.Run(tt.name, func(t *testing.T) { - for _, httpMethod := range FindHTTPMethods { - testFind(t, httpMethod, tt.ex, tt.ex2, tt.header) - } + for _, httpMethod := range FindHTTPMethods { + testFind(t, testFindOptions{ + httpMethod: httpMethod, }) } } + +type testFindOptions struct { + httpMethod string +} + +type testFindQuery struct { + expectMatchers *completeTagQueryMatcher + mockResult func(lt limitTest) *consolidators.CompleteTagsResult +} + +func testFind(t *testing.T, opts testFindOptions) { + warningsFooBar := block.Warnings{ + block.Warning{ + Name: "foo", + Message: "bar", + }, + } + + for _, test := range []struct { + query string + limitTests []limitTest + terminatedQuery *testFindQuery + childQuery *testFindQuery + expectedResults results + }{ + { + query: "foo.b*", + limitTests: limitTests, + terminatedQuery: &testFindQuery{ + expectMatchers: &completeTagQueryMatcher{ + matchers: []models.Matcher{ + {Type: models.MatchEqual, Name: b("__g0__"), Value: b("foo")}, + {Type: models.MatchRegexp, Name: b("__g1__"), Value: b(`b[^\.]*`)}, + {Type: models.MatchNotField, Name: b("__g2__")}, + }, + }, + mockResult: func(lt limitTest) *consolidators.CompleteTagsResult { + return &consolidators.CompleteTagsResult{ + CompleteNameOnly: false, + CompletedTags: []consolidators.CompletedTag{ + {Name: b("__g1__"), Values: bs("bug", "bar", "baz")}, + }, + Metadata: block.ResultMetadata{ + LocalOnly: true, + Exhaustive: lt.ex, + }, + } + }, + }, + childQuery: &testFindQuery{ + expectMatchers: &completeTagQueryMatcher{ + matchers: []models.Matcher{ + {Type: models.MatchEqual, Name: b("__g0__"), Value: b("foo")}, + {Type: models.MatchRegexp, Name: b("__g1__"), Value: b(`b[^\.]*`)}, + {Type: models.MatchField, Name: b("__g2__")}, + }, + }, + mockResult: func(lt limitTest) *consolidators.CompleteTagsResult { + var warnings block.Warnings + if !lt.ex2 { + warnings = warningsFooBar + } + return &consolidators.CompleteTagsResult{ + CompleteNameOnly: false, + CompletedTags: []consolidators.CompletedTag{ + {Name: b("__g1__"), Values: bs("baz", "bix", "bug")}, + }, + Metadata: block.ResultMetadata{ + LocalOnly: false, + Exhaustive: true, + Warnings: warnings, + }, + } + }, + }, + expectedResults: results{ + makeNoChildrenResult("foo.bar", "bar"), + makeNoChildrenResult("foo.baz", "baz"), + makeWithChildrenResult("foo.baz", "baz"), + makeWithChildrenResult("foo.bix", "bix"), + makeNoChildrenResult("foo.bug", "bug"), + makeWithChildrenResult("foo.bug", "bug"), + }, + }, + { + query: "foo.**.*", + childQuery: &testFindQuery{ + expectMatchers: &completeTagQueryMatcher{ + matchers: []models.Matcher{ + { + Type: models.MatchRegexp, + Name: b("__g0__"), Value: b(".*"), + }, + { + Type: models.MatchRegexp, + Name: doc.IDReservedFieldName, + Value: b(`foo\.+.*[^\.]*`), + }, + }, + filterNameTagsIndexStart: 2, + filterNameTagsIndexEnd: 102, + }, + mockResult: func(_ limitTest) *consolidators.CompleteTagsResult { + return &consolidators.CompleteTagsResult{ + CompleteNameOnly: false, + CompletedTags: []consolidators.CompletedTag{ + {Name: b("__g2__"), Values: bs("bar0", "bar1")}, + {Name: b("__g3__"), Values: bs("baz0", "baz1", "baz2")}, + }, + Metadata: block.ResultMetadata{ + LocalOnly: true, + Exhaustive: true, + }, + } + }, + }, + expectedResults: results{ + makeWithChildrenResult("foo.**.bar0", "bar0"), + makeWithChildrenResult("foo.**.bar1", "bar1"), + makeWithChildrenResult("foo.**.baz0", "baz0"), + makeWithChildrenResult("foo.**.baz1", "baz1"), + makeWithChildrenResult("foo.**.baz2", "baz2"), + }, + }, + } { + // Set which limit tests should be performed for this query. + testCaseLimitTests := test.limitTests + if len(limitTests) == 0 { + // Just test case where both are complete. + testCaseLimitTests = []limitTest{bothCompleteLimitTest} + } + + for _, limitTest := range testCaseLimitTests { + // nolint: govet + limitTest := limitTest + t.Run(fmt.Sprintf("%s-%s", test.query, limitTest.name), func(t *testing.T) { + ctrl := xtest.NewController(t) + defer ctrl.Finish() + + store := storage.NewMockStorage(ctrl) + + if q := test.terminatedQuery; q != nil { + // Set up no children case. + store.EXPECT(). + CompleteTags(gomock.Any(), q.expectMatchers, gomock.Any()). + Return(q.mockResult(limitTest), nil) + } + + if q := test.childQuery; q != nil { + // Set up children case. + store.EXPECT(). + CompleteTags(gomock.Any(), q.expectMatchers, gomock.Any()). + Return(q.mockResult(limitTest), nil) + } + + builder, err := handleroptions.NewFetchOptionsBuilder( + handleroptions.FetchOptionsBuilderOptions{ + Timeout: 15 * time.Second, + }) + require.NoError(t, err) + + handlerOpts := options.EmptyHandlerOptions(). + SetGraphiteFindFetchOptionsBuilder(builder). + SetStorage(store) + h := NewFindHandler(handlerOpts) + + // Execute the query. + params := make(url.Values) + params.Set("query", test.query) + params.Set("from", from.s) + params.Set("until", until.s) + + w := &writer{} + req := &http.Request{Method: opts.httpMethod} + switch opts.httpMethod { + case http.MethodGet: + req.URL = &url.URL{ + RawQuery: params.Encode(), + } + case http.MethodPost: + req.Form = params + } + + h.ServeHTTP(w, req) + + // Convert results to comparable format. + require.Equal(t, 1, len(w.results)) + r := make(results, 0) + decoder := json.NewDecoder(bytes.NewBufferString((w.results[0]))) + require.NoError(t, decoder.Decode(&r)) + sort.Sort(r) + + require.Equal(t, test.expectedResults, r) + actual := w.Header().Get(headers.LimitHeader) + assert.Equal(t, limitTest.header, actual) + }) + } + } +} diff --git a/src/query/graphite/storage/m3_wrapper.go b/src/query/graphite/storage/m3_wrapper.go index ca748300f3..658b0b9f29 100644 --- a/src/query/graphite/storage/m3_wrapper.go +++ b/src/query/graphite/storage/m3_wrapper.go @@ -92,16 +92,29 @@ func NewM3WrappedStorage( } } +// TranslatedQueryType describes a translated query type. +type TranslatedQueryType uint + +const ( + // TerminatedTranslatedQuery is a query that is terminated at an explicit + // leaf node (i.e. specific graphite path index number). + TerminatedTranslatedQuery TranslatedQueryType = iota + // StarStarUnterminatedTranslatedQuery is a query that is not terminated by + // an explicit leaf node since it matches indefinite child nodes due to + // a "**" in the query which matches indefinited child nodes. + StarStarUnterminatedTranslatedQuery +) + // TranslateQueryToMatchersWithTerminator converts a graphite query to tag // matcher pairs, and adds a terminator matcher to the end. func TranslateQueryToMatchersWithTerminator( query string, -) (models.Matchers, error) { +) (models.Matchers, TranslatedQueryType, error) { if strings.Contains(query, "**") { // First add matcher to ensure it's a graphite metric with __g0__ tag. hasFirstPathMatcher, err := convertMetricPartToMatcher(0, wildcard) if err != nil { - return nil, err + return nil, 0, err } // Need to regexp on the entire ID since ** matches over different // graphite path dimensions. @@ -110,7 +123,7 @@ func TranslateQueryToMatchersWithTerminator( } idRegexp, _, err := graphite.ExtendedGlobToRegexPattern(query, globOpts) if err != nil { - return nil, err + return nil, 0, err } return models.Matchers{ hasFirstPathMatcher, @@ -119,7 +132,7 @@ func TranslateQueryToMatchersWithTerminator( Name: doc.IDReservedFieldName, Value: idRegexp, }, - }, nil + }, StarStarUnterminatedTranslatedQuery, nil } metricLength := graphite.CountMetricParts(query) @@ -131,19 +144,20 @@ func TranslateQueryToMatchersWithTerminator( if len(metric) > 0 { m, err := convertMetricPartToMatcher(i, metric) if err != nil { - return nil, err + return nil, 0, err } matchers[i] = m } else { - return nil, fmt.Errorf("invalid matcher format: %s", query) + err := fmt.Errorf("invalid matcher format: %s", query) + return nil, 0, err } } // Add a terminator matcher at the end to ensure expansion is terminated at // the last given metric part. matchers[metricLength] = matcherTerminator(metricLength) - return matchers, nil + return matchers, TerminatedTranslatedQuery, nil } // GetQueryTerminatorTagName will return the name for the terminator matcher in @@ -158,7 +172,7 @@ func translateQuery( fetchOpts FetchOptions, opts M3WrappedStorageOptions, ) (*storage.FetchQuery, error) { - matchers, err := TranslateQueryToMatchersWithTerminator(query) + matchers, _, err := TranslateQueryToMatchersWithTerminator(query) if err != nil { return nil, err } diff --git a/src/query/graphite/storage/m3_wrapper_test.go b/src/query/graphite/storage/m3_wrapper_test.go index 8adfb06217..f71602b4c7 100644 --- a/src/query/graphite/storage/m3_wrapper_test.go +++ b/src/query/graphite/storage/m3_wrapper_test.go @@ -126,7 +126,7 @@ func TestTranslateQueryTrailingDot(t *testing.T) { assert.Nil(t, translated) assert.Error(t, err) - matchers, err := TranslateQueryToMatchersWithTerminator(query) + matchers, _, err := TranslateQueryToMatchersWithTerminator(query) assert.Nil(t, matchers) assert.Error(t, err) } @@ -216,7 +216,7 @@ func TestFetchByQuery(t *testing.T) { res.Metadata = block.ResultMetadata{ Exhaustive: false, LocalOnly: true, - Warnings: []block.Warning{block.Warning{Name: "foo", Message: "bar"}}, + Warnings: []block.Warning{{Name: "foo", Message: "bar"}}, Resolutions: []time.Duration{resolution}, }