Skip to content

Commit 948764b

Browse files
authored
Feat : resolved review comments (#42)
resolved review comments
1 parent 28483e3 commit 948764b

21 files changed

+766
-70
lines changed

go.mod

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,12 @@ require (
66
github.com/blang/semver/v4 v4.0.0
77
github.com/jmoiron/sqlx v1.4.0
88
github.com/lib/pq v1.10.9
9-
github.com/mitchellh/mapstructure v1.5.0
109
github.com/newrelic/infra-integrations-sdk/v3 v3.9.1
1110
github.com/stretchr/testify v1.10.0
1211
github.com/xeipuuv/gojsonschema v1.2.0
1312
gopkg.in/DATA-DOG/go-sqlmock.v1 v1.3.0
1413
gopkg.in/yaml.v3 v3.0.1
14+
github.com/go-viper/mapstructure/v2 v2.2.1
1515
)
1616

1717
require (

go.sum

+2-2
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c
88
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
99
github.com/go-sql-driver/mysql v1.8.1 h1:LedoTUt/eveggdHS9qUFC1EFSa8bU2+1pZjSRpvNJ1Y=
1010
github.com/go-sql-driver/mysql v1.8.1/go.mod h1:wEBSXgmK//2ZFJyE+qWnIsVGmvmEKlqwuVSjsCm7DZg=
11+
github.com/go-viper/mapstructure/v2 v2.2.1 h1:ZAaOCxANMuZx5RCeg0mBdEZk7DZasvvZIxtHqx8aGss=
12+
github.com/go-viper/mapstructure/v2 v2.2.1/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM=
1113
github.com/jmoiron/sqlx v1.4.0 h1:1PLqN7S1UYp5t4SrVVnt4nUVNemrDAtxlulVe+Qgm3o=
1214
github.com/jmoiron/sqlx v1.4.0/go.mod h1:ZrZ7UsYB/weZdl2Bxg6jCRO9c3YHl8r3ahlKmRT4JLY=
1315
github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
@@ -18,8 +20,6 @@ github.com/lib/pq v1.10.9 h1:YXG7RB+JIjhP29X+OtkiDnYaXQwpS4JEWq7dtCCRUEw=
1820
github.com/lib/pq v1.10.9/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o=
1921
github.com/mattn/go-sqlite3 v1.14.22 h1:2gZY6PC6kBnID23Tichd1K+Z0oS6nE/XwU+Vz/5o4kU=
2022
github.com/mattn/go-sqlite3 v1.14.22/go.mod h1:Uh1q+B4BYcTPb+yiD3kU8Ct7aC0hY9fxUwlHK0RXw+Y=
21-
github.com/mitchellh/mapstructure v1.5.0 h1:jeMsZIYE/09sWLaz43PL7Gy6RuMjD2eJVyuac5Z2hdY=
22-
github.com/mitchellh/mapstructure v1.5.0/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo=
2323
github.com/newrelic/infra-integrations-sdk/v3 v3.9.1 h1:dCtVLsYNHWTQ5aAlAaHroomOUlqxlGTrdi6XTlvBDfI=
2424
github.com/newrelic/infra-integrations-sdk/v3 v3.9.1/go.mod h1:yPeidhcq9Cla0QDquGXH0KqvS2k9xtetFOD7aLA0Z8M=
2525
github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e h1:fD57ERR4JtEqsWbfPhv4DMiApHyliiK5xCTNVSPiaAs=
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
package commonutils_test
2+
3+
import (
4+
"sort"
5+
"testing"
6+
"time"
7+
8+
"github.com/newrelic/nri-postgresql/src/collection"
9+
commonutils "github.com/newrelic/nri-postgresql/src/query-performance-monitoring/common-utils"
10+
"github.com/stretchr/testify/assert"
11+
)
12+
13+
func TestGetQuotedStringFromArray(t *testing.T) {
14+
input := []string{"db1", "db2", "db3"}
15+
expected := "'db1','db2','db3'"
16+
result := commonutils.GetQuotedStringFromArray(input)
17+
assert.Equal(t, expected, result)
18+
}
19+
20+
func TestGetDatabaseListInString(t *testing.T) {
21+
dbListKeys := []string{"db1", "db2"}
22+
sort.Strings(dbListKeys) // Sort the keys to ensure consistent order
23+
dbList := collection.DatabaseList{}
24+
for _, key := range dbListKeys {
25+
dbList[key] = collection.SchemaList{}
26+
}
27+
expected := "'db1','db2'"
28+
result := commonutils.GetDatabaseListInString(dbList)
29+
assert.Equal(t, expected, result)
30+
31+
// Test with empty database list
32+
dbList = collection.DatabaseList{}
33+
expected = ""
34+
result = commonutils.GetDatabaseListInString(dbList)
35+
assert.Equal(t, expected, result)
36+
}
37+
38+
func TestAnonymizeQueryText(t *testing.T) {
39+
query := "SELECT * FROM users WHERE id = 1 AND name = 'John'"
40+
expected := "SELECT * FROM users WHERE id = ? AND name = ?"
41+
result := commonutils.AnonymizeQueryText(query)
42+
assert.Equal(t, expected, result)
43+
}
44+
45+
func TestGeneratePlanID(t *testing.T) {
46+
queryID := "query123"
47+
result := commonutils.GeneratePlanID(queryID)
48+
assert.NotNil(t, result)
49+
assert.Contains(t, *result, queryID)
50+
assert.Contains(t, *result, "-")
51+
assert.Contains(t, *result, time.Now().Format(commonutils.TimeFormat)[:8]) // Check date part
52+
}

src/query-performance-monitoring/common-utils/common-helpers.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@ import (
1111
"github.com/newrelic/nri-postgresql/src/collection"
1212
)
1313

14-
func getQuotedStringFromArray(array []string) string {
14+
var re = regexp.MustCompile(`'[^']*'|\d+|".*?"`)
15+
16+
func GetQuotedStringFromArray(array []string) string {
1517
var quotedNames = make([]string, 0)
1618
for _, name := range array {
1719
quotedNames = append(quotedNames, fmt.Sprintf("'%s'", name))
@@ -27,11 +29,10 @@ func GetDatabaseListInString(dbList collection.DatabaseList) string {
2729
if len(databaseNames) == 0 {
2830
return ""
2931
}
30-
return getQuotedStringFromArray(databaseNames)
32+
return GetQuotedStringFromArray(databaseNames)
3133
}
3234

3335
func AnonymizeQueryText(query string) string {
34-
re := regexp.MustCompile(`'[^']*'|\d+|".*?"`)
3536
anonymizedQuery := re.ReplaceAllString(query, "?")
3637
return anonymizedQuery
3738
}

src/query-performance-monitoring/common-utils/constants.go

+4
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,14 @@ const VersionRegex = "PostgreSQL (\\d+)\\."
1111

1212
var ErrParseVersion = errors.New("unable to parse PostgreSQL version from string")
1313
var ErrUnsupportedVersion = errors.New("unsupported PostgreSQL version")
14+
var ErrUnExpectedError = errors.New("unexpected error")
15+
1416
var ErrVersionFetchError = errors.New("no rows returned from version query")
1517
var ErrInvalidModelType = errors.New("invalid model type")
18+
var ErrNotEligible = errors.New("not Eligible to fetch metrics")
1619

1720
const PostgresVersion12 = 12
21+
const PostgresVersion11 = 11
1822
const PostgresVersion13 = 13
1923
const PostgresVersion14 = 14
2024
const VersionIndex = 2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
package commonutils_test
2+
3+
import (
4+
"testing"
5+
6+
"github.com/newrelic/infra-integrations-sdk/v3/integration"
7+
"github.com/newrelic/nri-postgresql/src/args"
8+
commonutils "github.com/newrelic/nri-postgresql/src/query-performance-monitoring/common-utils"
9+
"github.com/stretchr/testify/assert"
10+
)
11+
12+
func TestSetMetric(t *testing.T) {
13+
pgIntegration, _ := integration.New("test", "1.0.0")
14+
entity, _ := pgIntegration.Entity("test-entity", "test-type")
15+
16+
metricSet := entity.NewMetricSet("test-event")
17+
18+
commonutils.SetMetric(metricSet, "testGauge", 123.0, "gauge")
19+
assert.Equal(t, 123.0, metricSet.Metrics["testGauge"])
20+
21+
commonutils.SetMetric(metricSet, "testAttribute", "value", "attribute")
22+
assert.Equal(t, "value", metricSet.Metrics["testAttribute"])
23+
24+
commonutils.SetMetric(metricSet, "testDefault", 456.0, "unknown")
25+
assert.Equal(t, 456.0, metricSet.Metrics["testDefault"])
26+
}
27+
28+
func TestIngestMetric(t *testing.T) {
29+
pgIntegration, _ := integration.New("test", "1.0.0")
30+
args := args.ArgumentList{
31+
Hostname: "localhost",
32+
Port: "5432",
33+
}
34+
metricList := []interface{}{
35+
struct {
36+
TestField int `metric_name:"testField" source_type:"gauge"`
37+
}{TestField: 123},
38+
}
39+
40+
commonutils.IngestMetric(metricList, "testEvent", pgIntegration, args)
41+
assert.NotEmpty(t, pgIntegration.Entities)
42+
}
43+
44+
func TestCreateEntity(t *testing.T) {
45+
pgIntegration, _ := integration.New("test", "1.0.0")
46+
args := args.ArgumentList{
47+
Hostname: "localhost",
48+
Port: "5432",
49+
}
50+
51+
entity, err := commonutils.CreateEntity(pgIntegration, args)
52+
assert.NoError(t, err)
53+
assert.NotNil(t, entity)
54+
assert.Equal(t, "localhost:5432", entity.Metadata.Name)
55+
}
56+
57+
func TestProcessModel(t *testing.T) {
58+
pgIntegration, _ := integration.New("test", "1.0.0")
59+
entity, _ := pgIntegration.Entity("test-entity", "test-type")
60+
61+
metricSet := entity.NewMetricSet("test-event")
62+
63+
model := struct {
64+
TestField int `metric_name:"testField" source_type:"gauge"`
65+
}{TestField: 123}
66+
67+
err := commonutils.ProcessModel(model, metricSet)
68+
assert.NoError(t, err)
69+
assert.Equal(t, 123.0, metricSet.Metrics["testField"])
70+
}
71+
72+
func TestPublishMetrics(t *testing.T) {
73+
pgIntegration, _ := integration.New("test", "1.0.0")
74+
args := args.ArgumentList{
75+
Hostname: "localhost",
76+
Port: "5432",
77+
}
78+
entity, _ := commonutils.CreateEntity(pgIntegration, args)
79+
80+
err := commonutils.PublishMetrics(pgIntegration, &entity, args)
81+
assert.NoError(t, err)
82+
assert.NotNil(t, entity)
83+
}

src/query-performance-monitoring/common-utils/ingestion-helpers.go

+7-7
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ func SetMetric(metricSet *metric.Set, name string, value interface{}, sourceType
3434
}
3535

3636
func IngestMetric(metricList []interface{}, eventName string, pgIntegration *integration.Integration, args args.ArgumentList) {
37-
instanceEntity, err := createEntity(pgIntegration, args)
37+
instanceEntity, err := CreateEntity(pgIntegration, args)
3838
if err != nil {
3939
log.Error("Error creating entity: %v", err)
4040
return
@@ -50,33 +50,33 @@ func IngestMetric(metricList []interface{}, eventName string, pgIntegration *int
5050
metricCount += 1
5151
metricSet := instanceEntity.NewMetricSet(eventName)
5252

53-
processErr := processModel(model, metricSet)
53+
processErr := ProcessModel(model, metricSet)
5454
if processErr != nil {
5555
log.Error("Error processing model: %v", processErr)
5656
continue
5757
}
5858

5959
if metricCount == PublishThreshold || metricCount == lenOfMetricList {
6060
metricCount = 0
61-
if err := publishMetrics(pgIntegration, &instanceEntity, args); err != nil {
61+
if err := PublishMetrics(pgIntegration, &instanceEntity, args); err != nil {
6262
log.Error("Error publishing metrics: %v", err)
6363
return
6464
}
6565
}
6666
}
6767
if metricCount > 0 {
68-
if err := publishMetrics(pgIntegration, &instanceEntity, args); err != nil {
68+
if err := PublishMetrics(pgIntegration, &instanceEntity, args); err != nil {
6969
log.Error("Error publishing metrics: %v", err)
7070
return
7171
}
7272
}
7373
}
7474

75-
func createEntity(pgIntegration *integration.Integration, args args.ArgumentList) (*integration.Entity, error) {
75+
func CreateEntity(pgIntegration *integration.Integration, args args.ArgumentList) (*integration.Entity, error) {
7676
return pgIntegration.Entity(fmt.Sprintf("%s:%s", args.Hostname, args.Port), "pg-instance")
7777
}
7878

79-
func processModel(model interface{}, metricSet *metric.Set) error {
79+
func ProcessModel(model interface{}, metricSet *metric.Set) error {
8080
modelValue := reflect.ValueOf(model)
8181
if modelValue.Kind() == reflect.Ptr {
8282
modelValue = modelValue.Elem()
@@ -108,7 +108,7 @@ func processModel(model interface{}, metricSet *metric.Set) error {
108108
return nil
109109
}
110110

111-
func publishMetrics(pgIntegration *integration.Integration, instanceEntity **integration.Entity, args args.ArgumentList) error {
111+
func PublishMetrics(pgIntegration *integration.Integration, instanceEntity **integration.Entity, args args.ArgumentList) error {
112112
if err := pgIntegration.Publish(); err != nil {
113113
return err
114114
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
package commonutils_test
2+
3+
import (
4+
"testing"
5+
6+
commonutils "github.com/newrelic/nri-postgresql/src/query-performance-monitoring/common-utils"
7+
8+
"github.com/newrelic/nri-postgresql/src/query-performance-monitoring/queries"
9+
"github.com/stretchr/testify/assert"
10+
)
11+
12+
func runTestCases(t *testing.T, tests []struct {
13+
version uint64
14+
expected string
15+
expectErr bool
16+
}, fetchFunc func(uint64) (string, error)) {
17+
for _, test := range tests {
18+
result, err := fetchFunc(test.version)
19+
if test.expectErr {
20+
assert.Error(t, err)
21+
} else {
22+
assert.NoError(t, err)
23+
assert.Equal(t, test.expected, result)
24+
}
25+
}
26+
}
27+
28+
func TestFetchVersionSpecificSlowQueries(t *testing.T) {
29+
tests := []struct {
30+
version uint64
31+
expected string
32+
expectErr bool
33+
}{
34+
{commonutils.PostgresVersion12, queries.SlowQueriesForV12, false},
35+
{commonutils.PostgresVersion13, queries.SlowQueriesForV13AndAbove, false},
36+
{commonutils.PostgresVersion11, "", true},
37+
}
38+
39+
runTestCases(t, tests, commonutils.FetchVersionSpecificSlowQueries)
40+
}
41+
42+
func TestFetchVersionSpecificBlockingQueries(t *testing.T) {
43+
tests := []struct {
44+
version uint64
45+
expected string
46+
expectErr bool
47+
}{
48+
{commonutils.PostgresVersion12, queries.BlockingQueriesForV12AndV13, false},
49+
{commonutils.PostgresVersion13, queries.BlockingQueriesForV12AndV13, false},
50+
{commonutils.PostgresVersion14, queries.BlockingQueriesForV14AndAbove, false},
51+
{commonutils.PostgresVersion11, "", true},
52+
}
53+
54+
runTestCases(t, tests, commonutils.FetchVersionSpecificBlockingQueries)
55+
}
56+
57+
func TestFetchVersionSpecificIndividualQueries(t *testing.T) {
58+
tests := []struct {
59+
version uint64
60+
expected string
61+
expectErr bool
62+
}{
63+
{commonutils.PostgresVersion12, queries.IndividualQuerySearchV12, false},
64+
{commonutils.PostgresVersion13, queries.IndividualQuerySearchV13AndAbove, false},
65+
{commonutils.PostgresVersion14, queries.IndividualQuerySearchV13AndAbove, false},
66+
{commonutils.PostgresVersion11, "", true},
67+
}
68+
69+
runTestCases(t, tests, commonutils.FetchVersionSpecificIndividualQueries)
70+
}

src/query-performance-monitoring/performance-metrics/blocking_sessions.go

+9-8
Original file line numberDiff line numberDiff line change
@@ -13,26 +13,27 @@ import (
1313
"github.com/newrelic/nri-postgresql/src/query-performance-monitoring/datamodels"
1414
)
1515

16-
func PopulateBlockingMetrics(conn *performancedbconnection.PGSQLConnection, pgIntegration *integration.Integration, args args.ArgumentList, databaseName string, version uint64) {
17-
isPgStatStatementEnabled, enableCheckError := validations.CheckBlockingSessionMetricsFetchEligibility(conn, version)
16+
func PopulateBlockingMetrics(conn *performancedbconnection.PGSQLConnection, pgIntegration *integration.Integration, args args.ArgumentList, databaseName string, version uint64) error {
17+
isEligible, enableCheckError := validations.CheckBlockingSessionMetricsFetchEligibility(conn, version)
1818
if enableCheckError != nil {
1919
log.Debug("Error executing query: %v in PopulateBlockingMetrics", enableCheckError)
20-
return
20+
return commonutils.ErrUnExpectedError
2121
}
22-
if !isPgStatStatementEnabled {
23-
log.Debug("Extension 'pg_stat_statements' is not enabled for the database.")
24-
return
22+
if !isEligible {
23+
log.Debug("Extension 'pg_stat_statements' is not enabled or unsupported version.")
24+
return commonutils.ErrNotEligible
2525
}
2626
blockingQueriesMetricsList, blockQueryFetchErr := GetBlockingMetrics(conn, args, databaseName, version)
2727
if blockQueryFetchErr != nil {
2828
log.Error("Error fetching Blocking queries: %v", blockQueryFetchErr)
29-
return
29+
return commonutils.ErrUnExpectedError
3030
}
3131
if len(blockingQueriesMetricsList) == 0 {
3232
log.Debug("No Blocking queries found.")
33-
return
33+
return nil
3434
}
3535
commonutils.IngestMetric(blockingQueriesMetricsList, "PostgresBlockingSessions", pgIntegration, args)
36+
return nil
3637
}
3738

3839
func GetBlockingMetrics(conn *performancedbconnection.PGSQLConnection, args args.ArgumentList, databaseName string, version uint64) ([]interface{}, error) {

0 commit comments

Comments
 (0)