Added support for pgBouncer v1.23 (#177)

* Added support for pgBouncer v1.23 Testcases added for v1.23 Docker compose file added for setting up pgBouncer environment in the future. * Modified tests to not use if/else condition Added steps to add support for new pgbouncer versions Tests no longer use the if/else conditions to check for nil and instead reuse and Added comments for columns added in pgbouncer v1.23 * changelog entry for pgbouncer
newrelic · Nov 6, 2024 · c59d41b · c59d41b
1 parent 22cc710
commit c59d41b
Show file tree

Hide file tree

Showing 5 changed files with 184 additions and 38 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,9 @@ Unreleased section should follow [Release Toolkit](https://github.com/newrelic/r
 
 ## Unreleased
 
+### Enhancements
+- Added support for pgbouncer v1.23 with new columns in `STATS` table.
+
 ## v2.15.0 - 2024-10-07
 
 ### dependency

diff --git a/src/metrics/metrics_test.go b/src/metrics/metrics_test.go
@@ -303,7 +303,7 @@ func Test_populateTableMetricsForDatabase(t *testing.T) {
 	assert.Equal(t, expectedBase, tableEntity.Metrics[1].Metrics)
 }
 
-func Test_populateTableMetricsForDatabase_noTables(t *testing.T) {
+func TestPopulateTableMetricsForDatabaseNoTables(t *testing.T) {
 	testIntegration, _ := integration.New("test", "test")
 
 	dbList := collection.DatabaseList{
@@ -323,7 +323,7 @@ func Test_populateTableMetricsForDatabase_noTables(t *testing.T) {
 	assert.Equal(t, 0, len(tableEntity.Metrics))
 }
 
-func Test_populateIndexMetricsForDatabase(t *testing.T) {
+func TestPopulateIndexMetricsForDatabase(t *testing.T) {
 	testIntegration, _ := integration.New("test", "test")
 
 	dbList := collection.DatabaseList{
@@ -413,7 +413,7 @@ func Test_populateIndexMetricsForDatabase(t *testing.T) {
 	assert.Equal(t, expected2, indexEntity2.Metrics[0].Metrics)
 }
 
-func Test_populateIndexMetricsForDatabase_noIndexes(t *testing.T) {
+func TestPopulateIndexMetricsForDatabaseNoIndexes(t *testing.T) {
 	testIntegration, _ := integration.New("test", "test")
 
 	dbList := collection.DatabaseList{
@@ -435,10 +435,54 @@ func Test_populateIndexMetricsForDatabase_noIndexes(t *testing.T) {
 }
 
 func TestPopulatePgBouncerMetrics(t *testing.T) {
+
+	pgbouncerPriorTo23StatsRows := func() *sqlmock.Rows {
+		return sqlmock.NewRows([]string{
+			"database",
+			"total_xact_count",
+			"total_query_count",
+			"total_received",
+			"total_sent",
+			"total_xact_time",
+			"total_query_time",
+			"total_wait_time",
+			"avg_xact_count",
+			"avg_xact_time",
+			"avg_query_count",
+			"avg_recv",
+			"avg_sent",
+			"avg_query_time",
+			"avg_wait_time",
+		}).AddRow("testDB", 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14)
+	}
+
+	pgbouncerPriorTo23ExpectedStats := func() map[string]interface{} {
+		return map[string]interface{}{
+			"pgbouncer.stats.transactionsPerSecond":                           float64(0),
+			"pgbouncer.stats.queriesPerSecond":                                float64(0),
+			"pgbouncer.stats.bytesInPerSecond":                                float64(0),
+			"pgbouncer.stats.bytesOutPerSecond":                               float64(0),
+			"pgbouncer.stats.totalTransactionDurationInMillisecondsPerSecond": float64(0),
+			"pgbouncer.stats.totalQueryDurationInMillisecondsPerSecond":       float64(0),
+			"pgbouncer.stats.avgTransactionCount":                             float64(8),
+			"pgbouncer.stats.avgTransactionDurationInMilliseconds":            float64(9),
+			"pgbouncer.stats.avgQueryCount":                                   float64(10),
+			"pgbouncer.stats.avgBytesIn":                                      float64(11),
+			"pgbouncer.stats.avgBytesOut":                                     float64(12),
+			"pgbouncer.stats.avgQueryDurationInMilliseconds":                  float64(13),
+			"displayName": "testDB",
+			"entityName":  "pgbouncer:testDB",
+			"event_type":  "PgBouncerSample",
+			"host":        "testhost",
+		}
+	}
+
 	testsCases := []struct {
 		name               string
 		pgbouncerPoolsRows *sqlmock.Rows
+		pgbouncerStatsRows *sqlmock.Rows
 		expectedPool       map[string]interface{}
+		expectedStats      map[string]interface{}
 	}{
 		{
 			name: "pgbouncer version =< 15",
@@ -456,6 +500,8 @@ func TestPopulatePgBouncerMetrics(t *testing.T) {
 				"maxwait_us",
 				"pool_mode",
 			}).AddRow("testDB", "testUser", 1, 2, 3, 4, 5, 6, 7, 8, 9, "testMode"),
+			pgbouncerStatsRows: pgbouncerPriorTo23StatsRows(),
+			expectedStats:      pgbouncerPriorTo23ExpectedStats(),
 			expectedPool: map[string]interface{}{
 				"pgbouncer.pools.clientConnectionsActive":  float64(1),
 				"pgbouncer.pools.clientConnectionsWaiting": float64(2),
@@ -489,6 +535,8 @@ func TestPopulatePgBouncerMetrics(t *testing.T) {
 				"pool_mode",
 				"cl_cancel_req", // Added column.
 			}).AddRow("testDB", "testUser", 1, 2, 3, 4, 5, 6, 7, 8, 9, "testMode", 10),
+			pgbouncerStatsRows: pgbouncerPriorTo23StatsRows(),
+			expectedStats:      pgbouncerPriorTo23ExpectedStats(),
 			expectedPool: map[string]interface{}{
 				"pgbouncer.pools.clientConnectionsActive":    float64(1),
 				"pgbouncer.pools.clientConnectionsWaiting":   float64(2),
@@ -526,6 +574,8 @@ func TestPopulatePgBouncerMetrics(t *testing.T) {
 				"sv_active_cancel",
 				"sv_being_canceled",
 			}).AddRow("testDB", "testUser", 1, 2, 3, 4, 5, 6, 7, 8, 9, "testMode", 10, 11, 12, 13),
+			pgbouncerStatsRows: pgbouncerPriorTo23StatsRows(),
+			expectedStats:      pgbouncerPriorTo23ExpectedStats(),
 			expectedPool: map[string]interface{}{
 				"pgbouncer.pools.clientConnectionsActive":           float64(1),
 				"pgbouncer.pools.clientConnectionsWaiting":          float64(2),
@@ -546,17 +596,27 @@ func TestPopulatePgBouncerMetrics(t *testing.T) {
 				"pgbouncer.pools.user":                              "testUser",
 			},
 		},
-	}
-
-	for _, testCase := range testsCases {
-		testCase := testCase
-
-		t.Run(testCase.name, func(t *testing.T) {
-			testIntegration, _ := integration.New("test", "test")
-
-			testConnection, mock := connection.CreateMockSQL(t)
-
-			pgbouncerStatsRows := sqlmock.NewRows([]string{
+		{
+			name: "pgbouncer version >= 23",
+			pgbouncerPoolsRows: sqlmock.NewRows([]string{
+				"database",
+				"user",
+				"cl_active",
+				"cl_waiting",
+				"sv_active",
+				"sv_idle",
+				"sv_used",
+				"sv_tested",
+				"sv_login",
+				"maxwait",
+				"maxwait_us",
+				"pool_mode",
+				"cl_waiting_cancel_req",
+				"cl_active_cancel_req",
+				"sv_active_cancel",
+				"sv_being_canceled",
+			}).AddRow("testDB", "testUser", 1, 2, 3, 4, 5, 6, 7, 8, 9, "testMode", 10, 11, 12, 13),
+			pgbouncerStatsRows: sqlmock.NewRows([]string{
 				"database",
 				"total_xact_count",
 				"total_query_count",
@@ -572,9 +632,29 @@ func TestPopulatePgBouncerMetrics(t *testing.T) {
 				"avg_sent",
 				"avg_query_time",
 				"avg_wait_time",
-			}).AddRow("testDB", 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14)
-
-			expectedStats := map[string]interface{}{
+				"total_server_assignment_count", // New in v23
+				"avg_server_assignment_count",   // New in v23
+			}).AddRow("testDB", 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16),
+			expectedPool: map[string]interface{}{
+				"pgbouncer.pools.clientConnectionsActive":           float64(1),
+				"pgbouncer.pools.clientConnectionsWaiting":          float64(2),
+				"pgbouncer.pools.serverConnectionsActive":           float64(3),
+				"pgbouncer.pools.serverConnectionsIdle":             float64(4),
+				"pgbouncer.pools.serverConnectionsUsed":             float64(5),
+				"pgbouncer.pools.serverConnectionsTested":           float64(6),
+				"pgbouncer.pools.serverConnectionsLogin":            float64(7),
+				"pgbouncer.pools.maxwaitInMilliseconds":             float64(8),
+				"displayName":                                       "testDB",
+				"entityName":                                        "pgbouncer:testDB",
+				"event_type":                                        "PgBouncerSample",
+				"host":                                              "testhost",
+				"pgbouncer.pools.clientConnectionsWaitingCancelReq": float64(10),
+				"pgbouncer.pools.clientConnectionsActiveCancelReq":  float64(11),
+				"pgbouncer.pools.serverConnectionsActiveCancel":     float64(12),
+				"pgbouncer.pools.serverConnectionsBeingCancel":      float64(13),
+				"pgbouncer.pools.user":                              "testUser",
+			},
+			expectedStats: map[string]interface{}{
 				"pgbouncer.stats.transactionsPerSecond":                           float64(0),
 				"pgbouncer.stats.queriesPerSecond":                                float64(0),
 				"pgbouncer.stats.bytesInPerSecond":                                float64(0),
@@ -587,15 +667,27 @@ func TestPopulatePgBouncerMetrics(t *testing.T) {
 				"pgbouncer.stats.avgBytesIn":                                      float64(11),
 				"pgbouncer.stats.avgBytesOut":                                     float64(12),
 				"pgbouncer.stats.avgQueryDurationInMilliseconds":                  float64(13),
-
+				"pgbouncer.stats.totalServerAssignmentCount":                      float64(15),
+				"pgbouncer.stats.avgServerAssignmentCount":                        float64(16),
 				"displayName": "testDB",
 				"entityName":  "pgbouncer:testDB",
 				"event_type":  "PgBouncerSample",
 				"host":        "testhost",
-			}
+			},
+		},
+	}
+
+	for _, testCase := range testsCases {
+		testCase := testCase
+
+		t.Run(testCase.name, func(t *testing.T) {
+			testIntegration, _ := integration.New("test", "test")
+
+			testConnection, mock := connection.CreateMockSQL(t)
 
 			mock.ExpectQuery("SHOW STATS;").
-				WillReturnRows(pgbouncerStatsRows)
+				WillReturnRows(testCase.pgbouncerStatsRows)
+
 			mock.ExpectQuery("SHOW POOLS;").
 				WillReturnRows(testCase.pgbouncerPoolsRows)
 
@@ -606,7 +698,8 @@ func TestPopulatePgBouncerMetrics(t *testing.T) {
 			id4 := integration.NewIDAttribute("port", "1234")
 			pbEntity, err := testIntegration.Entity("testDB", "pgbouncer", id3, id4)
 			assert.Nil(t, err)
-			assert.Equal(t, expectedStats, pbEntity.Metrics[0].Metrics)
+			assert.Equal(t, len(pbEntity.Metrics), 2)
+			assert.Equal(t, testCase.expectedStats, pbEntity.Metrics[0].Metrics)
 			assert.Equal(t, testCase.expectedPool, pbEntity.Metrics[1].Metrics)
 		})
 	}

diff --git a/src/metrics/pgbouncer_definitions.go b/src/metrics/pgbouncer_definitions.go
@@ -13,23 +13,25 @@ var pgbouncerStatsDefinition = &QueryDefinition{
 
 	dataModels: []struct {
 		databaseBase
-		TotalXactCount  *int64 `db:"total_xact_count"  metric_name:"pgbouncer.stats.transactionsPerSecond"                           source_type:"rate"`
-		TotalQueryCount *int64 `db:"total_query_count" metric_name:"pgbouncer.stats.queriesPerSecond"                                source_type:"rate"`
-		TotalReceived   *int64 `db:"total_received"    metric_name:"pgbouncer.stats.bytesInPerSecond"                                source_type:"rate"`
-		TotalSent       *int64 `db:"total_sent"        metric_name:"pgbouncer.stats.bytesOutPerSecond"                               source_type:"rate"`
-		TotalXactTime   *int64 `db:"total_xact_time"   metric_name:"pgbouncer.stats.totalTransactionDurationInMillisecondsPerSecond" source_type:"rate"`
-		TotalQueryTime  *int64 `db:"total_query_time"  metric_name:"pgbouncer.stats.totalQueryDurationInMillisecondsPerSecond"       source_type:"rate"`
-		TotalRequests   *int64 `db:"total_requests"    metric_name:"pgbouncer.stats.requestsPerSecond"                               source_type:"rate"`
-		TotalWaitTime   *int64 `db:"total_wait_time"`
-		AvgXactCount    *int64 `db:"avg_xact_count"    metric_name:"pgbouncer.stats.avgTransactionCount"                             source_type:"gauge"`
-		AvgXactTime     *int64 `db:"avg_xact_time"     metric_name:"pgbouncer.stats.avgTransactionDurationInMilliseconds"            source_type:"gauge"`
-		AvgQueryCount   *int64 `db:"avg_query_count"   metric_name:"pgbouncer.stats.avgQueryCount"                                   source_type:"gauge"`
-		AvgRecv         *int64 `db:"avg_recv"          metric_name:"pgbouncer.stats.avgBytesIn"                                      source_type:"gauge"`
-		AvgSent         *int64 `db:"avg_sent"          metric_name:"pgbouncer.stats.avgBytesOut"                                     source_type:"gauge"`
-		AvgReq          *int64 `db:"avg_req"           metric_name:"pgbouncer.stats.avgRequestsPerSecond"                            source_type:"gauge"`
-		AvgQueryTime    *int64 `db:"avg_query_time"    metric_name:"pgbouncer.stats.avgQueryDurationInMilliseconds"                  source_type:"gauge"`
-		AvgQuery        *int64 `db:"avg_query"         metric_name:"pgbouncer.stats.avgQueryDurationInMilliseconds"                  source_type:"gauge"`
-		AvgWaitTime     *int64 `db:"avg_wait_time"`
+		TotalXactCount             *int64 `db:"total_xact_count"  metric_name:"pgbouncer.stats.transactionsPerSecond"                           source_type:"rate"`
+		TotalQueryCount            *int64 `db:"total_query_count" metric_name:"pgbouncer.stats.queriesPerSecond"                                source_type:"rate"`
+		TotalServerAssignmentCount *int64 `db:"total_server_assignment_count" metric_name:"pgbouncer.stats.totalServerAssignmentCount" source_type:"gauge"` // added in v1.23
+		TotalReceived              *int64 `db:"total_received"    metric_name:"pgbouncer.stats.bytesInPerSecond"                                source_type:"rate"`
+		TotalSent                  *int64 `db:"total_sent"        metric_name:"pgbouncer.stats.bytesOutPerSecond"                               source_type:"rate"`
+		TotalXactTime              *int64 `db:"total_xact_time"   metric_name:"pgbouncer.stats.totalTransactionDurationInMillisecondsPerSecond" source_type:"rate"`
+		TotalQueryTime             *int64 `db:"total_query_time"  metric_name:"pgbouncer.stats.totalQueryDurationInMillisecondsPerSecond"       source_type:"rate"`
+		TotalRequests              *int64 `db:"total_requests"    metric_name:"pgbouncer.stats.requestsPerSecond"                               source_type:"rate"`
+		TotalWaitTime              *int64 `db:"total_wait_time"`
+		AvgXactCount               *int64 `db:"avg_xact_count"    metric_name:"pgbouncer.stats.avgTransactionCount"                             source_type:"gauge"`
+		AvgXactTime                *int64 `db:"avg_xact_time"     metric_name:"pgbouncer.stats.avgTransactionDurationInMilliseconds"            source_type:"gauge"`
+		AvgQueryCount              *int64 `db:"avg_query_count"   metric_name:"pgbouncer.stats.avgQueryCount"                                   source_type:"gauge"`
+		AvgServerAssignmentCount   *int64 `db:"avg_server_assignment_count" metric_name:"pgbouncer.stats.avgServerAssignmentCount" source_type:"gauge"` // added in v1.23
+		AvgRecv                    *int64 `db:"avg_recv"          metric_name:"pgbouncer.stats.avgBytesIn"                                      source_type:"gauge"`
+		AvgSent                    *int64 `db:"avg_sent"          metric_name:"pgbouncer.stats.avgBytesOut"                                     source_type:"gauge"`
+		AvgReq                     *int64 `db:"avg_req"           metric_name:"pgbouncer.stats.avgRequestsPerSecond"                            source_type:"gauge"`
+		AvgQueryTime               *int64 `db:"avg_query_time"    metric_name:"pgbouncer.stats.avgQueryDurationInMilliseconds"                  source_type:"gauge"`
+		AvgQuery                   *int64 `db:"avg_query"         metric_name:"pgbouncer.stats.avgQueryDurationInMilliseconds"                  source_type:"gauge"`
+		AvgWaitTime                *int64 `db:"avg_wait_time"`
 	}{},
 }
 

diff --git a/tests/README.md b/tests/README.md
@@ -10,3 +10,19 @@ Steps to update the integration tests for the latest supported version:
         - Check the release notes ([Postgres 16 example](https://www.postgresql.org/docs/release/16.0/))
 3. Once the failures are understood (if any), update the corresponding JSON-schema files, you may need to generate it
    using the integration output, specially if there is any metric failure.
+
+# Testing pgbouncer upgrades
+
+Steps to test breaking metrics changes that happen when new pgbouncer versions are released:
+
+1. Update the `db` image and `pgbouncer` images in [docker compose](./docker-compose-pgbouncer.yml).
+2. Use the command `docker compose -f ./docker-compose-pgbouncer.yml up` to get the environment running
+3. Run the integration with `go run ./src/main.go -pgbouncer -username {USERNAME} -password {PASSWORD} -p 5432 -pretty > pgbouncer_output.json`
+    * If the terminal logs errors:
+        - Check which query is failing
+        - Explore pgbouncer release notes for changes to the `STATS` and `POOLS` tables
+        - Add or remove metrics to make the query succeed
+        - Modify tests to check for the new metrics in the latest versions
+    * No errors:
+        - Take a look at `pgbouncer_output.json` and look at the pgbouncer entities and check if metrics are reported correctly.
+        - If metrics are incorrectly reported, go back and look at where queries might be failing.
diff --git a/tests/docker-compose-pgbouncer.yml b/tests/docker-compose-pgbouncer.yml
@@ -0,0 +1,32 @@
+services:
+  db:
+    container_name: db
+    image: postgres:16-alpine
+    volumes:
+      - pg_data:/var/lib/postgresql/data
+    environment:
+      - POSTGRES_USER=postgres
+      - POSTGRES_PASSWORD=hbZkzny5xrvVH
+    healthcheck:
+      test: ['CMD', 'pg_isready', '-U', 'postgres']
+
+  pgbouncer:
+    container_name: pgbouncer
+    image: edoburu/pgbouncer:latest
+    environment:
+      - DB_USER=postgres
+      - DB_PASSWORD=hbZkzny5xrvVH
+      - DB_HOST=db
+      # - DB_NAME=test
+      - AUTH_TYPE=scram-sha-256
+      - POOL_MODE=transaction
+      - ADMIN_USERS=postgres,dbuser
+    ports:
+      - "5432:5432"
+    depends_on:
+      - db
+    healthcheck:
+      test: ['CMD', 'pg_isready', '-h', 'localhost']
+
+volumes:
+  pg_data: