Skip to content

Commit

Permalink
fix(inputs.gnmi): Refactor alias handling
Browse files Browse the repository at this point in the history
Rather than abritrarily take the substring of key, ensure that the key
contains the alias in the first place, then do the removal. Otherwise,
always take the base path.

fixes: #14530
  • Loading branch information
powersj committed Jan 22, 2024
1 parent 2d4e860 commit b89861c
Show file tree
Hide file tree
Showing 5 changed files with 241 additions and 11 deletions.
24 changes: 13 additions & 11 deletions plugins/inputs/gnmi/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"fmt"
"io"
"net"
"path"
"sort"
"strconv"
"strings"
Expand Down Expand Up @@ -231,30 +230,33 @@ func (h *handler) handleSubscribeResponseUpdate(acc telegraf.Accumulator, respon
h.emptyNameWarnShown = true
}
}
aliasInfo := newInfoFromString(aliasPath)

// Group metrics
fieldPath := field.path.String()
key := strings.ReplaceAll(fieldPath, "-", "_")
var key string
if h.canonicalFieldNames {
// Strip the origin is any for the field names
if parts := strings.SplitN(key, ":", 2); len(parts) == 2 {
if parts := strings.SplitN(strings.ReplaceAll(field.path.String(), "-", "_"), ":", 2); len(parts) == 2 {
key = parts[1]
}
} else {
if len(aliasPath) < len(key) && len(aliasPath) != 0 {
// This may not be an exact prefix, due to naming style
// conversion on the key.
key = key[len(aliasPath)+1:]
} else if len(aliasPath) >= len(key) {
// If the origis are the same and the alias is shorter than the
// full path to avoid an empty key, then strip the common part if
// the field is prefixed with the alias path.
if aliasInfo.origin == field.path.origin && 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, "/")
} else {
// Otherwise use the last path element as the field key.
key = path.Base(key)
key = field.path.segments[len(field.path.segments)-1]
}
key = strings.ReplaceAll(key, "-", "_")
}
if h.trimSlash {
key = strings.TrimLeft(key, "/.")
}
if key == "" {
h.log.Errorf("Invalid empty path %q with alias %q", fieldPath, aliasPath)
h.log.Errorf("Invalid empty path %q with alias %q", field.path.String(), aliasPath)
continue
}
grouper.Add(name, tags, timestamp, key, field.value)
Expand Down
5 changes: 5 additions & 0 deletions plugins/inputs/gnmi/path.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,11 @@ func (pi *pathInfo) normalize() {
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:]
}
}
}

Expand Down
1 change: 1 addition & 0 deletions plugins/inputs/gnmi/testcases/issue_14530/expected.out
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ifcounters,name=Ethernet35,path=/interfaces/interface/state/counters,source=127.0.0.1 in_broadcast_pkts=0u,in_discards=0u,in_errors=0u,in_fcs_errors=0u,in_multicast_pkts=0u,in_octets=0u,in_pkts=0u,in_unicast_pkts=0u,out_broadcast_pkts=0u,out_discards=0u,out_errors=0u,out_multicast_pkts=0u,out_octets=0u,out_pkts=0u,out_unicast_pkts=0u 1704442117721474264
208 changes: 208 additions & 0 deletions plugins/inputs/gnmi/testcases/issue_14530/responses.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,208 @@
[
{
"update": {
"timestamp": "1704442117721474264",
"prefix": {
"elem": [
{
"name": "interfaces"
},
{
"name": "interface",
"key": {
"name": "Ethernet35"
}
},
{
"name": "state"
},
{
"name": "counters"
}
]
},
"update": [
{
"path": {
"elem": [
{
"name": "in-broadcast-pkts"
}
]
},
"val": {
"uintVal": "0"
}
},
{
"path": {
"elem": [
{
"name": "in-discards"
}
]
},
"val": {
"uintVal": "0"
}
},
{
"path": {
"elem": [
{
"name": "in-errors"
}
]
},
"val": {
"uintVal": "0"
}
},
{
"path": {
"elem": [
{
"name": "in-fcs-errors"
}
]
},
"val": {
"uintVal": "0"
}
},
{
"path": {
"elem": [
{
"name": "in-multicast-pkts"
}
]
},
"val": {
"uintVal": "0"
}
},
{
"path": {
"elem": [
{
"name": "in-octets"
}
]
},
"val": {
"uintVal": "0"
}
},
{
"path": {
"elem": [
{
"name": "in-pkts"
}
]
},
"val": {
"uintVal": "0"
}
},
{
"path": {
"elem": [
{
"name": "in-unicast-pkts"
}
]
},
"val": {
"uintVal": "0"
}
},
{
"path": {
"elem": [
{
"name": "out-broadcast-pkts"
}
]
},
"val": {
"uintVal": "0"
}
},
{
"path": {
"elem": [
{
"name": "out-discards"
}
]
},
"val": {
"uintVal": "0"
}
},
{
"path": {
"elem": [
{
"name": "out-errors"
}
]
},
"val": {
"uintVal": "0"
}
},
{
"path": {
"elem": [
{
"name": "out-multicast-pkts"
}
]
},
"val": {
"uintVal": "0"
}
},
{
"path": {
"elem": [
{
"name": "out-octets"
}
]
},
"val": {
"uintVal": "0"
}
},
{
"path": {
"elem": [
{
"name": "out-pkts"
}
]
},
"val": {
"uintVal": "0"
}
},
{
"path": {
"elem": [
{
"name": "out-unicast-pkts"
}
]
},
"val": {
"uintVal": "0"
}
}
]
}
}
]
14 changes: 14 additions & 0 deletions plugins/inputs/gnmi/testcases/issue_14530/telegraf.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[[inputs.gnmi]]
addresses = ["dummy"]
encoding = "json_ietf"
tagexclude = ["path"]

[inputs.gnmi.tags]
test_tag = "test"

[[inputs.gnmi.subscription]]
name = "ifcounters"
origin = "openconfig"
path = "/interfaces/interface/state/counters"
subscription_mode = "sample"
sample_interval = "10s"

0 comments on commit b89861c

Please sign in to comment.