Fix memory leak in SQL helper when database is not available (#26607) (…

…#26656)

Explicitly close SQL connections when connectivity checks fail during their
creation, to avoid leaking resources when a connection is created but it
is not going to be used.

(cherry picked from commit e4d4f76)

Co-authored-by: Jaime Soriano Pastor <jaime.soriano@elastic.co>

CHANGELOG.next.asciidoc

@@ -385,6 +385,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
 - Fix azure billing dashboard. {pull}25554[25554]
 - Major refactor of system/cpu and system/core metrics. {pull}25771[25771]
 - Fix GCP Project ID being ingested as `cloud.account.id` in `gcp.billing` module {issue}26357[26357] {pull}26412[26412]
+- Fix memory leak in SQL module when database is not available. {issue}25840[25840] {pull}26607[26607]
 - Fix aws metric tags with resourcegroupstaggingapi paginator.  {issue}26385[26385] {pull}26443[26443]
 
 *Packetbeat*

metricbeat/helper/sql/sql.go

@@ -52,6 +52,9 @@ func NewDBClient(driver, uri string, l *logp.Logger) (*DbClient, error) {
 	}
 	err = dbx.Ping()
 	if err != nil {
+		if closeErr := dbx.Close(); closeErr != nil {
+			return nil, errors.Wrapf(err, "failed to close with %s, after connection test failed", closeErr)
+		}
 		return nil, errors.Wrap(err, "testing connection")
 	}
 

metricbeat/helper/sql/sql_test.go

@@ -18,11 +18,18 @@
 package sql
 
 import (
+	"context"
+	"database/sql"
+	"database/sql/driver"
+	"fmt"
 	"math"
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/require"
+
 	"github.com/elastic/beats/v7/libbeat/common"
+	"github.com/elastic/beats/v7/libbeat/tests/resources"
 )
 
 type kv struct {
@@ -186,3 +193,55 @@ func TestToDotKeys(t *testing.T) {
 		t.Fail()
 	}
 }
+
+func TestNewDBClient(t *testing.T) {
+	t.Run("create and close", func(t *testing.T) {
+		goroutines := resources.NewGoroutinesChecker()
+		defer goroutines.Check(t)
+
+		client, err := NewDBClient("dummy", "localhost", nil)
+		require.NoError(t, err)
+
+		err = client.Close()
+		require.NoError(t, err)
+	})
+
+	t.Run("unavailable", func(t *testing.T) {
+		goroutines := resources.NewGoroutinesChecker()
+		defer goroutines.Check(t)
+
+		_, err := NewDBClient("dummy", "unavailable", nil)
+		require.Error(t, err)
+	})
+}
+
+func init() {
+	sql.Register("dummy", dummyDriver{})
+}
+
+type dummyDriver struct{}
+
+func (dummyDriver) Open(name string) (driver.Conn, error) {
+	if name == "error" {
+		return nil, fmt.Errorf("error")
+	}
+
+	return &dummyConnection{name: name}, nil
+}
+
+type dummyConnection struct {
+	name string
+}
+
+// Ensure that this dummy connection implements the pinger interface, used by the helper.
+var _ driver.Pinger = &dummyConnection{}
+
+func (*dummyConnection) Prepare(query string) (driver.Stmt, error) { return nil, nil }
+func (*dummyConnection) Close() error                              { return nil }
+func (*dummyConnection) Begin() (driver.Tx, error)                 { return nil, nil }
+func (c *dummyConnection) Ping(context.Context) error {
+	if c.name == "unavailable" {
+		return fmt.Errorf("database unavailable")
+	}
+	return nil
+}