From d035e3f5a5228c2037c2c4f31d024ffb42ef978a Mon Sep 17 00:00:00 2001 From: Sven Rebhan <36194019+srebhan@users.noreply.github.com> Date: Wed, 15 Sep 2021 19:58:40 +0200 Subject: [PATCH] fix: Fix panic for non-existing metric names (#9757) (cherry picked from commit c076398440971c01f67eb326c434c1eab1c361b2) --- plugins/parsers/registry.go | 2 +- plugins/parsers/xpath/parser.go | 26 ++-- plugins/parsers/xpath/parser_test.go | 189 ++++++++++++++++++--------- 3 files changed, 141 insertions(+), 76 deletions(-) diff --git a/plugins/parsers/registry.go b/plugins/parsers/registry.go index cc2102c9532d2..f07c789a272f1 100644 --- a/plugins/parsers/registry.go +++ b/plugins/parsers/registry.go @@ -395,7 +395,7 @@ func NewXPathParserConfigs(metricName string, cfgs []XPathConfig) []xpath.Config configs := make([]xpath.Config, 0, len(cfgs)) for _, cfg := range cfgs { config := xpath.Config(cfg) - config.MetricName = metricName + config.MetricDefaultName = metricName configs = append(configs, config) } return configs diff --git a/plugins/parsers/xpath/parser.go b/plugins/parsers/xpath/parser.go index 52224530a9250..75ebfd92035c1 100644 --- a/plugins/parsers/xpath/parser.go +++ b/plugins/parsers/xpath/parser.go @@ -35,14 +35,14 @@ type Parser struct { } type Config struct { - MetricName string - 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"` + MetricDefaultName string `toml:"-"` + 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"` @@ -160,13 +160,19 @@ func (p *Parser) parseQuery(starttime time.Time, doc, selected dataNode, config // Determine the metric name. If a query was specified, use the result of this query and the default metric name // otherwise. - metricname = config.MetricName + metricname = config.MetricDefaultName if len(config.MetricQuery) > 0 { v, err := p.executeQuery(doc, selected, config.MetricQuery) if err != nil { return nil, fmt.Errorf("failed to query metric name: %v", err) } - metricname = v.(string) + var ok bool + if metricname, ok = v.(string); !ok { + if v == nil { + p.Log.Infof("Hint: Empty metric-name-node. If you wanted to set a constant please use `metric_name = \"'name'\"`.") + } + return nil, fmt.Errorf("failed to query metric name: query result is of type %T not 'string'", v) + } } // By default take the time the parser was invoked and override the value diff --git a/plugins/parsers/xpath/parser_test.go b/plugins/parsers/xpath/parser_test.go index 46e4dba690102..8e7a3087c0888 100644 --- a/plugins/parsers/xpath/parser_test.go +++ b/plugins/parsers/xpath/parser_test.go @@ -148,8 +148,8 @@ func TestInvalidTypeQueriesFail(t *testing.T) { input: singleMetricValuesXML, configs: []Config{ { - MetricName: "test", - Timestamp: "/Device_1/Timestamp_unix", + MetricDefaultName: "test", + Timestamp: "/Device_1/Timestamp_unix", FieldsInt: map[string]string{ "a": "/Device_1/value_string", }, @@ -185,8 +185,8 @@ func TestInvalidTypeQueries(t *testing.T) { input: singleMetricValuesXML, configs: []Config{ { - MetricName: "test", - Timestamp: "/Device_1/Timestamp_unix", + MetricDefaultName: "test", + Timestamp: "/Device_1/Timestamp_unix", Fields: map[string]string{ "a": "number(/Device_1/value_string)", }, @@ -207,8 +207,8 @@ func TestInvalidTypeQueries(t *testing.T) { input: singleMetricValuesXML, configs: []Config{ { - MetricName: "test", - Timestamp: "/Device_1/Timestamp_unix", + MetricDefaultName: "test", + Timestamp: "/Device_1/Timestamp_unix", Fields: map[string]string{ "a": "boolean(/Device_1/value_string)", }, @@ -252,8 +252,8 @@ func TestParseTimestamps(t *testing.T) { input: singleMetricValuesXML, configs: []Config{ { - MetricName: "test", - Timestamp: "/Device_1/Timestamp_unix", + MetricDefaultName: "test", + Timestamp: "/Device_1/Timestamp_unix", }, }, defaultTags: map[string]string{}, @@ -269,9 +269,9 @@ func TestParseTimestamps(t *testing.T) { input: singleMetricValuesXML, configs: []Config{ { - MetricName: "test", - Timestamp: "/Device_1/Timestamp_unix", - TimestampFmt: "unix", + MetricDefaultName: "test", + Timestamp: "/Device_1/Timestamp_unix", + TimestampFmt: "unix", }, }, defaultTags: map[string]string{}, @@ -287,9 +287,9 @@ func TestParseTimestamps(t *testing.T) { input: singleMetricValuesXML, configs: []Config{ { - MetricName: "test", - Timestamp: "/Device_1/Timestamp_unix_ms", - TimestampFmt: "unix_ms", + MetricDefaultName: "test", + Timestamp: "/Device_1/Timestamp_unix_ms", + TimestampFmt: "unix_ms", }, }, defaultTags: map[string]string{}, @@ -305,9 +305,9 @@ func TestParseTimestamps(t *testing.T) { input: singleMetricValuesXML, configs: []Config{ { - MetricName: "test", - Timestamp: "/Device_1/Timestamp_unix_us", - TimestampFmt: "unix_us", + MetricDefaultName: "test", + Timestamp: "/Device_1/Timestamp_unix_us", + TimestampFmt: "unix_us", }, }, defaultTags: map[string]string{}, @@ -323,9 +323,9 @@ func TestParseTimestamps(t *testing.T) { input: singleMetricValuesXML, configs: []Config{ { - MetricName: "test", - Timestamp: "/Device_1/Timestamp_unix_ns", - TimestampFmt: "unix_ns", + MetricDefaultName: "test", + Timestamp: "/Device_1/Timestamp_unix_ns", + TimestampFmt: "unix_ns", }, }, defaultTags: map[string]string{}, @@ -341,9 +341,9 @@ func TestParseTimestamps(t *testing.T) { input: singleMetricValuesXML, configs: []Config{ { - MetricName: "test", - Timestamp: "/Device_1/Timestamp_iso", - TimestampFmt: "2006-01-02T15:04:05Z", + MetricDefaultName: "test", + Timestamp: "/Device_1/Timestamp_iso", + TimestampFmt: "2006-01-02T15:04:05Z", }, }, defaultTags: map[string]string{}, @@ -382,8 +382,8 @@ func TestParseSingleValues(t *testing.T) { input: singleMetricValuesXML, configs: []Config{ { - MetricName: "test", - Timestamp: "/Device_1/Timestamp_unix", + MetricDefaultName: "test", + Timestamp: "/Device_1/Timestamp_unix", Fields: map[string]string{ "a": "/Device_1/value_int", "b": "/Device_1/value_float", @@ -410,8 +410,8 @@ func TestParseSingleValues(t *testing.T) { input: singleMetricValuesXML, configs: []Config{ { - MetricName: "test", - Timestamp: "/Device_1/Timestamp_unix", + MetricDefaultName: "test", + Timestamp: "/Device_1/Timestamp_unix", Fields: map[string]string{ "a": "number(Device_1/value_int)", "b": "number(/Device_1/value_float)", @@ -438,8 +438,8 @@ func TestParseSingleValues(t *testing.T) { input: singleMetricValuesXML, configs: []Config{ { - MetricName: "test", - Timestamp: "/Device_1/Timestamp_unix", + MetricDefaultName: "test", + Timestamp: "/Device_1/Timestamp_unix", Fields: map[string]string{ "b": "number(/Device_1/value_float)", "c": "boolean(/Device_1/value_bool)", @@ -468,8 +468,8 @@ func TestParseSingleValues(t *testing.T) { input: singleMetricValuesXML, configs: []Config{ { - MetricName: "test", - Timestamp: "/Device_1/Timestamp_unix", + MetricDefaultName: "test", + Timestamp: "/Device_1/Timestamp_unix", Fields: map[string]string{ "x": "substring-before(/Device_1/value_position, ';')", "y": "substring-after(/Device_1/value_position, ';')", @@ -492,8 +492,8 @@ func TestParseSingleValues(t *testing.T) { input: singleMetricValuesXML, configs: []Config{ { - MetricName: "test", - Timestamp: "/Device_1/Timestamp_unix", + MetricDefaultName: "test", + Timestamp: "/Device_1/Timestamp_unix", Fields: map[string]string{ "x": "number(substring-before(/Device_1/value_position, ';'))", "y": "number(substring-after(/Device_1/value_position, ';'))", @@ -516,8 +516,8 @@ func TestParseSingleValues(t *testing.T) { input: singleMetricValuesXML, configs: []Config{ { - MetricName: "test", - Timestamp: "/Device_1/Timestamp_unix", + MetricDefaultName: "test", + Timestamp: "/Device_1/Timestamp_unix", FieldsInt: map[string]string{ "x": "substring-before(/Device_1/value_position, ';')", "y": "substring-after(/Device_1/value_position, ';')", @@ -540,8 +540,8 @@ func TestParseSingleValues(t *testing.T) { input: singleMetricValuesXML, configs: []Config{ { - MetricName: "test", - Timestamp: "/Device_1/Timestamp_unix", + MetricDefaultName: "test", + Timestamp: "/Device_1/Timestamp_unix", Tags: map[string]string{ "state": "/Device_1/State", "name": "substring-after(/Device_1/Name, ' ')", @@ -587,8 +587,8 @@ func TestParseSingleAttributes(t *testing.T) { input: singleMetricAttributesXML, configs: []Config{ { - MetricName: "test", - Timestamp: "/Device_1/Timestamp_unix/@value", + MetricDefaultName: "test", + Timestamp: "/Device_1/Timestamp_unix/@value", }, }, defaultTags: map[string]string{}, @@ -604,9 +604,9 @@ func TestParseSingleAttributes(t *testing.T) { input: singleMetricAttributesXML, configs: []Config{ { - MetricName: "test", - Timestamp: "/Device_1/Timestamp_iso/@value", - TimestampFmt: "2006-01-02T15:04:05Z", + MetricDefaultName: "test", + Timestamp: "/Device_1/Timestamp_iso/@value", + TimestampFmt: "2006-01-02T15:04:05Z", }, }, defaultTags: map[string]string{}, @@ -622,8 +622,8 @@ func TestParseSingleAttributes(t *testing.T) { input: singleMetricAttributesXML, configs: []Config{ { - MetricName: "test", - Timestamp: "/Device_1/Timestamp_unix/@value", + MetricDefaultName: "test", + Timestamp: "/Device_1/Timestamp_unix/@value", Fields: map[string]string{ "a": "/Device_1/attr_int/@_", "b": "/Device_1/attr_float/@_", @@ -650,8 +650,8 @@ func TestParseSingleAttributes(t *testing.T) { input: singleMetricAttributesXML, configs: []Config{ { - MetricName: "test", - Timestamp: "/Device_1/Timestamp_unix/@value", + MetricDefaultName: "test", + Timestamp: "/Device_1/Timestamp_unix/@value", Fields: map[string]string{ "a": "number(/Device_1/attr_int/@_)", "b": "number(/Device_1/attr_float/@_)", @@ -678,8 +678,8 @@ func TestParseSingleAttributes(t *testing.T) { input: singleMetricAttributesXML, configs: []Config{ { - MetricName: "test", - Timestamp: "/Device_1/Timestamp_unix/@value", + MetricDefaultName: "test", + Timestamp: "/Device_1/Timestamp_unix/@value", Fields: map[string]string{ "b": "number(/Device_1/attr_float/@_)", "c": "boolean(/Device_1/attr_bool/@_)", @@ -708,8 +708,8 @@ func TestParseSingleAttributes(t *testing.T) { input: singleMetricAttributesXML, configs: []Config{ { - MetricName: "test", - Timestamp: "/Device_1/Timestamp_unix/@value", + MetricDefaultName: "test", + Timestamp: "/Device_1/Timestamp_unix/@value", Fields: map[string]string{ "name": "substring-after(/Device_1/Name/@value, ' ')", }, @@ -730,8 +730,8 @@ func TestParseSingleAttributes(t *testing.T) { input: singleMetricAttributesXML, configs: []Config{ { - MetricName: "test", - Timestamp: "/Device_1/Timestamp_unix/@value", + MetricDefaultName: "test", + Timestamp: "/Device_1/Timestamp_unix/@value", Tags: map[string]string{ "state": "/Device_1/State/@_", "name": "substring-after(/Device_1/Name/@value, ' ')", @@ -754,8 +754,8 @@ func TestParseSingleAttributes(t *testing.T) { input: singleMetricAttributesXML, configs: []Config{ { - MetricName: "test", - Timestamp: "/Device_1/Timestamp_unix/@value", + MetricDefaultName: "test", + Timestamp: "/Device_1/Timestamp_unix/@value", Fields: map[string]string{ "a": "/Device_1/attr_bool_numeric/@_ = 1", }, @@ -799,8 +799,8 @@ func TestParseMultiValues(t *testing.T) { input: singleMetricMultiValuesXML, configs: []Config{ { - MetricName: "test", - Timestamp: "/Timestamp/@value", + MetricDefaultName: "test", + Timestamp: "/Timestamp/@value", Fields: map[string]string{ "a": "number(/Device/Value[1])", "b": "number(/Device/Value[2])", @@ -831,8 +831,8 @@ func TestParseMultiValues(t *testing.T) { input: singleMetricMultiValuesXML, configs: []Config{ { - MetricName: "test", - Timestamp: "/Timestamp/@value", + MetricDefaultName: "test", + Timestamp: "/Timestamp/@value", FieldsInt: map[string]string{ "a": "/Device/Value[1]", "b": "/Device/Value[2]", @@ -886,9 +886,9 @@ func TestParseMultiNodes(t *testing.T) { input: multipleNodesXML, configs: []Config{ { - MetricName: "test", - Selection: "/Device", - Timestamp: "/Timestamp/@value", + MetricDefaultName: "test", + Selection: "/Device", + Timestamp: "/Timestamp/@value", Fields: map[string]string{ "value": "number(Value)", "active": "Active = 1", @@ -999,9 +999,9 @@ func TestParseMetricQuery(t *testing.T) { input: metricNameQueryXML, configs: []Config{ { - MetricName: "test", - MetricQuery: "name(/Device_1/Metric/@*[1])", - Timestamp: "/Device_1/Timestamp_unix", + MetricDefaultName: "test", + MetricQuery: "name(/Device_1/Metric/@*[1])", + Timestamp: "/Device_1/Timestamp_unix", Fields: map[string]string{ "value": "/Device_1/Metric/@*[1]", }, @@ -1017,6 +1017,29 @@ func TestParseMetricQuery(t *testing.T) { time.Unix(1577923199, 0), ), }, + { + name: "parse metric name constant", + input: metricNameQueryXML, + configs: []Config{ + { + MetricDefaultName: "test", + MetricQuery: "'the_metric'", + Timestamp: "/Device_1/Timestamp_unix", + Fields: map[string]string{ + "value": "/Device_1/Metric/@*[1]", + }, + }, + }, + defaultTags: map[string]string{}, + expected: testutil.MustMetric( + "the_metric", + map[string]string{}, + map[string]interface{}{ + "value": "ok", + }, + time.Unix(1577923199, 0), + ), + }, } for _, tt := range tests { @@ -1032,6 +1055,42 @@ func TestParseMetricQuery(t *testing.T) { } } +func TestParseErrors(t *testing.T) { + var tests = []struct { + name string + input string + configs []Config + expected string + }{ + { + name: "string metric name query", + input: metricNameQueryXML, + configs: []Config{ + { + MetricDefaultName: "test", + MetricQuery: "arbitrary", + Timestamp: "/Device_1/Timestamp_unix", + Fields: map[string]string{ + "value": "/Device_1/Metric/@*[1]", + }, + }, + }, + expected: "failed to query metric name: query result is of type not 'string'", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + parser := &Parser{Configs: tt.configs, DefaultTags: map[string]string{}, Log: testutil.Logger{Name: "parsers.xml"}} + require.NoError(t, parser.Init()) + + _, err := parser.ParseLine(tt.input) + require.Error(t, err) + require.Equal(t, tt.expected, err.Error()) + }) + } +} + func TestEmptySelection(t *testing.T) { var tests = []struct { name string @@ -1146,7 +1205,7 @@ func TestTestCases(t *testing.T) { filename := filepath.FromSlash(tt.filename) cfg, header, err := loadTestConfiguration(filename) require.NoError(t, err) - cfg.MetricName = "xml" + cfg.MetricDefaultName = "xml" // Load the xml-content input, err := testutil.ParseRawLinesFrom(header, "File:")