fix: improved postgres version parsing for explain plans in database_observability component (#5131)

* Move semver parsing of postgres version into explain plan collector to facilitate better testing

* More guardrails for parsing the postgres versions with semver to support more possible/probable version strings

* More useful comments

* Add percona version string test

Author: rgeyer

internal/component/database_observability/postgres/collector/explain_plan.go

@@ -50,6 +50,7 @@ var unrecoverablePostgresSQLErrors = []string{
 }
 
 var paramCountRegex = regexp.MustCompile(`\$\d+`)
+var versSanitizeRegex = regexp.MustCompile(`^v?[0-9]+\.?[0-9]+`)
 
 type PgSQLExplainplan struct {
 	Plan PlanNode `json:"Plan"`
@@ -214,7 +215,7 @@ type ExplainPlanArguments struct {
 	ExcludeSchemas  []string
 	EntryHandler    loki.EntryHandler
 	InitialLookback time.Time
-	DBVersion       semver.Version
+	DBVersion       string
 
 	Logger log.Logger
 }
@@ -239,10 +240,9 @@ type ExplainPlan struct {
 }
 
 func NewExplainPlan(args ExplainPlanArguments) (*ExplainPlan, error) {
-	return &ExplainPlan{
+	ep := &ExplainPlan{
 		dbConnection:        args.DB,
 		dbDSN:               args.DSN,
-		dbVersion:           args.DBVersion,
 		dbConnectionFactory: defaultDbConnectionFactory,
 		scrapeInterval:      args.ScrapeInterval,
 		queryCache:          make(map[string]*queryInfo),
@@ -253,7 +253,16 @@ func NewExplainPlan(args ExplainPlanArguments) (*ExplainPlan, error) {
 		entryHandler:        args.EntryHandler,
 		logger:              log.With(args.Logger, "collector", ExplainPlanCollector),
 		running:             atomic.NewBool(false),
-	}, nil
+	}
+	// Pre-sanitize the version by removing any trailing characters before semver gets it
+	foundVers := versSanitizeRegex.FindString(args.DBVersion)
+	engineSemver, err := semver.ParseTolerant(foundVers)
+	if err != nil {
+		return ep, fmt.Errorf("failed to parse database engine version: %s: %w", args.DBVersion, err)
+	}
+	ep.dbVersion = engineSemver
+
+	return ep, nil
 }
 
 func (c *ExplainPlan) sendExplainPlansOutput(schemaName string, digest string, generatedAt string, result database_observability.ExplainProcessingResult, reason string, plan *database_observability.ExplainPlanNode) error {

internal/component/database_observability/postgres/collector/explain_plan_test.go

@@ -2255,37 +2255,66 @@ func TestNewExplainPlan(t *testing.T) {
 	entryHandler := loki.NewCollectingHandler()
 	defer entryHandler.Stop()
 
-	pre17ver, err := semver.ParseTolerant("14.1")
-	require.NoError(t, err)
+	t.Run("pre17 version", func(t *testing.T) {
+		pre17ver := "14.1"
+		pre17semver, err := semver.ParseTolerant(pre17ver)
+		require.NoError(t, err)
 
-	args := ExplainPlanArguments{
-		DB:              db,
-		DSN:             "postgres://user:pass@localhost:5432/testdb",
-		ScrapeInterval:  time.Minute,
-		PerScrapeRatio:  0.1,
-		ExcludeSchemas:  []string{"information_schema", "pg_catalog"},
-		EntryHandler:    entryHandler,
-		InitialLookback: time.Now().Add(-time.Hour),
-		DBVersion:       pre17ver,
-		Logger:          logger,
-	}
+		args := ExplainPlanArguments{
+			DB:              db,
+			DSN:             "postgres://user:pass@localhost:5432/testdb",
+			ScrapeInterval:  time.Minute,
+			PerScrapeRatio:  0.1,
+			ExcludeSchemas:  []string{"information_schema", "pg_catalog"},
+			EntryHandler:    entryHandler,
+			InitialLookback: time.Now().Add(-time.Hour),
+			DBVersion:       pre17ver,
+			Logger:          logger,
+		}
 
-	explainPlan, err := NewExplainPlan(args)
+		explainPlan, err := NewExplainPlan(args)
 
-	require.NoError(t, err)
-	require.NotNil(t, explainPlan)
-	assert.Equal(t, db, explainPlan.dbConnection)
-	assert.Equal(t, args.DSN, explainPlan.dbDSN)
-	assert.Equal(t, args.DBVersion, explainPlan.dbVersion)
-	assert.Equal(t, args.ScrapeInterval, explainPlan.scrapeInterval)
-	assert.Equal(t, args.PerScrapeRatio, explainPlan.perScrapeRatio)
-	assert.Equal(t, args.ExcludeSchemas, explainPlan.excludeSchemas)
-	assert.Equal(t, entryHandler, explainPlan.entryHandler)
-	assert.NotNil(t, explainPlan.queryCache)
-	assert.NotNil(t, explainPlan.queryDenylist)
-	assert.NotNil(t, explainPlan.finishedQueryCache)
-	assert.NotNil(t, explainPlan.running)
-	assert.False(t, explainPlan.running.Load())
+		require.NoError(t, err)
+		require.NotNil(t, explainPlan)
+		assert.Equal(t, db, explainPlan.dbConnection)
+		assert.Equal(t, args.DSN, explainPlan.dbDSN)
+		assert.Equal(t, pre17semver, explainPlan.dbVersion)
+		assert.Equal(t, args.ScrapeInterval, explainPlan.scrapeInterval)
+		assert.Equal(t, args.PerScrapeRatio, explainPlan.perScrapeRatio)
+		assert.Equal(t, args.ExcludeSchemas, explainPlan.excludeSchemas)
+		assert.Equal(t, entryHandler, explainPlan.entryHandler)
+		assert.NotNil(t, explainPlan.queryCache)
+		assert.NotNil(t, explainPlan.queryDenylist)
+		assert.NotNil(t, explainPlan.finishedQueryCache)
+		assert.NotNil(t, explainPlan.running)
+		assert.False(t, explainPlan.running.Load())
+	})
+
+	t.Run("version with trailing characters from docker image", func(t *testing.T) {
+		args := ExplainPlanArguments{
+			DBVersion: "16.10 (Debian 16.10-1.pgdg13+1)",
+		}
+
+		ep, err := NewExplainPlan(args)
+		require.NoError(t, err)
+
+		assert.Equal(t, ep.dbVersion.Major, uint64(16))
+		assert.Equal(t, ep.dbVersion.Minor, uint64(10))
+		assert.Equal(t, ep.dbVersion.Patch, uint64(0))
+	})
+
+	t.Run("version with trailing characters from percona helm", func(t *testing.T) {
+		args := ExplainPlanArguments{
+			DBVersion: "17.7 - Percona Server for PostgreSQL 17.7.1",
+		}
+
+		ep, err := NewExplainPlan(args)
+		require.NoError(t, err)
+
+		assert.Equal(t, ep.dbVersion.Major, uint64(17))
+		assert.Equal(t, ep.dbVersion.Minor, uint64(7))
+		assert.Equal(t, ep.dbVersion.Patch, uint64(0))
+	})
 }
 
 func TestExplainPlan_Name(t *testing.T) {

internal/component/database_observability/postgres/component.go

@@ -11,7 +11,6 @@ import (
 	"sync"
 	"time"
 
-	"github.com/blang/semver/v4"
 	"github.com/lib/pq"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
@@ -447,17 +446,13 @@ func (c *Component) startCollectors(systemID string, engineVersion string, cloud
 	c.collectors = append(c.collectors, ciCollector)
 
 	if collectors[collector.ExplainPlanCollector] {
-		engineSemver, err := semver.ParseTolerant(engineVersion)
-		if err != nil {
-			logStartError(collector.ExplainPlanCollector, "parse version", err)
-		}
 		epCollector, err := collector.NewExplainPlan(collector.ExplainPlanArguments{
 			DB:             c.dbConnection,
 			DSN:            string(c.args.DataSourceName),
 			ScrapeInterval: c.args.ExplainPlanArguments.CollectInterval,
 			PerScrapeRatio: c.args.ExplainPlanArguments.PerCollectRatio,
 			Logger:         c.opts.Logger,
-			DBVersion:      engineSemver,
+			DBVersion:      engineVersion,
 			EntryHandler:   entryHandler,
 		})
 		if err != nil {