From 1b849ebf923bb3b6c330daa6982c6a542f3661e8 Mon Sep 17 00:00:00 2001 From: Sven Rebhan <36194019+srebhan@users.noreply.github.com> Date: Fri, 28 Jun 2024 11:11:53 -0400 Subject: [PATCH] fix(inputs.gnmi): Handle YANG namespaces in paths correctly (#15565) --- plugins/inputs/gnmi/gnmi.go | 2 +- plugins/inputs/gnmi/handler.go | 19 +- plugins/inputs/gnmi/path.go | 164 +++++++++++++----- plugins/inputs/gnmi/tag_store.go | 6 +- .../gnmi/testcases/issue_15546/expected.out | 1 + .../gnmi/testcases/issue_15546/responses.json | 70 ++++++++ .../gnmi/testcases/issue_15546/telegraf.conf | 10 ++ 7 files changed, 214 insertions(+), 58 deletions(-) create mode 100644 plugins/inputs/gnmi/testcases/issue_15546/expected.out create mode 100644 plugins/inputs/gnmi/testcases/issue_15546/responses.json create mode 100644 plugins/inputs/gnmi/testcases/issue_15546/telegraf.conf diff --git a/plugins/inputs/gnmi/gnmi.go b/plugins/inputs/gnmi/gnmi.go index fd1f7a9a8..7b7319522 100644 --- a/plugins/inputs/gnmi/gnmi.go +++ b/plugins/inputs/gnmi/gnmi.go @@ -442,7 +442,7 @@ func (s *Subscription) buildAlias(aliases map[*pathInfo]string) error { // If the user didn't provide a measurement name, use last path element name := s.Name if name == "" && len(info.segments) > 0 { - name = info.segments[len(info.segments)-1] + name = info.segments[len(info.segments)-1].id } if name != "" { aliases[info] = name diff --git a/plugins/inputs/gnmi/handler.go b/plugins/inputs/gnmi/handler.go index a4e857b6b..9dddb8a1c 100644 --- a/plugins/inputs/gnmi/handler.go +++ b/plugins/inputs/gnmi/handler.go @@ -166,7 +166,7 @@ func (h *handler) handleSubscribeResponseUpdate(acc telegraf.Accumulator, respon // Add info to the tags headerTags["source"], _, _ = net.SplitHostPort(h.address) if !prefix.empty() { - headerTags["path"] = prefix.String() + headerTags["path"] = prefix.FullPath() } // Process and remove tag-updates from the response first so we can @@ -270,15 +270,11 @@ func (h *handler) handleSubscribeResponseUpdate(acc telegraf.Accumulator, respon // shorter than the full path to avoid an empty key, then strip the // common part of the field is prefixed with the alias path. Note // the origins can match or be empty and be considered equal. - if aliasInfo.isSubPathOf(field.path) && len(aliasInfo.segments) < len(field.path.segments) { - relative := field.path.segments[len(aliasInfo.segments):len(field.path.segments)] - key = strings.Join(relative, "/") + if relative := aliasInfo.relative(field.path, true); relative != "" { + key = relative } else { - // Otherwise use the last path element as the field key if it - // exists. - if len(field.path.segments) > 0 { - key = field.path.segments[len(field.path.segments)-1] - } + // Otherwise use the last path element as the field key + key = field.path.Base() } key = strings.ReplaceAll(key, "-", "_") } @@ -328,12 +324,11 @@ func guessPrefixFromUpdate(fields []updateField) string { return "" } if len(fields) == 1 { - dir, _ := fields[0].path.split() - return dir + return fields[0].path.Dir() } commonPath := &pathInfo{ origin: fields[0].path.origin, - segments: append([]string{}, fields[0].path.segments...), + segments: append([]segment{}, fields[0].path.segments...), } for _, f := range fields[1:] { commonPath.keepCommonPart(f.path) diff --git a/plugins/inputs/gnmi/path.go b/plugins/inputs/gnmi/path.go index 32636f250..cdb074be9 100644 --- a/plugins/inputs/gnmi/path.go +++ b/plugins/inputs/gnmi/path.go @@ -1,25 +1,26 @@ package gnmi import ( - "regexp" "strings" gnmiLib "github.com/openconfig/gnmi/proto/gnmi" ) -// Regular expression to see if a path element contains an origin -var originPattern = regexp.MustCompile(`^([\w-]+):`) - type keySegment struct { name string path string kv map[string]string } +type segment struct { + namespace string + id string +} + type pathInfo struct { origin string target string - segments []string + segments []segment keyValues []keySegment } @@ -29,10 +30,11 @@ func newInfoFromString(path string) *pathInfo { } info := &pathInfo{} - for _, s := range strings.Split(path, "/") { - if s != "" { - info.segments = append(info.segments, s) + for _, part := range strings.Split(path, "/") { + if part == "" { + continue } + info.segments = append(info.segments, segment{id: part}) } info.normalize() @@ -42,13 +44,13 @@ func newInfoFromString(path string) *pathInfo { func newInfoFromPathWithoutKeys(path *gnmiLib.Path) *pathInfo { info := &pathInfo{ origin: path.Origin, - segments: make([]string, 0, len(path.Elem)), + segments: make([]segment, 0, len(path.Elem)), } for _, elem := range path.Elem { if elem.Name == "" { continue } - info.segments = append(info.segments, elem.Name) + info.segments = append(info.segments, segment{id: elem.Name}) } info.normalize() @@ -74,7 +76,7 @@ func newInfoFromPath(paths ...*gnmiLib.Path) *pathInfo { if elem.Name == "" { continue } - info.segments = append(info.segments, elem.Name) + info.segments = append(info.segments, segment{id: elem.Name}) if len(elem.Key) == 0 { continue @@ -104,7 +106,7 @@ func (pi *pathInfo) append(paths ...*gnmiLib.Path) *pathInfo { path := &pathInfo{ origin: pi.origin, target: pi.target, - segments: append([]string{}, pi.segments...), + segments: append([]segment{}, pi.segments...), keyValues: make([]keySegment, 0, len(pi.keyValues)), } for _, elem := range pi.keyValues { @@ -125,7 +127,7 @@ func (pi *pathInfo) append(paths ...*gnmiLib.Path) *pathInfo { if elem.Name == "" { continue } - path.segments = append(path.segments, elem.Name) + path.segments = append(path.segments, segment{id: elem.Name}) if len(elem.Key) == 0 { continue @@ -151,7 +153,7 @@ func (pi *pathInfo) appendSegments(segments ...string) *pathInfo { path := &pathInfo{ origin: pi.origin, target: pi.target, - segments: append([]string{}, pi.segments...), + segments: append([]segment{}, pi.segments...), keyValues: make([]keySegment, 0, len(pi.keyValues)), } for _, elem := range pi.keyValues { @@ -171,7 +173,7 @@ func (pi *pathInfo) appendSegments(segments ...string) *pathInfo { if s == "" { continue } - path.segments = append(path.segments, s) + path.segments = append(path.segments, segment{id: s}) } path.normalize() @@ -183,18 +185,28 @@ func (pi *pathInfo) normalize() { return } - // Some devices supply the origin as part of the first path element, - // so try to find and extract it there. - groups := originPattern.FindStringSubmatch(pi.segments[0]) - if len(groups) == 2 { - pi.origin = groups[1] - pi.segments[0] = pi.segments[0][len(groups[1])+1:] - - // if we get empty string back, remove the segment - if pi.segments[0] == "" { - pi.segments = pi.segments[1:] + // Extract namespaces from segments + for i, s := range pi.segments { + if ns, id, found := strings.Cut(s.id, ":"); found { + pi.segments[i].namespace = ns + pi.segments[i].id = id } } + + // Some devices supply the origin as part of the first path element, + // so try to find and extract it there. + if pi.segments[0].namespace != "" { + pi.origin = pi.segments[0].namespace + } + + // Remove empty segments + segments := make([]segment, 0, len(pi.segments)) + for _, s := range pi.segments { + if s.id != "" { + segments = append(segments, s) + } + } + pi.segments = segments } func (pi *pathInfo) equalsPathNoKeys(path *gnmiLib.Path) bool { @@ -202,7 +214,7 @@ func (pi *pathInfo) equalsPathNoKeys(path *gnmiLib.Path) bool { return false } for i, s := range pi.segments { - if s != path.Elem[i].Name { + if s.id != path.Elem[i].Name { return false } } @@ -223,7 +235,11 @@ func (pi *pathInfo) isSubPathOf(path *pathInfo) bool { // Compare the elements and exit if we find a mismatch for i, p := range pi.segments { - if p != path.segments[i] { + ps := path.segments[i] + if p.namespace != "" && ps.namespace != "" && p.namespace != ps.namespace { + return false + } + if p.id != ps.id { return false } } @@ -231,6 +247,28 @@ func (pi *pathInfo) isSubPathOf(path *pathInfo) bool { return true } +func (pi *pathInfo) relative(path *pathInfo, withNamespace bool) string { + if !pi.isSubPathOf(path) || len(pi.segments) == len(path.segments) { + return "" + } + + segments := path.segments[len(pi.segments):len(path.segments)] + var r string + if withNamespace && segments[0].namespace != "" { + r = segments[0].namespace + ":" + segments[0].id + } else { + r = segments[0].id + } + for _, s := range segments[1:] { + if withNamespace && s.namespace != "" { + r += "/" + s.namespace + ":" + s.id + } else { + r += "/" + s.id + } + } + return r +} + func (pi *pathInfo) keepCommonPart(path *pathInfo) { shortestLen := len(pi.segments) if len(path.segments) < shortestLen { @@ -252,31 +290,35 @@ func (pi *pathInfo) keepCommonPart(path *pathInfo) { pi.segments = pi.segments[:matchLen] } -func (pi *pathInfo) split() (dir, base string) { - if len(pi.segments) == 0 { - return "", "" - } - if len(pi.segments) == 1 { - return "", pi.segments[0] +func (pi *pathInfo) Dir() string { + if len(pi.segments) <= 1 { + return "" } - dir = "/" + strings.Join(pi.segments[:len(pi.segments)-1], "/") + var dir string if pi.origin != "" { - dir = pi.origin + ":" + dir + dir = pi.origin + ":" } - return dir, pi.segments[len(pi.segments)-1] + for _, s := range pi.segments[:len(pi.segments)-1] { + if s.namespace != "" { + dir += "/" + s.namespace + ":" + s.id + } else { + dir += "/" + s.id + } + } + return dir } -func (pi *pathInfo) String() string { +func (pi *pathInfo) Base() string { if len(pi.segments) == 0 { return "" } - out := "/" + strings.Join(pi.segments, "/") - if pi.origin != "" { - out = pi.origin + ":" + out + s := pi.segments[len(pi.segments)-1] + if s.namespace != "" { + return s.namespace + ":" + s.id } - return out + return s.id } func (pi *pathInfo) Path() (origin, path string) { @@ -284,7 +326,45 @@ func (pi *pathInfo) Path() (origin, path string) { return pi.origin, "/" } - return pi.origin, "/" + strings.Join(pi.segments, "/") + for _, s := range pi.segments { + path += "/" + s.id + } + + return pi.origin, path +} + +func (pi *pathInfo) FullPath() string { + var path string + if pi.origin != "" { + path = pi.origin + ":" + } + if len(pi.segments) == 0 { + return path + } + + path += "/" + pi.segments[0].id + + for _, s := range pi.segments[1:] { + if s.namespace != "" { + path += "/" + s.namespace + ":" + s.id + } else { + path += "/" + s.id + } + } + + return path +} + +func (pi *pathInfo) String() string { + if len(pi.segments) == 0 { + return "" + } + + origin, path := pi.Path() + if origin != "" { + return origin + ":" + path + } + return path } func (pi *pathInfo) Tags(pathPrefix bool) map[string]string { diff --git a/plugins/inputs/gnmi/tag_store.go b/plugins/inputs/gnmi/tag_store.go index 357669734..c6eafc188 100644 --- a/plugins/inputs/gnmi/tag_store.go +++ b/plugins/inputs/gnmi/tag_store.go @@ -44,7 +44,7 @@ func (s *tagStore) insert(subscription TagSubscription, path *pathInfo, values [ for _, f := range values { tagName := subscription.Name if len(f.path.segments) > 0 { - key := f.path.segments[len(f.path.segments)-1] + key := f.path.Base() key = strings.ReplaceAll(key, "-", "_") tagName += "/" + key } @@ -74,7 +74,7 @@ func (s *tagStore) insert(subscription TagSubscription, path *pathInfo, values [ for _, f := range values { tagName := subscription.Name if len(f.path.segments) > 0 { - key := f.path.segments[len(f.path.segments)-1] + key := f.path.Base() key = strings.ReplaceAll(key, "-", "_") tagName += "/" + key } @@ -103,7 +103,7 @@ func (s *tagStore) insert(subscription TagSubscription, path *pathInfo, values [ for _, f := range values { tagName := subscription.Name if len(f.path.segments) > 0 { - key := f.path.segments[len(f.path.segments)-1] + key := f.path.Base() key = strings.ReplaceAll(key, "-", "_") tagName += "/" + key } diff --git a/plugins/inputs/gnmi/testcases/issue_15546/expected.out b/plugins/inputs/gnmi/testcases/issue_15546/expected.out new file mode 100644 index 000000000..10b92d886 --- /dev/null +++ b/plugins/inputs/gnmi/testcases/issue_15546/expected.out @@ -0,0 +1 @@ +event-stats,path=openconfig-system:/system/openconfig-events:event-stats/state,source=127.0.0.1 state/acked=0u,state/cleared=0u,state/events=4u,state/raised=0u 1718942414831832038 diff --git a/plugins/inputs/gnmi/testcases/issue_15546/responses.json b/plugins/inputs/gnmi/testcases/issue_15546/responses.json new file mode 100644 index 000000000..683d3f8ee --- /dev/null +++ b/plugins/inputs/gnmi/testcases/issue_15546/responses.json @@ -0,0 +1,70 @@ +[ + { + "update": { + "timestamp": "1718942414831832038", + "prefix": { + "elem": [ + { + "name": "openconfig-system:system" + }, + { + "name": "openconfig-events:event-stats" + }, + { + "name": "state" + } + ] + }, + "update": [ + { + "path": { + "elem": [ + { + "name": "acked" + } + ] + }, + "val": { + "uintVal": "0" + } + }, + { + "path": { + "elem": [ + { + "name": "cleared" + } + ] + }, + "val": { + "uintVal": "0" + } + }, + { + "path": { + "elem": [ + { + "name": "events" + } + ] + }, + "val": { + "uintVal": "4" + } + }, + { + "path": { + "elem": [ + { + "name": "raised" + } + ] + }, + "val": { + "uintVal": "0" + } + } + ] + } + } +] \ No newline at end of file diff --git a/plugins/inputs/gnmi/testcases/issue_15546/telegraf.conf b/plugins/inputs/gnmi/testcases/issue_15546/telegraf.conf new file mode 100644 index 000000000..8238a5234 --- /dev/null +++ b/plugins/inputs/gnmi/testcases/issue_15546/telegraf.conf @@ -0,0 +1,10 @@ +[[inputs.gnmi]] + addresses = ["dummy"] + prefix_tag_key_with_path = true + + [[inputs.gnmi.subscription]] + name = "event-stats" + origin = "openconfig-system" + path = "/system/event-stats" + subscription_mode = "sample" + sample_interval = "10s" \ No newline at end of file