From 0b0052496da3bd1dc93dc80852043be3a990b017 Mon Sep 17 00:00:00 2001 From: yug-elastic Date: Thu, 19 May 2022 18:49:12 +0530 Subject: [PATCH 1/4] Update host parsing mechanism --- x-pack/metricbeat/module/oracle/connection.go | 75 ++++++++++--------- .../module/oracle/performance/metricset.go | 18 ++--- .../module/oracle/tablespace/metricset.go | 10 +-- 3 files changed, 50 insertions(+), 53 deletions(-) diff --git a/x-pack/metricbeat/module/oracle/connection.go b/x-pack/metricbeat/module/oracle/connection.go index 8beb7c936ad6..40cb94e7b4ae 100644 --- a/x-pack/metricbeat/module/oracle/connection.go +++ b/x-pack/metricbeat/module/oracle/connection.go @@ -6,62 +6,54 @@ package oracle import ( "database/sql" + "fmt" "github.com/godror/godror" "github.com/elastic/beats/v7/metricbeat/mb" - "github.com/elastic/beats/v7/metricbeat/mb/parse" - - "github.com/pkg/errors" ) // ConnectionDetails contains all possible data that can be used to create a connection with // an Oracle db type ConnectionDetails struct { - Username string `config:"username"` - Password string `config:"password"` - Hosts []string `config:"hosts" validate:"required"` + Username string `config:"username"` + Password string `config:"password"` } -// HostParser parsers the host value as a URL -var HostParser = parse.URLHostParserBuilder{ - DefaultScheme: "oracle", -}.Build() - -func init() { - // Register the ModuleFactory function for the "oracle" module. - if err := mb.Registry.AddModule("oracle", newModule); err != nil { - panic(err) +// HostParser parses host value +func HostParser(mod mb.Module, rawURL string) (mb.HostData, error) { + params, err := godror.ParseConnString(rawURL) + if err != nil { + return mb.HostData{}, fmt.Errorf("error trying to parse connection string in field 'hosts': %w", err) } -} -// NewConnection returns a connection already established with Oracle -func NewConnection(c *ConnectionDetails) (*sql.DB, error) { - params, err := godror.ParseConnString(c.Hosts[0]) - if err != nil { - return nil, errors.Wrap(err, "error trying to parse connection string in field 'hosts'") + config := ConnectionDetails{} + if err := mod.UnpackConfig(&config); err != nil { + return mb.HostData{}, fmt.Errorf("error parsing config file: %w", err) } if params.Username == "" { - params.Username = c.Username + params.Username = config.Username } if params.Password == "" { - params.Password = c.Password + params.Password = config.Password } - db, err := sql.Open("godror", params.StringWithPassword()) - if err != nil { - return nil, errors.Wrap(err, "could not open database") - } + return mb.HostData{ + URI: params.StringWithPassword(), + SanitizedURI: params.SID, + Host: params.SID, + User: params.Username, + Password: params.Password, + }, nil +} - // Check the connection before executing all queries to reduce the number - // of connection errors that we might encounter. - if err = db.Ping(); err != nil { - err = errors.Wrap(err, "error doing ping to database") +func init() { + // Register the ModuleFactory function for the "oracle" module. + if err := mb.Registry.AddModule("oracle", newModule); err != nil { + panic(err) } - - return db, err } // newModule adds validation that hosts is non-empty, a requirement to use the @@ -70,8 +62,23 @@ func newModule(base mb.BaseModule) (mb.Module, error) { // Validate that at least one host has been specified. config := ConnectionDetails{} if err := base.UnpackConfig(&config); err != nil { - return nil, errors.Wrap(err, "error parsing config module") + return nil, fmt.Errorf("error parsing config module: %w", err) } return &base, nil } + +func NewConnection(connString string) (*sql.DB, error) { + db, err := sql.Open("godror", connString) + if err != nil { + return nil, fmt.Errorf("could not open database: %w", err) + } + + // Check the connection before executing all queries to reduce the number + // of connection errors that we might encounter. + if err = db.Ping(); err != nil { + err = fmt.Errorf("error doing ping to database: %w", err) + } + + return db, err +} diff --git a/x-pack/metricbeat/module/oracle/performance/metricset.go b/x-pack/metricbeat/module/oracle/performance/metricset.go index 2c79b5813c3d..f4e47ace75bf 100644 --- a/x-pack/metricbeat/module/oracle/performance/metricset.go +++ b/x-pack/metricbeat/module/oracle/performance/metricset.go @@ -6,8 +6,7 @@ package performance import ( "context" - - "github.com/pkg/errors" + "fmt" "github.com/elastic/beats/v7/metricbeat/mb" "github.com/elastic/beats/v7/x-pack/metricbeat/module/oracle" @@ -28,21 +27,14 @@ func init() { // interface methods except for Fetch. type MetricSet struct { mb.BaseMetricSet - extractor performanceExtractMethods - connectionDetails oracle.ConnectionDetails + extractor performanceExtractMethods } // New creates a new instance of the MetricSet. New is responsible for unpacking // any MetricSet specific configuration options if there are any. func New(base mb.BaseMetricSet) (mb.MetricSet, error) { - config := oracle.ConnectionDetails{} - if err := base.Module().UnpackConfig(&config); err != nil { - return nil, errors.Wrap(err, "error parsing config file") - } - return &MetricSet{ - BaseMetricSet: base, - connectionDetails: config, + BaseMetricSet: base, }, nil } @@ -50,9 +42,9 @@ func New(base mb.BaseMetricSet) (mb.MetricSet, error) { // format. It publishes the event which is then forwarded to the output. In case // of an error set the Error field of mb.Event or simply call report.Error(). func (m *MetricSet) Fetch(ctx context.Context, reporter mb.ReporterV2) error { - db, err := oracle.NewConnection(&m.connectionDetails) + db, err := oracle.NewConnection(m.HostData().URI) if err != nil { - return errors.Wrap(err, "error creating connection to Oracle") + return fmt.Errorf("error creating connection to Oracle: %w", err) } defer db.Close() diff --git a/x-pack/metricbeat/module/oracle/tablespace/metricset.go b/x-pack/metricbeat/module/oracle/tablespace/metricset.go index abc217b38440..9104d8301e75 100644 --- a/x-pack/metricbeat/module/oracle/tablespace/metricset.go +++ b/x-pack/metricbeat/module/oracle/tablespace/metricset.go @@ -28,14 +28,13 @@ func init() { // interface methods except for Fetch. type MetricSet struct { mb.BaseMetricSet - extractor tablespaceExtractMethods - connectionDetails oracle.ConnectionDetails + extractor tablespaceExtractMethods } // New creates a new instance of the MetricSet. New is responsible for unpacking // any MetricSet specific configuration options if there are any. func New(base mb.BaseMetricSet) (mb.MetricSet, error) { - config := oracle.ConnectionDetails{} + config := struct{}{} if err := base.Module().UnpackConfig(&config); err != nil { return nil, fmt.Errorf("error parsing config file: %w", err) } @@ -46,8 +45,7 @@ func New(base mb.BaseMetricSet) (mb.MetricSet, error) { } return &MetricSet{ - BaseMetricSet: base, - connectionDetails: config, + BaseMetricSet: base, }, nil } @@ -60,7 +58,7 @@ func CheckCollectionPeriod(period time.Duration) bool { // format. It publishes the event which is then forwarded to the output. In case // of an error set the Error field of mb.Event or simply call report.Error(). func (m *MetricSet) Fetch(ctx context.Context, reporter mb.ReporterV2) (err error) { - db, err := oracle.NewConnection(&m.connectionDetails) + db, err := oracle.NewConnection(m.HostData().URI) if err != nil { return fmt.Errorf("error creating connection to Oracle: %w", err) } From de74da4a217a0286aab1aa434727cdc059dba74f Mon Sep 17 00:00:00 2001 From: yug-elastic Date: Mon, 23 May 2022 14:50:53 +0530 Subject: [PATCH 2/4] Update comment to add more description --- x-pack/metricbeat/module/oracle/connection.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/metricbeat/module/oracle/connection.go b/x-pack/metricbeat/module/oracle/connection.go index 40cb94e7b4ae..a2b49679dcb7 100644 --- a/x-pack/metricbeat/module/oracle/connection.go +++ b/x-pack/metricbeat/module/oracle/connection.go @@ -20,7 +20,8 @@ type ConnectionDetails struct { Password string `config:"password"` } -// HostParser parses host value +// HostParser parses host and extracts connection information and returns it to HostData +// HostData can then be used to make connection to SQL func HostParser(mod mb.Module, rawURL string) (mb.HostData, error) { params, err := godror.ParseConnString(rawURL) if err != nil { From 4afc97b7ba0f7cc88a13269df990ddcb84a6e08a Mon Sep 17 00:00:00 2001 From: yug-elastic Date: Mon, 23 May 2022 15:38:21 +0530 Subject: [PATCH 3/4] Add CHANGELOG.next.asciidoc entry --- CHANGELOG.next.asciidoc | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index b29fb180c447..c160befab4f9 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -141,6 +141,7 @@ https://github.com/elastic/beats/compare/v8.2.0\...main[Check the HEAD diff] - Add metadata for missing k8s resources/metricsets {pull}31590[31590] - Fix `include_top_n` fields in system/process {pull}31595[31595] - Upgrade Mongodb library in Beats to v5 {pull}31185[31185] +- Enhance Oracle Module: Refactor module to properly use host parsers instead of doing its own parsing of hosts {issue}31611[31611] {pull}31692[#31692] *Packetbeat* From 709721a07cd3ad63ff153bfffddb710f9e7f108b Mon Sep 17 00:00:00 2001 From: yug-elastic Date: Wed, 1 Jun 2022 07:46:01 +0530 Subject: [PATCH 4/4] Update CHANGELOG.next.asciidoc to remove accidentally added PRs --- CHANGELOG.next.asciidoc | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 91b135ed31d0..c751035c1591 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -121,13 +121,9 @@ https://github.com/elastic/beats/compare/v8.2.0\...main[Check the HEAD diff] - Enhance Oracle Module: Change tablespace metricset collection period {issue}30948[30948] {pull}31259[#31259] - Add orchestrator cluster ECS fields in kubernetes events {pull}31341[31341] -- Generic SQL code reorganization, with support for raw metrics and query lists {pull}31568[31568] -- Add metadata for missing k8s resources/metricsets {pull}31590[31590] -- Fix `include_top_n` fields in system/process {pull}31595[31595] -- Upgrade Mongodb library in Beats to v5 {pull}31185[31185] -- Enhance Oracle Module: Refactor module to properly use host parsers instead of doing its own parsing of hosts {issue}31611[31611] {pull}31692[#31692] - Add new Kubernetes module dashboards {pull}31591[31591] - system/core: add cpuinfo information for Linux hosts {pull}31643[31643] +- Enhance Oracle Module: Refactor module to properly use host parsers instead of doing its own parsing of hosts {issue}31611[31611] {pull}31692[#31692] *Packetbeat*