[Oracle Module] Refactor module to properly use host parsers instead of doing its own parsing of hosts

CHANGELOG.next.asciidoc

@@ -121,6 +121,11 @@ 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]
 

x-pack/metricbeat/module/oracle/connection.go

@@ -6,62 +6,55 @@ 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 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 {
+		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 +63,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
+}

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,31 +27,24 @@ 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
 }
 
 // Fetch methods implements the data gathering and data conversion to the right
 // 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()
 

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)
 	}