From badb2cb0a29807f2a83fa8d7c4005b958e5ddeba Mon Sep 17 00:00:00 2001 From: Jaime Soriano Pastor Date: Wed, 22 Jan 2020 16:53:33 +0100 Subject: [PATCH] Fix mapping and race condition in sql module (#15738) Mapping was defined at the metricset level, but it should be defined at the module level. Add system test to earlier detect this kind of issues. Also add validation so driver and sql_query options cannot be empty. Fetch was reusing some maps when building the event, what was causing mixed data in events and panics on queries with lots of rows. Fixes #15736 --- metricbeat/docs/fields.asciidoc | 23 +++-------- x-pack/metricbeat/module/sql/_meta/fields.yml | 22 ++++++++-- x-pack/metricbeat/module/sql/fields.go | 2 +- .../module/sql/query/_meta/fields.yml | 25 +---------- x-pack/metricbeat/module/sql/query/query.go | 40 +++++++++--------- x-pack/metricbeat/module/sql/test_sql.py | 41 +++++++++++++++++++ 6 files changed, 86 insertions(+), 67 deletions(-) create mode 100644 x-pack/metricbeat/module/sql/test_sql.py diff --git a/metricbeat/docs/fields.asciidoc b/metricbeat/docs/fields.asciidoc index e86c0975b17..135188d9831 100644 --- a/metricbeat/docs/fields.asciidoc +++ b/metricbeat/docs/fields.asciidoc @@ -30466,25 +30466,12 @@ type: long [[exported-fields-sql]] == sql fields -sql module +SQL module fetches metrics from a SQL database -[float] -=== sql - -`sql` fetches metrics from a SQL database - - - -[float] -=== query - -query - - -*`sql.query.driver`*:: +*`sql.driver`*:: + -- Driver used to execute the query. @@ -30494,7 +30481,7 @@ type: keyword -- -*`sql.query.query`*:: +*`sql.query`*:: + -- Query executed to collect metrics. @@ -30504,7 +30491,7 @@ type: keyword -- -*`sql.query.metrics.numeric.*`*:: +*`sql.metrics.numeric.*`*:: + -- Numeric metrics collected. @@ -30514,7 +30501,7 @@ type: object -- -*`sql.query.metrics.string.*`*:: +*`sql.metrics.string.*`*:: + -- Non-numeric values collected. diff --git a/x-pack/metricbeat/module/sql/_meta/fields.yml b/x-pack/metricbeat/module/sql/_meta/fields.yml index b27abd8896a..81a3dfb3df5 100644 --- a/x-pack/metricbeat/module/sql/_meta/fields.yml +++ b/x-pack/metricbeat/module/sql/_meta/fields.yml @@ -2,10 +2,26 @@ title: "sql" release: beta description: > - sql module + SQL module fetches metrics from a SQL database fields: - name: sql type: group - description: > - `sql` fetches metrics from a SQL database fields: + - name: driver + type: keyword + description: > + Driver used to execute the query. + - name: query + type: keyword + description: > + Query executed to collect metrics. + - name: metrics.numeric.* + type: object + object_type: double + description: > + Numeric metrics collected. + - name: metrics.string.* + type: object + object_type: keyword + description: > + Non-numeric values collected. diff --git a/x-pack/metricbeat/module/sql/fields.go b/x-pack/metricbeat/module/sql/fields.go index 87127a1c2f2..cefc5b3c29b 100644 --- a/x-pack/metricbeat/module/sql/fields.go +++ b/x-pack/metricbeat/module/sql/fields.go @@ -19,5 +19,5 @@ func init() { // AssetSql returns asset data. // This is the base64 encoded gzipped contents of module/sql. func AssetSql() string { - return "eJykkk2O4jAUhPc5RYnlSHCALGY1yxESmgMMjl2AGycm9jPduX3L+UEQpZGaruWruOqL/dY4sysRW1cAYsWxxCq2blUAgY4qskRFUQVgGHWwF7G+KfG7AJDPofYmORbAwdKZWPbGGo2qOQVnSXdhiWPw6TJOFvKy9rF1exwo+sSImhKsjjgEX0Ph3+4vjBJVqUiMR+5777vbxNDdpksEWfO/nPQFXdY8eA5wD2GCvTI8WBPJmd27D2bmPenN+tPnIUUaiAc/qJMQcuKAtVmkmBP/EGKX46bunkN756hleq5lislsUs1g9ebXIpGv3qhlZg3D/8MXxqeq37hvIG+Hzts+jcA0z1GjBNscXyZ96Xa3vlmPV4SrcokPtJ8BAAD//zTX/E8=" + return "eJykkkFuwjAQRfc5xRfLSnAAL7rqskJCPUDl2B9wcWJij2lz+4okRqlSpKJ6+T0z72XiNU7sFVLnK0CceCqsUudXFRDpqRMVaoquAMtkojuLC63CcwUAb7tXNMFmT+wp5siEhhKdSdjH0EAPFVaLrnViBewdvU1qaF6j1Q0L/HqkP1PhEEM+T8m8ft5jo7sw3uLSemL/GaKd5b9Il/MyzEBOtJAAftFkIeRIdJmx3yyoQ/w/6O46orAGrgne00hZ3JJaLtrcMDqzeVoYhPqDRmbxGLyPtzbk2vNvetuRcfuLkxztfa0k0bWHh60e2to2tOvp83HRPvOO2c8n+x0AAP//MiXQYA==" } diff --git a/x-pack/metricbeat/module/sql/query/_meta/fields.yml b/x-pack/metricbeat/module/sql/query/_meta/fields.yml index b6e7eb58132..8033a27f5ac 100644 --- a/x-pack/metricbeat/module/sql/query/_meta/fields.yml +++ b/x-pack/metricbeat/module/sql/query/_meta/fields.yml @@ -1,24 +1 @@ -- name: query - type: group - release: beta - description: > - query - fields: - - name: driver - type: keyword - description: > - Driver used to execute the query. - - name: query - type: keyword - description: > - Query executed to collect metrics. - - name: metrics.numeric.* - type: object - object_type: double - description: > - Numeric metrics collected. - - name: metrics.string.* - type: object - object_type: keyword - description: > - Non-numeric values collected. +- release: beta diff --git a/x-pack/metricbeat/module/sql/query/query.go b/x-pack/metricbeat/module/sql/query/query.go index 3322144d993..c3507cd964e 100644 --- a/x-pack/metricbeat/module/sql/query/query.go +++ b/x-pack/metricbeat/module/sql/query/query.go @@ -44,8 +44,8 @@ func New(base mb.BaseMetricSet) (mb.MetricSet, error) { cfgwarn.Beta("The sql query metricset is beta.") config := struct { - Driver string `config:"driver"` - Query string `config:"sql_query"` + Driver string `config:"driver" validate:"nonzero,required"` + Query string `config:"sql_query" validate:"nonzero,required"` }{} if err := base.Module().UnpackConfig(&config); err != nil { @@ -96,10 +96,6 @@ func (m *MetricSet) Fetch(report mb.ReporterV2) error { vals[i] = new(interface{}) } - numericValue := common.MapStr{} - - stringValue := common.MapStr{} - for rows.Next() { err = rows.Scan(vals...) if err != nil { @@ -107,30 +103,32 @@ func (m *MetricSet) Fetch(report mb.ReporterV2) error { continue } - data := common.MapStr{ - "metrics": common.MapStr{}, - } - data.Put("driver", m.Driver) - data.Put("query", m.Query) + numericMetrics := common.MapStr{} + stringMetrics := common.MapStr{} for i := 0; i < len(vals); i++ { - dataMetrics := data["metrics"].(common.MapStr) - value := getValue(vals[i].(*interface{})) - num, err := strconv.ParseFloat(value, 64) if err == nil { - numericValue.Put(cols[i], num) - dataMetrics.Put("numeric", numericValue) + numericMetrics[cols[i]] = num } else { - stringValue.Put(cols[i], value) - dataMetrics.Put("string", stringValue) + stringMetrics[cols[i]] = value } - report.Event(mb.Event{ - RootFields: common.MapStr{"sql": data}, - }) } + + report.Event(mb.Event{ + RootFields: common.MapStr{ + "sql": common.MapStr{ + "driver": m.Driver, + "query": m.Query, + "metrics": common.MapStr{ + "numeric": numericMetrics, + "string": stringMetrics, + }, + }, + }, + }) } if rows.Err() != nil { diff --git a/x-pack/metricbeat/module/sql/test_sql.py b/x-pack/metricbeat/module/sql/test_sql.py new file mode 100644 index 00000000000..8a18791b386 --- /dev/null +++ b/x-pack/metricbeat/module/sql/test_sql.py @@ -0,0 +1,41 @@ +import os +import sys +import unittest + +sys.path.append(os.path.join(os.path.dirname(__file__), '../../tests/system')) +from xpack_metricbeat import XPackTest, metricbeat + + +class Test(XPackTest): + + COMPOSE_SERVICES = ['mysql'] + + @unittest.skipUnless(metricbeat.INTEGRATION_TESTS, "integration test") + def test_query(self): + """ + sql custom query test + """ + self.render_config_template(modules=[{ + "name": "sql", + "metricsets": ["query"], + "hosts": self.get_hosts(), + "period": "5s", + "additional_content": """ + driver: mysql + sql_query: 'select table_schema, table_name, engine, table_rows from information_schema.tables where table_rows > 0'""" + }]) + proc = self.start_beat(home=self.beat_path) + self.wait_until(lambda: self.output_lines() > 0) + proc.check_kill_and_wait() + self.assert_no_logged_warnings() + + output = self.read_output_json() + self.assertGreater(len(output), 0) + + for evt in output: + self.assert_fields_are_documented(evt) + self.assertIn("sql", evt.keys(), evt) + self.assertIn("query", evt["sql"].keys(), evt) + + def get_hosts(self): + return ['root:test@tcp({})/'.format(self.compose_host())]