fix(parsers.xpath): Fix field-names for arrays of simple types (#13665)

plugins/parsers/xpath/cbor_document.go

@@ -44,19 +44,8 @@ func (d *cborDocument) GetNodePath(node, relativeTo dataNode, sep string) string
 	// Climb up the tree and collect the node names
 	n := nativeNode.Parent
 	for n != nil && n != nativeRelativeTo {
-		kind := reflect.Invalid
+		nodeName := d.GetNodeName(n, sep, false)
-		if n.Parent != nil && n.Parent.Value() != nil {
+		names = append(names, nodeName)
-			kind = reflect.TypeOf(n.Parent.Value()).Kind()
-		}
-
-		switch kind {
-		case reflect.Slice, reflect.Array:
-			// Determine the index for array elements
-			names = append(names, d.index(n))
-		default:
-			// Use the name if not an array
-			names = append(names, n.Name)
-		}
 		n = n.Parent
 	}
 
@@ -73,6 +62,31 @@ func (d *cborDocument) GetNodePath(node, relativeTo dataNode, sep string) string
 	return nodepath[:len(nodepath)-1]
 }
 
+func (d *cborDocument) GetNodeName(node dataNode, sep string, withParent bool) string {
+	// If this panics it's a programming error as we changed the document type while processing
+	nativeNode := node.(*cborquery.Node)
+
+	name := nativeNode.Name
+
+	// Check if the node is part of an array. If so, determine the index and
+	// concatenate the parent name and the index.
+	kind := reflect.Invalid
+	if nativeNode.Parent != nil && nativeNode.Parent.Value() != nil {
+		kind = reflect.TypeOf(nativeNode.Parent.Value()).Kind()
+	}
+
+	switch kind {
+	case reflect.Slice, reflect.Array:
+		// Determine the index for array elements
+		if name == "" && nativeNode.Parent != nil && withParent {
+			name = nativeNode.Parent.Name + sep
+		}
+		return name + d.index(nativeNode)
+	}
+
+	return name
+}
+
 func (d *cborDocument) OutputXML(node dataNode) string {
 	native := node.(*cborquery.Node)
 	return native.OutputXML()

plugins/parsers/xpath/json_document.go

@@ -44,19 +44,8 @@ func (d *jsonDocument) GetNodePath(node, relativeTo dataNode, sep string) string
 	// Climb up the tree and collect the node names
 	n := nativeNode.Parent
 	for n != nil && n != nativeRelativeTo {
-		kind := reflect.Invalid
+		nodeName := d.GetNodeName(n, sep, false)
-		if n.Parent != nil && n.Parent.Value() != nil {
+		names = append(names, nodeName)
-			kind = reflect.TypeOf(n.Parent.Value()).Kind()
-		}
-
-		switch kind {
-		case reflect.Slice, reflect.Array:
-			// Determine the index for array elements
-			names = append(names, d.index(n))
-		default:
-			// Use the name if not an array
-			names = append(names, n.Data)
-		}
 		n = n.Parent
 	}
 
@@ -73,6 +62,31 @@ func (d *jsonDocument) GetNodePath(node, relativeTo dataNode, sep string) string
 	return nodepath[:len(nodepath)-1]
 }
 
+func (d *jsonDocument) GetNodeName(node dataNode, sep string, withParent bool) string {
+	// If this panics it's a programming error as we changed the document type while processing
+	nativeNode := node.(*jsonquery.Node)
+
+	name := nativeNode.Data
+
+	// Check if the node is part of an array. If so, determine the index and
+	// concatenate the parent name and the index.
+	kind := reflect.Invalid
+	if nativeNode.Parent != nil && nativeNode.Parent.Value() != nil {
+		kind = reflect.TypeOf(nativeNode.Parent.Value()).Kind()
+	}
+
+	switch kind {
+	case reflect.Slice, reflect.Array:
+		// Determine the index for array elements
+		if name == "" && nativeNode.Parent != nil && withParent {
+			name = nativeNode.Parent.Data + sep
+		}
+		return name + d.index(nativeNode)
+	}
+
+	return name
+}
+
 func (d *jsonDocument) OutputXML(node dataNode) string {
 	native := node.(*jsonquery.Node)
 	return native.OutputXML()

plugins/parsers/xpath/msgpack_document.go

@@ -32,6 +32,9 @@ func (d *msgpackDocument) CreateXPathNavigator(node dataNode) path.NodeNavigator
 func (d *msgpackDocument) GetNodePath(node, relativeTo dataNode, sep string) string {
 	return (*jsonDocument)(d).GetNodePath(node, relativeTo, sep)
 }
+func (d *msgpackDocument) GetNodeName(node dataNode, sep string, withParent bool) string {
+	return (*jsonDocument)(d).GetNodeName(node, sep, withParent)
+}
 
 func (d *msgpackDocument) OutputXML(node dataNode) string {
 	return (*jsonDocument)(d).OutputXML(node)

plugins/parsers/xpath/parser.go

@@ -28,6 +28,7 @@ type dataDocument interface {
 	QueryAll(node dataNode, expr string) ([]dataNode, error)
 	CreateXPathNavigator(node dataNode) path.NodeNavigator
 	GetNodePath(node, relativeTo dataNode, sep string) string
+	GetNodeName(node dataNode, sep string, withParent bool) string
 	OutputXML(node dataNode) string
 }
 
@@ -324,18 +325,13 @@ func (p *Parser) parseQuery(starttime time.Time, doc, selected dataNode, config
 				if !ok {
 					return nil, fmt.Errorf("failed to query tag name with query %q: result is not a string (%v)", tagnamequery, n)
 				}
+				name = p.constructFieldName(selected, selectedtag, name, config.TagNameExpand)
+
 				v, err := p.executeQuery(doc, selectedtag, tagvaluequery)
 				if err != nil {
 					return nil, fmt.Errorf("failed to query tag value for %q: %w", name, err)
 				}
 
-				if config.TagNameExpand {
-					p := p.document.GetNodePath(selectedtag, selected, "_")
-					if len(p) > 0 {
-						name = p + "_" + name
-					}
-				}
-
 				// Check if field name already exists and if so, append an index number.
 				if _, ok := tags[name]; ok {
 					for i := 1; ; i++ {
@@ -434,18 +430,13 @@ func (p *Parser) parseQuery(starttime time.Time, doc, selected dataNode, config
 				if !ok {
 					return nil, fmt.Errorf("failed to query field name with query %q: result is not a string (%v)", fieldnamequery, n)
 				}
+				name = p.constructFieldName(selected, selectedfield, name, config.FieldNameExpand)
+
 				v, err := p.executeQuery(doc, selectedfield, fieldvaluequery)
 				if err != nil {
 					return nil, fmt.Errorf("failed to query field value for %q: %w", name, err)
 				}
 
-				if config.FieldNameExpand {
-					p := p.document.GetNodePath(selectedfield, selected, "_")
-					if len(p) > 0 {
-						name = p + "_" + name
-					}
-				}
-
 				// Check if field name already exists and if so, append an index number.
 				if _, ok := fields[name]; ok {
 					for i := 1; ; i++ {
@@ -567,6 +558,30 @@ func splitLastPathElement(query string) []string {
 	return elements
 }
 
+func (p *Parser) constructFieldName(root, node dataNode, name string, expand bool) string {
+	var expansion string
+
+	// In case the name is empty we should determine the current node's name.
+	// This involves array index expansion in case the parent of the node is
+	// and array. If we expanded here, we should skip our parent as this is
+	// already encoded in the name
+	if name == "" {
+		name = p.document.GetNodeName(node, "_", !expand)
+	}
+
+	// If name expansion is requested, construct a path between the current
+	// node and the root node of the selection. Concatenate the elements with
+	// an underscore.
+	if expand {
+		expansion = p.document.GetNodePath(node, root, "_")
+	}
+
+	if len(expansion) > 0 {
+		name = expansion + "_" + name
+	}
+	return name
+}
+
 func (p *Parser) debugEmptyQuery(operation string, root dataNode, initialquery string) {
 	if p.Log == nil {
 		return

plugins/parsers/xpath/protocolbuffer_document.go

@@ -3,7 +3,9 @@ package xpath
 import (
 	"encoding/hex"
 	"fmt"
+	"reflect"
 	"sort"
+	"strconv"
 	"strings"
 
 	path "github.com/antchfx/xpath"
@@ -132,7 +134,19 @@ func (d *protobufDocument) GetNodePath(node, relativeTo dataNode, sep string) st
 	// Climb up the tree and collect the node names
 	n := nativeNode.Parent
 	for n != nil && n != nativeRelativeTo {
-		names = append(names, n.Name)
+		kind := reflect.Invalid
+		if n.Parent != nil && n.Parent.Value() != nil {
+			kind = reflect.TypeOf(n.Parent.Value()).Kind()
+		}
+		fmt.Printf("node %q: %v (value=%v)   parent: %v\n", n.Data, kind, n.Value(), n.Parent.Value())
+		switch kind {
+		case reflect.Slice, reflect.Array:
+			// Determine the index for array elements
+			names = append(names, d.index(n))
+		default:
+			// Use the name if not an array
+			names = append(names, n.Name)
+		}
 		n = n.Parent
 	}
 
@@ -149,10 +163,41 @@ func (d *protobufDocument) GetNodePath(node, relativeTo dataNode, sep string) st
 	return nodepath[:len(nodepath)-1]
 }
 
+func (d *protobufDocument) GetNodeName(node dataNode, sep string, withParent bool) string {
+	// If this panics it's a programming error as we changed the document type while processing
+	nativeNode := node.(*protobufquery.Node)
+
+	name := nativeNode.Name
+
+	// Check if the node is part of an array. If so, determine the index and
+	// concatenate the parent name and the index.
+	kind := reflect.Invalid
+	if nativeNode.Parent != nil && nativeNode.Parent.Value() != nil {
+		kind = reflect.TypeOf(nativeNode.Parent.Value()).Kind()
+	}
+
+	switch kind {
+	case reflect.Slice, reflect.Array:
+		if name == "" && nativeNode.Parent != nil && withParent {
+			name = nativeNode.Parent.Name + sep
+		}
+		return name + d.index(nativeNode)
+	}
+
+	return name
+}
+
 func (d *protobufDocument) OutputXML(node dataNode) string {
 	native := node.(*protobufquery.Node)
 	return native.OutputXML()
 }
 
-func init() {
+func (d *protobufDocument) index(node *protobufquery.Node) string {
+	idx := 0
+
+	for n := node; n.PrevSibling != nil; n = n.PrevSibling {
+		idx++
+	}
+
+	return strconv.Itoa(idx)
 }

plugins/parsers/xpath/testcases/json_array_expand/telegraf.conf

@@ -7,6 +7,6 @@
   [[inputs.file.xpath]]
     field_name_expansion = true
     metric_name = "'foo'"
-    field_selection = "descendant::*"
+    field_selection = "descendant::*[not(*)]"
     timestamp = "//timestamp"
     timestamp_format = "unix_ns"

plugins/parsers/xpath/testcases/json_array_expand_simple_types/expected.out

@@ -0,0 +1 @@
+foo name="PC1",cpus_0="cpu1",cpus_1="cpu2",cpus_2="cpu3",disks_sata_0="disk1",disks_sata_1="disk2",disks_nvme_0="disk3",disks_nvme_1="disk4",disks_nvme_2="disk5",timestamp=1690218699 1690218699000000000

plugins/parsers/xpath/testcases/json_array_expand_simple_types/telegraf.conf

@@ -0,0 +1,12 @@
+[[inputs.file]]
+  files = ["./testcases/json_array_expand_simple_types/test.json"]
+  data_format = "xpath_json"
+
+  xpath_native_types = true
+
+  [[inputs.file.xpath]]
+    metric_name = "'foo'"
+    field_selection = "descendant::*[not(*)]"
+    field_name_expansion = true
+    timestamp = "//timestamp"
+    timestamp_format = "unix"

plugins/parsers/xpath/testcases/json_array_expand_simple_types/test.json

@@ -0,0 +1,20 @@
+{
+   "name": "PC1",
+   "cpus": [
+     "cpu1",
+     "cpu2",
+     "cpu3"
+   ],
+   "disks": {
+     "sata": [
+       "disk1",
+       "disk2"
+     ],
+     "nvme": [
+       "disk3",
+       "disk4",
+       "disk5"
+     ]
+   },
+   "timestamp": 1690218699
+ }

plugins/parsers/xpath/testcases/json_array_simple_types/expected.out

@@ -0,0 +1 @@
+foo name="PC1",cpus_0="cpu1",cpus_1="cpu2",cpus_2="cpu3",sata_0="disk1",sata_1="disk2",nvme_0="disk3",nvme_1="disk4",nvme_2="disk5",timestamp=1690218699 1690218699000000000

plugins/parsers/xpath/testcases/json_array_simple_types/telegraf.conf

@@ -0,0 +1,11 @@
+[[inputs.file]]
+  files = ["./testcases/json_array_simple_types/test.json"]
+  data_format = "xpath_json"
+
+  xpath_native_types = true
+
+  [[inputs.file.xpath]]
+    metric_name = "'foo'"
+    field_selection = "descendant::*[not(*)]"
+    timestamp = "//timestamp"
+    timestamp_format = "unix"

plugins/parsers/xpath/testcases/json_array_simple_types/test.json

@@ -0,0 +1,20 @@
+{
+   "name": "PC1",
+   "cpus": [
+     "cpu1",
+     "cpu2",
+     "cpu3"
+   ],
+   "disks": {
+     "sata": [
+       "disk1",
+       "disk2"
+     ],
+     "nvme": [
+       "disk3",
+       "disk4",
+       "disk5"
+     ]
+   },
+   "timestamp": 1690218699
+ }

plugins/parsers/xpath/xml_document.go

@@ -42,7 +42,8 @@ func (d *xmlDocument) GetNodePath(node, relativeTo dataNode, sep string) string
 	// Climb up the tree and collect the node names
 	n := nativeNode.Parent
 	for n != nil && n != nativeRelativeTo {
-		names = append(names, n.Data)
+		nodeName := d.GetNodeName(n, sep, false)
+		names = append(names, nodeName)
 		n = n.Parent
 	}
 
@@ -59,6 +60,13 @@ func (d *xmlDocument) GetNodePath(node, relativeTo dataNode, sep string) string
 	return nodepath[:len(nodepath)-1]
 }
 
+func (d *xmlDocument) GetNodeName(node dataNode, _ string, _ bool) string {
+	// If this panics it's a programming error as we changed the document type while processing
+	nativeNode := node.(*xmlquery.Node)
+
+	return nativeNode.Data
+}
+
 func (d *xmlDocument) OutputXML(node dataNode) string {
 	native := node.(*xmlquery.Node)
 	return native.OutputXML(false)