From e66f7edfc92499e2f61c4d2ea442200dfd044bcf Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Tue, 29 Sep 2020 11:00:41 +0300 Subject: [PATCH] app/vmselect/graphite: properly handle case when `/metrics/find` finds both leaf and node for the given `query=prefix.*` In this case only node must be returned with stripped dot in the end of id as carbonapi does --- app/vmselect/graphite/graphite.go | 39 +++- .../graphite/metrics_find_response.qtpl | 10 +- .../graphite/metrics_find_response.qtpl.go | 184 +++++++++--------- 3 files changed, 135 insertions(+), 98 deletions(-) diff --git a/app/vmselect/graphite/graphite.go b/app/vmselect/graphite/graphite.go index af29fca48..c8593009c 100644 --- a/app/vmselect/graphite/graphite.go +++ b/app/vmselect/graphite/graphite.go @@ -83,10 +83,10 @@ func MetricsFindHandler(startTime time.Time, at *auth.Token, w http.ResponseWrit if isPartial && searchutils.GetDenyPartialResponse(r) { return fmt.Errorf("cannot return full response, since some of vmstorage nodes are unavailable") } - paths = deduplicatePaths(paths) if leavesOnly { paths = filterLeaves(paths, delimiter) } + paths = deduplicatePaths(paths, delimiter) sortPaths(paths, delimiter) contentType := "application/json" if jsonp != "" { @@ -103,13 +103,38 @@ func MetricsFindHandler(startTime time.Time, at *auth.Token, w http.ResponseWrit return nil } -func deduplicatePaths(paths []string) []string { - m := make(map[string]struct{}, len(paths)) - for _, path := range paths { - m[path] = struct{}{} +func deduplicatePaths(paths []string, delimiter string) []string { + if len(paths) == 0 { + return nil } - dst := make([]string, 0, len(m)) - for path := range m { + + sort.Strings(paths) + + // remove duplicates + dst := paths[:1] + for _, path := range paths[1:] { + prevPath := dst[len(dst)-1] + if path == prevPath { + // Skip duplicate path. + continue + } + dst = append(dst, path) + } + paths = dst + + // substitute `path` and `path` with `path` like carbonapi does. + // Such path is treated specially during rendering - see metrics_find_response.qtpl for details. + dst = paths[:1] + for _, path := range paths[1:] { + prevPath := dst[len(dst)-1] + if len(path) == len(prevPath)+1 && strings.HasSuffix(path, delimiter) && strings.HasPrefix(path, prevPath) { + // The path is equivalent to + + // Overwrite the prevPath with + as carbonapi does. + // I.e. the resulting path ends with double delimiter. + // Such path is treated specially during rendering - see metrics_find_response.qtpl for details. + dst[len(dst)-1] = path + delimiter + continue + } dst = append(dst, path) } return dst diff --git a/app/vmselect/graphite/metrics_find_response.qtpl b/app/vmselect/graphite/metrics_find_response.qtpl index d073bc7b6..1cc68c0b5 100644 --- a/app/vmselect/graphite/metrics_find_response.qtpl +++ b/app/vmselect/graphite/metrics_find_response.qtpl @@ -46,14 +46,20 @@ See https://graphite-api.readthedocs.io/en/latest/api.html#metrics-find {% for i, path := range paths %} { {% code + id := path allowChildren := "0" isLeaf := "1" - if strings.HasSuffix(path, delimiter) { + if strings.HasSuffix(id, delimiter) { + if strings.HasSuffix(id[:len(id)-1], delimiter) { + // Special case when id ends with double delimiter. + // See deduplicatePaths() code for details. + id = id[:len(id)-2] + } allowChildren = "1" isLeaf = "0" } %} - "id": {%q= path %}, + "id": {%q= id %}, "text": {%= metricPathName(path, delimiter) %}, "allowChildren": {%s= allowChildren %}, "expandable": {%s= allowChildren %}, diff --git a/app/vmselect/graphite/metrics_find_response.qtpl.go b/app/vmselect/graphite/metrics_find_response.qtpl.go index 00b8c2555..4133079b1 100644 --- a/app/vmselect/graphite/metrics_find_response.qtpl.go +++ b/app/vmselect/graphite/metrics_find_response.qtpl.go @@ -170,48 +170,54 @@ func streammetricsFindResponseTreeJSON(qw422016 *qt422016.Writer, paths []string //line app/vmselect/graphite/metrics_find_response.qtpl:46 qw422016.N().S(`{`) //line app/vmselect/graphite/metrics_find_response.qtpl:49 + id := path allowChildren := "0" isLeaf := "1" - if strings.HasSuffix(path, delimiter) { + if strings.HasSuffix(id, delimiter) { + if strings.HasSuffix(id[:len(id)-1], delimiter) { + // Special case when id ends with double delimiter. + // See deduplicatePaths() code for details. + id = id[:len(id)-2] + } allowChildren = "1" isLeaf = "0" } -//line app/vmselect/graphite/metrics_find_response.qtpl:55 +//line app/vmselect/graphite/metrics_find_response.qtpl:61 qw422016.N().S(`"id":`) -//line app/vmselect/graphite/metrics_find_response.qtpl:56 - qw422016.N().Q(path) -//line app/vmselect/graphite/metrics_find_response.qtpl:56 +//line app/vmselect/graphite/metrics_find_response.qtpl:62 + qw422016.N().Q(id) +//line app/vmselect/graphite/metrics_find_response.qtpl:62 qw422016.N().S(`,"text":`) -//line app/vmselect/graphite/metrics_find_response.qtpl:57 - streammetricPathName(qw422016, path, delimiter) -//line app/vmselect/graphite/metrics_find_response.qtpl:57 - qw422016.N().S(`,"allowChildren":`) -//line app/vmselect/graphite/metrics_find_response.qtpl:58 - qw422016.N().S(allowChildren) -//line app/vmselect/graphite/metrics_find_response.qtpl:58 - qw422016.N().S(`,"expandable":`) -//line app/vmselect/graphite/metrics_find_response.qtpl:59 - qw422016.N().S(allowChildren) -//line app/vmselect/graphite/metrics_find_response.qtpl:59 - qw422016.N().S(`,"leaf":`) -//line app/vmselect/graphite/metrics_find_response.qtpl:60 - qw422016.N().S(isLeaf) -//line app/vmselect/graphite/metrics_find_response.qtpl:60 - qw422016.N().S(`}`) -//line app/vmselect/graphite/metrics_find_response.qtpl:62 - if i+1 < len(paths) { -//line app/vmselect/graphite/metrics_find_response.qtpl:62 - qw422016.N().S(`,`) -//line app/vmselect/graphite/metrics_find_response.qtpl:62 - } //line app/vmselect/graphite/metrics_find_response.qtpl:63 + streammetricPathName(qw422016, path, delimiter) +//line app/vmselect/graphite/metrics_find_response.qtpl:63 + qw422016.N().S(`,"allowChildren":`) +//line app/vmselect/graphite/metrics_find_response.qtpl:64 + qw422016.N().S(allowChildren) +//line app/vmselect/graphite/metrics_find_response.qtpl:64 + qw422016.N().S(`,"expandable":`) +//line app/vmselect/graphite/metrics_find_response.qtpl:65 + qw422016.N().S(allowChildren) +//line app/vmselect/graphite/metrics_find_response.qtpl:65 + qw422016.N().S(`,"leaf":`) +//line app/vmselect/graphite/metrics_find_response.qtpl:66 + qw422016.N().S(isLeaf) +//line app/vmselect/graphite/metrics_find_response.qtpl:66 + qw422016.N().S(`}`) +//line app/vmselect/graphite/metrics_find_response.qtpl:68 + if i+1 < len(paths) { +//line app/vmselect/graphite/metrics_find_response.qtpl:68 + qw422016.N().S(`,`) +//line app/vmselect/graphite/metrics_find_response.qtpl:68 + } +//line app/vmselect/graphite/metrics_find_response.qtpl:69 } -//line app/vmselect/graphite/metrics_find_response.qtpl:64 +//line app/vmselect/graphite/metrics_find_response.qtpl:70 if addWildcards && len(paths) > 1 { -//line app/vmselect/graphite/metrics_find_response.qtpl:64 +//line app/vmselect/graphite/metrics_find_response.qtpl:70 qw422016.N().S(`,{`) -//line app/vmselect/graphite/metrics_find_response.qtpl:67 +//line app/vmselect/graphite/metrics_find_response.qtpl:73 path := paths[0] for strings.HasSuffix(path, delimiter) { path = path[:len(path)-1] @@ -232,60 +238,60 @@ func streammetricsFindResponseTreeJSON(qw422016 *qt422016.Writer, paths []string } } -//line app/vmselect/graphite/metrics_find_response.qtpl:86 +//line app/vmselect/graphite/metrics_find_response.qtpl:92 qw422016.N().S(`"id":`) -//line app/vmselect/graphite/metrics_find_response.qtpl:87 +//line app/vmselect/graphite/metrics_find_response.qtpl:93 qw422016.N().Q(id) -//line app/vmselect/graphite/metrics_find_response.qtpl:87 +//line app/vmselect/graphite/metrics_find_response.qtpl:93 qw422016.N().S(`,"text": "*","allowChildren":`) -//line app/vmselect/graphite/metrics_find_response.qtpl:89 +//line app/vmselect/graphite/metrics_find_response.qtpl:95 qw422016.N().S(allowChildren) -//line app/vmselect/graphite/metrics_find_response.qtpl:89 +//line app/vmselect/graphite/metrics_find_response.qtpl:95 qw422016.N().S(`,"expandable":`) -//line app/vmselect/graphite/metrics_find_response.qtpl:90 +//line app/vmselect/graphite/metrics_find_response.qtpl:96 qw422016.N().S(allowChildren) -//line app/vmselect/graphite/metrics_find_response.qtpl:90 +//line app/vmselect/graphite/metrics_find_response.qtpl:96 qw422016.N().S(`,"leaf":`) -//line app/vmselect/graphite/metrics_find_response.qtpl:91 - qw422016.N().S(isLeaf) -//line app/vmselect/graphite/metrics_find_response.qtpl:91 - qw422016.N().S(`}`) -//line app/vmselect/graphite/metrics_find_response.qtpl:93 - } -//line app/vmselect/graphite/metrics_find_response.qtpl:93 - qw422016.N().S(`]`) -//line app/vmselect/graphite/metrics_find_response.qtpl:95 -} - -//line app/vmselect/graphite/metrics_find_response.qtpl:95 -func writemetricsFindResponseTreeJSON(qq422016 qtio422016.Writer, paths []string, delimiter string, addWildcards bool) { -//line app/vmselect/graphite/metrics_find_response.qtpl:95 - qw422016 := qt422016.AcquireWriter(qq422016) -//line app/vmselect/graphite/metrics_find_response.qtpl:95 - streammetricsFindResponseTreeJSON(qw422016, paths, delimiter, addWildcards) -//line app/vmselect/graphite/metrics_find_response.qtpl:95 - qt422016.ReleaseWriter(qw422016) -//line app/vmselect/graphite/metrics_find_response.qtpl:95 -} - -//line app/vmselect/graphite/metrics_find_response.qtpl:95 -func metricsFindResponseTreeJSON(paths []string, delimiter string, addWildcards bool) string { -//line app/vmselect/graphite/metrics_find_response.qtpl:95 - qb422016 := qt422016.AcquireByteBuffer() -//line app/vmselect/graphite/metrics_find_response.qtpl:95 - writemetricsFindResponseTreeJSON(qb422016, paths, delimiter, addWildcards) -//line app/vmselect/graphite/metrics_find_response.qtpl:95 - qs422016 := string(qb422016.B) -//line app/vmselect/graphite/metrics_find_response.qtpl:95 - qt422016.ReleaseByteBuffer(qb422016) -//line app/vmselect/graphite/metrics_find_response.qtpl:95 - return qs422016 -//line app/vmselect/graphite/metrics_find_response.qtpl:95 -} - //line app/vmselect/graphite/metrics_find_response.qtpl:97 -func streammetricPathName(qw422016 *qt422016.Writer, path, delimiter string) { + qw422016.N().S(isLeaf) +//line app/vmselect/graphite/metrics_find_response.qtpl:97 + qw422016.N().S(`}`) //line app/vmselect/graphite/metrics_find_response.qtpl:99 + } +//line app/vmselect/graphite/metrics_find_response.qtpl:99 + qw422016.N().S(`]`) +//line app/vmselect/graphite/metrics_find_response.qtpl:101 +} + +//line app/vmselect/graphite/metrics_find_response.qtpl:101 +func writemetricsFindResponseTreeJSON(qq422016 qtio422016.Writer, paths []string, delimiter string, addWildcards bool) { +//line app/vmselect/graphite/metrics_find_response.qtpl:101 + qw422016 := qt422016.AcquireWriter(qq422016) +//line app/vmselect/graphite/metrics_find_response.qtpl:101 + streammetricsFindResponseTreeJSON(qw422016, paths, delimiter, addWildcards) +//line app/vmselect/graphite/metrics_find_response.qtpl:101 + qt422016.ReleaseWriter(qw422016) +//line app/vmselect/graphite/metrics_find_response.qtpl:101 +} + +//line app/vmselect/graphite/metrics_find_response.qtpl:101 +func metricsFindResponseTreeJSON(paths []string, delimiter string, addWildcards bool) string { +//line app/vmselect/graphite/metrics_find_response.qtpl:101 + qb422016 := qt422016.AcquireByteBuffer() +//line app/vmselect/graphite/metrics_find_response.qtpl:101 + writemetricsFindResponseTreeJSON(qb422016, paths, delimiter, addWildcards) +//line app/vmselect/graphite/metrics_find_response.qtpl:101 + qs422016 := string(qb422016.B) +//line app/vmselect/graphite/metrics_find_response.qtpl:101 + qt422016.ReleaseByteBuffer(qb422016) +//line app/vmselect/graphite/metrics_find_response.qtpl:101 + return qs422016 +//line app/vmselect/graphite/metrics_find_response.qtpl:101 +} + +//line app/vmselect/graphite/metrics_find_response.qtpl:103 +func streammetricPathName(qw422016 *qt422016.Writer, path, delimiter string) { +//line app/vmselect/graphite/metrics_find_response.qtpl:105 name := path for strings.HasSuffix(name, delimiter) { name = name[:len(name)-1] @@ -294,33 +300,33 @@ func streammetricPathName(qw422016 *qt422016.Writer, path, delimiter string) { name = name[n+1:] } -//line app/vmselect/graphite/metrics_find_response.qtpl:107 +//line app/vmselect/graphite/metrics_find_response.qtpl:113 qw422016.N().Q(name) -//line app/vmselect/graphite/metrics_find_response.qtpl:108 +//line app/vmselect/graphite/metrics_find_response.qtpl:114 } -//line app/vmselect/graphite/metrics_find_response.qtpl:108 +//line app/vmselect/graphite/metrics_find_response.qtpl:114 func writemetricPathName(qq422016 qtio422016.Writer, path, delimiter string) { -//line app/vmselect/graphite/metrics_find_response.qtpl:108 +//line app/vmselect/graphite/metrics_find_response.qtpl:114 qw422016 := qt422016.AcquireWriter(qq422016) -//line app/vmselect/graphite/metrics_find_response.qtpl:108 +//line app/vmselect/graphite/metrics_find_response.qtpl:114 streammetricPathName(qw422016, path, delimiter) -//line app/vmselect/graphite/metrics_find_response.qtpl:108 +//line app/vmselect/graphite/metrics_find_response.qtpl:114 qt422016.ReleaseWriter(qw422016) -//line app/vmselect/graphite/metrics_find_response.qtpl:108 +//line app/vmselect/graphite/metrics_find_response.qtpl:114 } -//line app/vmselect/graphite/metrics_find_response.qtpl:108 +//line app/vmselect/graphite/metrics_find_response.qtpl:114 func metricPathName(path, delimiter string) string { -//line app/vmselect/graphite/metrics_find_response.qtpl:108 +//line app/vmselect/graphite/metrics_find_response.qtpl:114 qb422016 := qt422016.AcquireByteBuffer() -//line app/vmselect/graphite/metrics_find_response.qtpl:108 +//line app/vmselect/graphite/metrics_find_response.qtpl:114 writemetricPathName(qb422016, path, delimiter) -//line app/vmselect/graphite/metrics_find_response.qtpl:108 +//line app/vmselect/graphite/metrics_find_response.qtpl:114 qs422016 := string(qb422016.B) -//line app/vmselect/graphite/metrics_find_response.qtpl:108 +//line app/vmselect/graphite/metrics_find_response.qtpl:114 qt422016.ReleaseByteBuffer(qb422016) -//line app/vmselect/graphite/metrics_find_response.qtpl:108 +//line app/vmselect/graphite/metrics_find_response.qtpl:114 return qs422016 -//line app/vmselect/graphite/metrics_find_response.qtpl:108 +//line app/vmselect/graphite/metrics_find_response.qtpl:114 }