feat: Migrate xpath parser to new style

config/config.go

@@ -1842,9 +1842,7 @@ func (c *Config) missingTomlField(_ reflect.Type, key string) error {
 		"prefix", "prometheus_export_timestamp", "prometheus_ignore_timestamp", "prometheus_sort_metrics", "prometheus_string_as_label",
 		"separator", "splunkmetric_hec_routing", "splunkmetric_multimetric", "tag_keys",
 		"tagdrop", "tagexclude", "taginclude", "tagpass", "tags", "template", "templates",
-		"value_field_name", "wavefront_source_override", "wavefront_use_strict", "wavefront_disable_prefix_conversion",
-		"xml", "xpath", "xpath_json", "xpath_msgpack", "xpath_protobuf", "xpath_print_document",
-		"xpath_protobuf_file", "xpath_protobuf_type", "xpath_protobuf_import_paths":
+		"value_field_name", "wavefront_source_override", "wavefront_use_strict", "wavefront_disable_prefix_conversion":
 
 		// ignore fields that are common to all plugins.
 	default:

config/config_test.go

@@ -406,7 +406,6 @@ func TestConfig_ParserInterfaceNewFormat(t *testing.T) {
 	}
 
 	override := map[string]struct {
-		cfg   *parsers.Config
 		param map[string]interface{}
 		mask  []string
 	}{
@@ -420,11 +419,6 @@ func TestConfig_ParserInterfaceNewFormat(t *testing.T) {
 			mask: []string{"Now"},
 		},
 		"xpath_protobuf": {
-			cfg: &parsers.Config{
-				MetricName:        "parser_test_new",
-				XPathProtobufFile: "testdata/addressbook.proto",
-				XPathProtobufType: "addressbook.AddressBook",
-			},
 			param: map[string]interface{}{
 				"ProtobufMessageDef":  "testdata/addressbook.proto",
 				"ProtobufMessageType": "addressbook.AddressBook",
@@ -435,10 +429,6 @@ func TestConfig_ParserInterfaceNewFormat(t *testing.T) {
 	expected := make([]telegraf.Parser, 0, len(formats))
 	for _, format := range formats {
 		formatCfg := &cfg
-		settings, hasOverride := override[format]
-		if hasOverride && settings.cfg != nil {
-			formatCfg = settings.cfg
-		}
 		formatCfg.DataFormat = format
 
 		logger := models.NewLogger("parsers", format, cfg.MetricName)
@@ -555,7 +545,6 @@ func TestConfig_ParserInterfaceOldFormat(t *testing.T) {
 	}
 
 	override := map[string]struct {
-		cfg   *parsers.Config
 		param map[string]interface{}
 		mask  []string
 	}{
@@ -569,11 +558,6 @@ func TestConfig_ParserInterfaceOldFormat(t *testing.T) {
 			mask: []string{"Now"},
 		},
 		"xpath_protobuf": {
-			cfg: &parsers.Config{
-				MetricName:        "parser_test_new",
-				XPathProtobufFile: "testdata/addressbook.proto",
-				XPathProtobufType: "addressbook.AddressBook",
-			},
 			param: map[string]interface{}{
 				"ProtobufMessageDef":  "testdata/addressbook.proto",
 				"ProtobufMessageType": "addressbook.AddressBook",
@@ -584,10 +568,6 @@ func TestConfig_ParserInterfaceOldFormat(t *testing.T) {
 	expected := make([]telegraf.Parser, 0, len(formats))
 	for _, format := range formats {
 		formatCfg := &cfg
-		settings, hasOverride := override[format]
-		if hasOverride && settings.cfg != nil {
-			formatCfg = settings.cfg
-		}
 		formatCfg.DataFormat = format
 
 		logger := models.NewLogger("parsers", format, cfg.MetricName)

plugins/parsers/all/all.go

@@ -3,4 +3,5 @@ package all
 import (
 	//Blank imports for plugins to register themselves
 	_ "github.com/influxdata/telegraf/plugins/parsers/csv"
+	_ "github.com/influxdata/telegraf/plugins/parsers/xpath"
 )

plugins/parsers/registry.go

@@ -19,7 +19,6 @@ import (
 	"github.com/influxdata/telegraf/plugins/parsers/prometheusremotewrite"
 	"github.com/influxdata/telegraf/plugins/parsers/value"
 	"github.com/influxdata/telegraf/plugins/parsers/wavefront"
-	"github.com/influxdata/telegraf/plugins/parsers/xpath"
 )
 
 // Creator is the function to create a new parser
@@ -184,12 +183,12 @@ type Config struct {
 	ValueFieldName string `toml:"value_field_name"`
 
 	// XPath configuration
-	XPathPrintDocument       bool     `toml:"xpath_print_document"`
-	XPathProtobufFile        string   `toml:"xpath_protobuf_file"`
-	XPathProtobufType        string   `toml:"xpath_protobuf_type"`
-	XPathProtobufImportPaths []string `toml:"xpath_protobuf_import_paths"`
-	XPathAllowEmptySelection bool     `toml:"xpath_allow_empty_selection"`
-	XPathConfig              []XPathConfig
+	XPathPrintDocument       bool          `toml:"xpath_print_document"`
+	XPathProtobufFile        string        `toml:"xpath_protobuf_file"`
+	XPathProtobufType        string        `toml:"xpath_protobuf_type"`
+	XPathProtobufImportPaths []string      `toml:"xpath_protobuf_import_paths"`
+	XPathAllowEmptySelection bool          `toml:"xpath_allow_empty_selection"`
+	XPathConfig              []XPathConfig `toml:"xpath"`
 
 	// JSONPath configuration
 	JSONV2Config []JSONV2Config `toml:"json_v2"`
@@ -201,7 +200,29 @@ type Config struct {
 	LogFmtTagKeys []string `toml:"logfmt_tag_keys"`
 }
 
-type XPathConfig xpath.Config
+// XPathConfig definition for backward compatibitlity ONLY.
+// We need this here to avoid cyclic dependencies. However, we need
+// to move this to plugins/parsers/xpath once we deprecate parser
+// construction via `NewParser()`.
+type XPathConfig struct {
+	MetricQuery  string            `toml:"metric_name"`
+	Selection    string            `toml:"metric_selection"`
+	Timestamp    string            `toml:"timestamp"`
+	TimestampFmt string            `toml:"timestamp_format"`
+	Tags         map[string]string `toml:"tags"`
+	Fields       map[string]string `toml:"fields"`
+	FieldsInt    map[string]string `toml:"fields_int"`
+
+	FieldSelection  string `toml:"field_selection"`
+	FieldNameQuery  string `toml:"field_name"`
+	FieldValueQuery string `toml:"field_value"`
+	FieldNameExpand bool   `toml:"field_name_expansion"`
+
+	TagSelection  string `toml:"tag_selection"`
+	TagNameQuery  string `toml:"tag_name"`
+	TagValueQuery string `toml:"tag_value"`
+	TagNameExpand bool   `toml:"tag_name_expansion"`
+}
 
 type JSONV2Config struct {
 	json_v2.Config
@@ -280,17 +301,6 @@ func NewParser(config *Config) (Parser, error) {
 		)
 	case "prometheusremotewrite":
 		parser, err = NewPrometheusRemoteWriteParser(config.DefaultTags)
-	case "xml", "xpath_json", "xpath_msgpack", "xpath_protobuf":
-		parser = &xpath.Parser{
-			Format:              config.DataFormat,
-			ProtobufMessageDef:  config.XPathProtobufFile,
-			ProtobufMessageType: config.XPathProtobufType,
-			ProtobufImportPaths: config.XPathProtobufImportPaths,
-			PrintDocument:       config.XPathPrintDocument,
-			DefaultTags:         config.DefaultTags,
-			AllowEmptySelection: config.XPathAllowEmptySelection,
-			Configs:             NewXPathParserConfigs(config.MetricName, config.XPathConfig),
-		}
 	case "json_v2":
 		parser, err = NewJSONPathParser(config.JSONV2Config)
 	default:
@@ -429,17 +439,6 @@ func NewPrometheusRemoteWriteParser(defaultTags map[string]string) (Parser, erro
 	}, nil
 }
 
-func NewXPathParserConfigs(metricName string, cfgs []XPathConfig) []xpath.Config {
-	// Convert the config formats which is a one-to-one copy
-	configs := make([]xpath.Config, 0, len(cfgs))
-	for _, cfg := range cfgs {
-		config := xpath.Config(cfg)
-		config.MetricDefaultName = metricName
-		configs = append(configs, config)
-	}
-	return configs
-}
-
 func NewJSONPathParser(jsonv2config []JSONV2Config) (Parser, error) {
 	configs := make([]json_v2.Config, len(jsonv2config))
 	for i, cfg := range jsonv2config {

plugins/parsers/registry_test.go

@@ -2,8 +2,11 @@ package parsers_test
 
 import (
 	"reflect"
+	"sync"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/stretchr/testify/require"
 
 	"github.com/influxdata/telegraf"
@@ -15,6 +18,8 @@ func TestRegistry_BackwardCompatibility(t *testing.T) {
 	cfg := &parsers.Config{
 		MetricName:        "parser_compatibility_test",
 		CSVHeaderRowCount: 42,
+		XPathProtobufFile: "xpath/testcases/protos/addressbook.proto",
+		XPathProtobufType: "addressbook.AddressBook",
 	}
 
 	// Some parsers need certain settings to not error. Furthermore, we
@@ -29,6 +34,12 @@ func TestRegistry_BackwardCompatibility(t *testing.T) {
 			},
 			mask: []string{"TimeFunc"},
 		},
+		"xpath_protobuf": {
+			param: map[string]interface{}{
+				"ProtobufMessageDef":  cfg.XPathProtobufFile,
+				"ProtobufMessageType": cfg.XPathProtobufType,
+			},
+		},
 	}
 
 	for name, creator := range parsers.Parsers {
@@ -52,19 +63,23 @@ func TestRegistry_BackwardCompatibility(t *testing.T) {
 		actual, err := parsers.NewParser(cfg)
 		require.NoError(t, err)
 
-		// Compare with mask
-		if settings, found := override[name]; found {
-			a := reflect.Indirect(reflect.ValueOf(actual))
-			e := reflect.Indirect(reflect.ValueOf(expected))
-			for _, key := range settings.mask {
-				af := a.FieldByName(key)
-				ef := e.FieldByName(key)
+		// Determine the underlying type of the parser
+		stype := reflect.Indirect(reflect.ValueOf(expected)).Interface()
+		// Ignore all unexported fields and fields not relevant for functionality
+		options := []cmp.Option{
+			cmpopts.IgnoreUnexported(stype),
+			cmpopts.IgnoreTypes(sync.Mutex{}),
+			cmpopts.IgnoreInterfaces(struct{ telegraf.Logger }{}),
+		}
 
-				v := reflect.Zero(ef.Type())
-				af.Set(v)
-				ef.Set(v)
-			}
+		// Add overrides and masks to compare options
+		if settings, found := override[name]; found {
+			options = append(options, cmpopts.IgnoreFields(stype, settings.mask...))
 		}
-		require.EqualValuesf(t, expected, actual, "format %q", name)
+
+		// Do a manual comparision as require.EqualValues will also work on unexported fields
+		// that cannot be cleared or ignored.
+		diff := cmp.Diff(expected, actual, options...)
+		require.Emptyf(t, diff, "Difference for %q", name)
 	}
 }