Skip to content

Commit

Permalink
resolved: review comments (#44)
Browse files Browse the repository at this point in the history
* resolved: review comments
  • Loading branch information
tharun0064 authored Jan 16, 2025
1 parent 948764b commit 8509bcd
Show file tree
Hide file tree
Showing 20 changed files with 364 additions and 318 deletions.
3 changes: 2 additions & 1 deletion src/main.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//go:generate goversioninfo
package main

import (
Expand Down Expand Up @@ -91,7 +92,7 @@ func main() {
log.Error(err.Error())
}

if args.EnableQueryMonitoring {
if args.EnableQueryMonitoring && args.HasMetrics() {
queryperformancemonitoring.QueryPerformanceMain(args, pgIntegration, collectionList)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/newrelic/nri-postgresql/src/collection"
)

// re is a regular expression that matches single-quoted strings, numbers, or double-quoted strings
var re = regexp.MustCompile(`'[^']*'|\d+|".*?"`)

func GetQuotedStringFromArray(array []string) string {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ func TestGetQuotedStringFromArray(t *testing.T) {
}

func TestGetDatabaseListInString(t *testing.T) {
dbListKeys := []string{"db1", "db2"}
dbListKeys := []string{"db1"}
sort.Strings(dbListKeys) // Sort the keys to ensure consistent order
dbList := collection.DatabaseList{}
for _, key := range dbListKeys {
dbList[key] = collection.SchemaList{}
}
expected := "'db1','db2'"
expected := "'db1'"
result := commonutils.GetDatabaseListInString(dbList)
assert.Equal(t, expected, result)

Expand Down
20 changes: 11 additions & 9 deletions src/query-performance-monitoring/common-utils/ingestion-helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ import (
"fmt"
"reflect"

globalvariables "github.com/newrelic/nri-postgresql/src/query-performance-monitoring/global-variables"

"github.com/newrelic/infra-integrations-sdk/v3/data/metric"
"github.com/newrelic/infra-integrations-sdk/v3/integration"
"github.com/newrelic/infra-integrations-sdk/v3/log"
"github.com/newrelic/nri-postgresql/src/args"
)

func SetMetric(metricSet *metric.Set, name string, value interface{}, sourceType string) {
Expand All @@ -33,8 +34,9 @@ func SetMetric(metricSet *metric.Set, name string, value interface{}, sourceType
}
}

func IngestMetric(metricList []interface{}, eventName string, pgIntegration *integration.Integration, args args.ArgumentList) {
instanceEntity, err := CreateEntity(pgIntegration, args)
// IngestMetric is a util by which we publish data in batches .Reason for this is to avoid publishing large data in one go and its a limitation for NewRelic.
func IngestMetric(metricList []interface{}, eventName string, pgIntegration *integration.Integration, gv *globalvariables.GlobalVariables) {
instanceEntity, err := CreateEntity(pgIntegration, gv)
if err != nil {
log.Error("Error creating entity: %v", err)
return
Expand All @@ -58,22 +60,22 @@ func IngestMetric(metricList []interface{}, eventName string, pgIntegration *int

if metricCount == PublishThreshold || metricCount == lenOfMetricList {
metricCount = 0
if err := PublishMetrics(pgIntegration, &instanceEntity, args); err != nil {
if err := PublishMetrics(pgIntegration, &instanceEntity, gv); err != nil {
log.Error("Error publishing metrics: %v", err)
return
}
}
}
if metricCount > 0 {
if err := PublishMetrics(pgIntegration, &instanceEntity, args); err != nil {
if err := PublishMetrics(pgIntegration, &instanceEntity, gv); err != nil {
log.Error("Error publishing metrics: %v", err)
return
}
}
}

func CreateEntity(pgIntegration *integration.Integration, args args.ArgumentList) (*integration.Entity, error) {
return pgIntegration.Entity(fmt.Sprintf("%s:%s", args.Hostname, args.Port), "pg-instance")
func CreateEntity(pgIntegration *integration.Integration, gv *globalvariables.GlobalVariables) (*integration.Entity, error) {
return pgIntegration.Entity(fmt.Sprintf("%s:%s", gv.Hostname, gv.Port), "pg-instance")
}

func ProcessModel(model interface{}, metricSet *metric.Set) error {
Expand Down Expand Up @@ -108,11 +110,11 @@ func ProcessModel(model interface{}, metricSet *metric.Set) error {
return nil
}

func PublishMetrics(pgIntegration *integration.Integration, instanceEntity **integration.Entity, args args.ArgumentList) error {
func PublishMetrics(pgIntegration *integration.Integration, instanceEntity **integration.Entity, gv *globalvariables.GlobalVariables) error {
if err := pgIntegration.Publish(); err != nil {
return err
}
var err error
*instanceEntity, err = pgIntegration.Entity(fmt.Sprintf("%s:%s", args.Hostname, args.Port), "pg-instance")
*instanceEntity, err = CreateEntity(pgIntegration, gv)
return err
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package commonutils_test
import (
"testing"

global_variables "github.com/newrelic/nri-postgresql/src/query-performance-monitoring/global-variables"

"github.com/newrelic/infra-integrations-sdk/v3/integration"
"github.com/newrelic/nri-postgresql/src/args"
commonutils "github.com/newrelic/nri-postgresql/src/query-performance-monitoring/common-utils"
Expand All @@ -12,15 +14,11 @@ import (
func TestSetMetric(t *testing.T) {
pgIntegration, _ := integration.New("test", "1.0.0")
entity, _ := pgIntegration.Entity("test-entity", "test-type")

metricSet := entity.NewMetricSet("test-event")

commonutils.SetMetric(metricSet, "testGauge", 123.0, "gauge")
assert.Equal(t, 123.0, metricSet.Metrics["testGauge"])

commonutils.SetMetric(metricSet, "testAttribute", "value", "attribute")
assert.Equal(t, "value", metricSet.Metrics["testAttribute"])

commonutils.SetMetric(metricSet, "testDefault", 456.0, "unknown")
assert.Equal(t, 456.0, metricSet.Metrics["testDefault"])
}
Expand All @@ -31,13 +29,13 @@ func TestIngestMetric(t *testing.T) {
Hostname: "localhost",
Port: "5432",
}
gv := global_variables.SetGlobalVariables(args, uint64(14), "testdb")
metricList := []interface{}{
struct {
TestField int `metric_name:"testField" source_type:"gauge"`
}{TestField: 123},
}

commonutils.IngestMetric(metricList, "testEvent", pgIntegration, args)
commonutils.IngestMetric(metricList, "testEvent", pgIntegration, gv)
assert.NotEmpty(t, pgIntegration.Entities)
}

Expand All @@ -47,8 +45,9 @@ func TestCreateEntity(t *testing.T) {
Hostname: "localhost",
Port: "5432",
}
gv := global_variables.SetGlobalVariables(args, uint64(14), "testdb")

entity, err := commonutils.CreateEntity(pgIntegration, args)
entity, err := commonutils.CreateEntity(pgIntegration, gv)
assert.NoError(t, err)
assert.NotNil(t, entity)
assert.Equal(t, "localhost:5432", entity.Metadata.Name)
Expand All @@ -75,9 +74,10 @@ func TestPublishMetrics(t *testing.T) {
Hostname: "localhost",
Port: "5432",
}
entity, _ := commonutils.CreateEntity(pgIntegration, args)
gv := global_variables.SetGlobalVariables(args, uint64(14), "testdb")
entity, _ := commonutils.CreateEntity(pgIntegration, gv)

err := commonutils.PublishMetrics(pgIntegration, &entity, args)
err := commonutils.PublishMetrics(pgIntegration, &entity, gv)
assert.NoError(t, err)
assert.NotNil(t, entity)
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func FetchVersionSpecificIndividualQueries(version uint64) (string, error) {
switch {
case version == PostgresVersion12:
return queries.IndividualQuerySearchV12, nil
case version >= PostgresVersion12:
case version > PostgresVersion12:
return queries.IndividualQuerySearchV13AndAbove, nil
default:
return "", ErrUnsupportedVersion
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package globalvariables

import (
"github.com/newrelic/nri-postgresql/src/args"
)

type GlobalVariables struct {
QueryResponseTimeThreshold int
QueryCountThreshold int
Version uint64
DatabaseString string
Hostname string
Port string
Arguments args.ArgumentList
}

func SetGlobalVariables(args args.ArgumentList, version uint64, databaseString string) *GlobalVariables {
return &GlobalVariables{
QueryResponseTimeThreshold: args.QueryResponseTimeThreshold,
QueryCountThreshold: args.QueryCountThreshold,
Version: version,
DatabaseString: databaseString,
Hostname: args.Hostname,
Port: args.Port,
Arguments: args,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,19 @@ package performancemetrics
import (
"fmt"

globalvariables "github.com/newrelic/nri-postgresql/src/query-performance-monitoring/global-variables"

"github.com/newrelic/infra-integrations-sdk/v3/integration"
commonutils "github.com/newrelic/nri-postgresql/src/query-performance-monitoring/common-utils"
"github.com/newrelic/nri-postgresql/src/query-performance-monitoring/validations"

"github.com/newrelic/infra-integrations-sdk/v3/log"
"github.com/newrelic/nri-postgresql/src/args"
performancedbconnection "github.com/newrelic/nri-postgresql/src/connection"
"github.com/newrelic/nri-postgresql/src/query-performance-monitoring/datamodels"
)

func PopulateBlockingMetrics(conn *performancedbconnection.PGSQLConnection, pgIntegration *integration.Integration, args args.ArgumentList, databaseName string, version uint64) error {
isEligible, enableCheckError := validations.CheckBlockingSessionMetricsFetchEligibility(conn, version)
func PopulateBlockingMetrics(conn *performancedbconnection.PGSQLConnection, pgIntegration *integration.Integration, gv *globalvariables.GlobalVariables) error {

Check failure on line 17 in src/query-performance-monitoring/performance-metrics/blocking_sessions.go

View workflow job for this annotation

GitHub Actions / push-pr / static-analysis / Run all static analysis checks

17-38 lines are duplicate of `src/query-performance-monitoring/performance-metrics/wait_event_metrics.go:16-38` (dupl)

Check failure on line 17 in src/query-performance-monitoring/performance-metrics/blocking_sessions.go

View workflow job for this annotation

GitHub Actions / push-pr / static-analysis / Run all static analysis checks

17-38 lines are duplicate of `src/query-performance-monitoring/performance-metrics/wait_event_metrics.go:16-38` (dupl)
isEligible, enableCheckError := validations.CheckBlockingSessionMetricsFetchEligibility(conn, gv.Version)
if enableCheckError != nil {
log.Debug("Error executing query: %v in PopulateBlockingMetrics", enableCheckError)
return commonutils.ErrUnExpectedError
Expand All @@ -23,7 +24,7 @@ func PopulateBlockingMetrics(conn *performancedbconnection.PGSQLConnection, pgIn
log.Debug("Extension 'pg_stat_statements' is not enabled or unsupported version.")
return commonutils.ErrNotEligible
}
blockingQueriesMetricsList, blockQueryFetchErr := GetBlockingMetrics(conn, args, databaseName, version)
blockingQueriesMetricsList, blockQueryFetchErr := GetBlockingMetrics(conn, gv)
if blockQueryFetchErr != nil {
log.Error("Error fetching Blocking queries: %v", blockQueryFetchErr)
return commonutils.ErrUnExpectedError
Expand All @@ -32,18 +33,18 @@ func PopulateBlockingMetrics(conn *performancedbconnection.PGSQLConnection, pgIn
log.Debug("No Blocking queries found.")
return nil
}
commonutils.IngestMetric(blockingQueriesMetricsList, "PostgresBlockingSessions", pgIntegration, args)
commonutils.IngestMetric(blockingQueriesMetricsList, "PostgresBlockingSessions", pgIntegration, gv)
return nil
}

func GetBlockingMetrics(conn *performancedbconnection.PGSQLConnection, args args.ArgumentList, databaseName string, version uint64) ([]interface{}, error) {
func GetBlockingMetrics(conn *performancedbconnection.PGSQLConnection, gv *globalvariables.GlobalVariables) ([]interface{}, error) {
var blockingQueriesMetricsList []interface{}
versionSpecificBlockingQuery, err := commonutils.FetchVersionSpecificBlockingQueries(version)
versionSpecificBlockingQuery, err := commonutils.FetchVersionSpecificBlockingQueries(gv.Version)
if err != nil {
log.Error("Unsupported postgres version: %v", err)
return nil, err
}
var query = fmt.Sprintf(versionSpecificBlockingQuery, databaseName, min(args.QueryCountThreshold, commonutils.MaxQueryThreshold))
var query = fmt.Sprintf(versionSpecificBlockingQuery, gv.DatabaseString, min(gv.QueryCountThreshold, commonutils.MaxQueryThreshold))
rows, err := conn.Queryx(query)
if err != nil {
log.Error("Failed to execute query: %v", err)
Expand All @@ -54,7 +55,7 @@ func GetBlockingMetrics(conn *performancedbconnection.PGSQLConnection, args args
if scanError := rows.StructScan(&blockingQueryMetric); scanError != nil {
return nil, scanError
}
if version == commonutils.PostgresVersion13 || version == commonutils.PostgresVersion12 {
if gv.Version == commonutils.PostgresVersion13 || gv.Version == commonutils.PostgresVersion12 {
*blockingQueryMetric.BlockedQuery = commonutils.AnonymizeQueryText(*blockingQueryMetric.BlockedQuery)
*blockingQueryMetric.BlockingQuery = commonutils.AnonymizeQueryText(*blockingQueryMetric.BlockingQuery)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@ import (
"regexp"
"testing"

commonutils "github.com/newrelic/nri-postgresql/src/query-performance-monitoring/common-utils"
"github.com/newrelic/nri-postgresql/src/query-performance-monitoring/queries"
"gopkg.in/DATA-DOG/go-sqlmock.v1"

"github.com/newrelic/infra-integrations-sdk/v3/integration"
"github.com/newrelic/nri-postgresql/src/args"
"github.com/newrelic/nri-postgresql/src/connection"
commonutils "github.com/newrelic/nri-postgresql/src/query-performance-monitoring/common-utils"
global_variables "github.com/newrelic/nri-postgresql/src/query-performance-monitoring/global-variables"
performancemetrics "github.com/newrelic/nri-postgresql/src/query-performance-monitoring/performance-metrics"
"github.com/newrelic/nri-postgresql/src/query-performance-monitoring/queries"
"github.com/stretchr/testify/assert"
"gopkg.in/DATA-DOG/go-sqlmock.v1"
)

func TestPopulateBlockingMetrics(t *testing.T) {
Expand All @@ -22,6 +22,7 @@ func TestPopulateBlockingMetrics(t *testing.T) {
args := args.ArgumentList{QueryCountThreshold: 10}
databaseName := "testdb"
version := uint64(14)
gv := global_variables.SetGlobalVariables(args, version, databaseName)
validationQueryStatStatements := fmt.Sprintf("SELECT count(*) FROM pg_extension WHERE extname = '%s'", "pg_stat_statements")
mock.ExpectQuery(regexp.QuoteMeta(validationQueryStatStatements)).WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(1))
expectedQuery := queries.BlockingQueriesForV14AndAbove
Expand All @@ -34,7 +35,7 @@ func TestPopulateBlockingMetrics(t *testing.T) {
456, "SELECT 2", 4566, "2023-01-01 00:00:00",
))

err := performancemetrics.PopulateBlockingMetrics(conn, pgIntegration, args, databaseName, version)
err := performancemetrics.PopulateBlockingMetrics(conn, pgIntegration, gv)
assert.NoError(t, err)
assert.NoError(t, mock.ExpectationsWereMet())
}
Expand All @@ -45,6 +46,7 @@ func TestPopulateBlockingMetricsSupportedVersionExtensionNotRequired(t *testing.
args := args.ArgumentList{QueryCountThreshold: 10}
databaseName := "testdb"
version := uint64(12)
gv := global_variables.SetGlobalVariables(args, version, databaseName)
expectedQuery := queries.BlockingQueriesForV12AndV13
query := fmt.Sprintf(expectedQuery, databaseName, min(args.QueryCountThreshold, commonutils.MaxQueryThreshold))
mock.ExpectQuery(regexp.QuoteMeta(query)).WillReturnRows(sqlmock.NewRows([]string{
Expand All @@ -54,7 +56,7 @@ func TestPopulateBlockingMetricsSupportedVersionExtensionNotRequired(t *testing.
"newrelic_value", 123, "SELECT 1", 1233444, "2023-01-01 00:00:00", "testdb",
456, "SELECT 2", 4566, "2023-01-01 00:00:00",
))
err := performancemetrics.PopulateBlockingMetrics(conn, pgIntegration, args, databaseName, version)
err := performancemetrics.PopulateBlockingMetrics(conn, pgIntegration, gv)
assert.NoError(t, err)
assert.NoError(t, mock.ExpectationsWereMet())
}
Expand All @@ -65,7 +67,8 @@ func TestPopulateBlockingMetricsUnSupportedVersion(t *testing.T) {
args := args.ArgumentList{QueryCountThreshold: 10}
databaseName := "testdb"
version := uint64(11)
err := performancemetrics.PopulateBlockingMetrics(conn, pgIntegration, args, databaseName, version)
gv := global_variables.SetGlobalVariables(args, version, databaseName)
err := performancemetrics.PopulateBlockingMetrics(conn, pgIntegration, gv)
assert.EqualError(t, err, commonutils.ErrNotEligible.Error())
assert.NoError(t, mock.ExpectationsWereMet())
}
Expand All @@ -76,9 +79,10 @@ func TestPopulateBlockingMetricsExtensionsNotEnabled(t *testing.T) {
args := args.ArgumentList{QueryCountThreshold: 10}
databaseName := "testdb"
version := uint64(14)
gv := global_variables.SetGlobalVariables(args, version, databaseName)
validationQueryStatStatements := fmt.Sprintf("SELECT count(*) FROM pg_extension WHERE extname = '%s'", "pg_stat_statements")
mock.ExpectQuery(regexp.QuoteMeta(validationQueryStatStatements)).WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(0))
err := performancemetrics.PopulateBlockingMetrics(conn, pgIntegration, args, databaseName, version)
err := performancemetrics.PopulateBlockingMetrics(conn, pgIntegration, gv)
assert.EqualError(t, err, commonutils.ErrNotEligible.Error())
assert.NoError(t, mock.ExpectationsWereMet())
}
Expand All @@ -88,6 +92,8 @@ func TestGetBlockingMetrics(t *testing.T) {
args := args.ArgumentList{QueryCountThreshold: 10}
databaseName := "testdb"
version := uint64(13)
gv := global_variables.SetGlobalVariables(args, version, databaseName)

expectedQuery := queries.BlockingQueriesForV12AndV13
query := fmt.Sprintf(expectedQuery, databaseName, min(args.QueryCountThreshold, commonutils.MaxQueryThreshold))
mock.ExpectQuery(regexp.QuoteMeta(query)).WillReturnRows(sqlmock.NewRows([]string{
Expand All @@ -97,8 +103,7 @@ func TestGetBlockingMetrics(t *testing.T) {
"newrelic_value", 123, "SELECT 1", 1233444, "2023-01-01 00:00:00", "testdb",
456, "SELECT 2", 4566, "2023-01-01 00:00:00",
))
blockingQueriesMetricsList, err := performancemetrics.GetBlockingMetrics(conn, args, databaseName, version)

blockingQueriesMetricsList, err := performancemetrics.GetBlockingMetrics(conn, gv)
assert.NoError(t, err)
assert.Len(t, blockingQueriesMetricsList, 1)
assert.NoError(t, mock.ExpectationsWereMet())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,26 +6,26 @@ import (
"github.com/go-viper/mapstructure/v2"
"github.com/newrelic/infra-integrations-sdk/v3/integration"
"github.com/newrelic/infra-integrations-sdk/v3/log"
"github.com/newrelic/nri-postgresql/src/args"
performancedbconnection "github.com/newrelic/nri-postgresql/src/connection"
commonutils "github.com/newrelic/nri-postgresql/src/query-performance-monitoring/common-utils"
"github.com/newrelic/nri-postgresql/src/query-performance-monitoring/datamodels"
globalvariables "github.com/newrelic/nri-postgresql/src/query-performance-monitoring/global-variables"
)

func PopulateExecutionPlanMetrics(results []datamodels.IndividualQueryMetrics, pgIntegration *integration.Integration, args args.ArgumentList) {
func PopulateExecutionPlanMetrics(results []datamodels.IndividualQueryMetrics, pgIntegration *integration.Integration, gv *globalvariables.GlobalVariables) {
if len(results) == 0 {
log.Debug("No individual queries found.")
return
}
executionDetailsList := GetExecutionPlanMetrics(results, args)
commonutils.IngestMetric(executionDetailsList, "PostgresExecutionPlanMetrics", pgIntegration, args)
executionDetailsList := GetExecutionPlanMetrics(results, gv)
commonutils.IngestMetric(executionDetailsList, "PostgresExecutionPlanMetrics", pgIntegration, gv)
}

func GetExecutionPlanMetrics(results []datamodels.IndividualQueryMetrics, args args.ArgumentList) []interface{} {
func GetExecutionPlanMetrics(results []datamodels.IndividualQueryMetrics, gv *globalvariables.GlobalVariables) []interface{} {
var executionPlanMetricsList []interface{}
var groupIndividualQueriesByDatabase = GroupQueriesByDatabase(results)
for dbName, individualQueriesList := range groupIndividualQueriesByDatabase {
connectionInfo := performancedbconnection.DefaultConnectionInfo(&args)
connectionInfo := performancedbconnection.DefaultConnectionInfo(&gv.Arguments)
dbConn, err := connectionInfo.NewConnection(dbName)
if err != nil {
log.Error("Error opening database connection: %v", err)
Expand Down
Loading

0 comments on commit 8509bcd

Please sign in to comment.