-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat performance monitoring #189
base: epic_db_query_performance_monitoring
Are you sure you want to change the base?
Changes from 33 commits
53151cc
14cfcec
cf04038
38c4664
bf8ccbb
ab19603
cd21509
bc7a3f6
3238d47
fbd5eb8
5a73a0b
d648911
18ed92c
00e4097
852f29d
a3cd73e
4b2868a
afbfa63
4da1554
0f765f2
cfce344
3711a86
e7dff9d
5f31038
2e87ba7
e89cbcb
08d9aa8
4ed8799
2bf940a
3461f3a
2b972fe
4a8d893
b6b2b3a
0739685
7599cb9
ddc9212
f974650
006c159
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
FROM golang:1.23.4-bookworm | ||
FROM golang:1.23.5-bookworm | ||
|
||
ARG GH_VERSION='1.9.2' | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -50,6 +50,15 @@ integrations: | |
# True if SSL is to be used. Defaults to false. | ||
ENABLE_SSL: "false" | ||
|
||
# Enable query performance monitoring - Defaults to false | ||
# ENABLE_QUERY_MONITORING : "false" | ||
|
||
# Threshold in milliseconds for query response time to fetch individual query performance metrics - Defaults to 500 | ||
# QUERY_RESPONSE_TIME_THRESHOLD : "500" | ||
|
||
# The number of records for each query performance metrics (maximum slow query metrics can be 20 and execution plan metrics can be 30) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. number of records to be retrieved? |
||
# QUERY_COUNT_THRESHOLD : "10" | ||
|
||
# True if the SSL certificate should be trusted without validating. | ||
# Setting this to true may open up the monitoring service to MITM attacks. | ||
# Defaults to false. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,6 +31,9 @@ type ArgumentList struct { | |
CollectDbLockMetrics bool `default:"false" help:"If true, enables collection of lock metrics for the specified database. (Note: requires that the 'tablefunc' extension is installed)"` //nolint: stylecheck | ||
CollectBloatMetrics bool `default:"true" help:"Enable collecting bloat metrics which can be performance intensive"` | ||
ShowVersion bool `default:"false" help:"Print build information and exit"` | ||
EnableQueryMonitoring bool `default:"false" help:"Enable collection of detailed query performance metrics."` | ||
QueryResponseTimeThreshold int `default:"500" help:"Threshold in milliseconds for query response time. If response time exceeds this threshold, the query will be considered slow."` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This argument is being used only while fetching individual query metrics. So instead of |
||
QueryCountThreshold int `default:"20" help:"Maximum number of queries returned in query analysis results."` | ||
} | ||
|
||
// Validate validates PostgreSQl arguments | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,8 @@ import ( | |
"runtime" | ||
"strings" | ||
|
||
queryperformancemonitoring "github.com/newrelic/nri-postgresql/src/query-performance-monitoring" | ||
tharun0064 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
"github.com/newrelic/infra-integrations-sdk/v3/integration" | ||
"github.com/newrelic/infra-integrations-sdk/v3/log" | ||
"github.com/newrelic/nri-postgresql/src/args" | ||
|
@@ -27,6 +29,7 @@ var ( | |
) | ||
|
||
func main() { | ||
|
||
var args args.ArgumentList | ||
// Create Integration | ||
pgIntegration, err := integration.New(integrationName, integrationVersion, integration.Args(&args)) | ||
|
@@ -62,7 +65,6 @@ func main() { | |
log.Error("Error creating list of entities to collect: %s", err) | ||
os.Exit(1) | ||
} | ||
|
||
instance, err := pgIntegration.Entity(fmt.Sprintf("%s:%s", args.Hostname, args.Port), "pg-instance") | ||
if err != nil { | ||
log.Error("Error creating instance entity: %s", err.Error()) | ||
|
@@ -89,4 +91,9 @@ func main() { | |
if err = pgIntegration.Publish(); err != nil { | ||
log.Error(err.Error()) | ||
} | ||
|
||
if args.EnableQueryMonitoring && args.HasMetrics() { | ||
queryperformancemonitoring.QueryPerformanceMain(args, pgIntegration, collectionList) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we usually avoid passing around "args" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we have to pass them as we need to have db specific connection in execution plan metrics There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Form yesterday's discussion with paolo, we can build the connection and pass it as an argument instead of passing |
||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
package commonutils | ||
|
||
import ( | ||
"crypto/rand" | ||
"fmt" | ||
"math/big" | ||
"regexp" | ||
"strings" | ||
"time" | ||
|
||
"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 GetDatabaseListInString(dbMap collection.DatabaseList) string { | ||
if len(dbMap) == 0 { | ||
return "" | ||
} | ||
var quotedNames = make([]string, 0) | ||
for dbName := range dbMap { | ||
quotedNames = append(quotedNames, fmt.Sprintf("'%s'", dbName)) | ||
} | ||
return strings.Join(quotedNames, ",") | ||
} | ||
|
||
func AnonymizeQueryText(query string) string { | ||
anonymizedQuery := re.ReplaceAllString(query, "?") | ||
return anonymizedQuery | ||
} | ||
|
||
// This function is used to generate a unique plan ID for a query | ||
func GeneratePlanID(queryID string) *string { | ||
tharun0064 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
randomInt, err := rand.Int(rand.Reader, big.NewInt(RandomIntRange)) | ||
if err != nil { | ||
return nil | ||
} | ||
currentTime := time.Now().Format(TimeFormat) | ||
result := fmt.Sprintf("%s-%d-%s", queryID, randomInt.Int64(), currentTime) | ||
return &result | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
package commonutils_test | ||
|
||
import ( | ||
"sort" | ||
"testing" | ||
"time" | ||
|
||
"github.com/newrelic/nri-postgresql/src/collection" | ||
commonutils "github.com/newrelic/nri-postgresql/src/query-performance-monitoring/common-utils" | ||
"github.com/stretchr/testify/assert" | ||
) | ||
|
||
func TestGetDatabaseListInString(t *testing.T) { | ||
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'" | ||
result := commonutils.GetDatabaseListInString(dbList) | ||
assert.Equal(t, expected, result) | ||
|
||
// Test with empty database list | ||
dbList = collection.DatabaseList{} | ||
expected = "" | ||
result = commonutils.GetDatabaseListInString(dbList) | ||
assert.Equal(t, expected, result) | ||
} | ||
|
||
func TestAnonymizeQueryText(t *testing.T) { | ||
tharun0064 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
query := "SELECT * FROM users WHERE id = 1 AND name = 'John'" | ||
expected := "SELECT * FROM users WHERE id = ? AND name = ?" | ||
result := commonutils.AnonymizeQueryText(query) | ||
assert.Equal(t, expected, result) | ||
query = "SELECT * FROM employees WHERE id = 10 OR name <> 'John Doe' OR name != 'John Doe' OR age < 30 OR age <= 30 OR salary > 50000OR salary >= 50000 OR department LIKE 'Sales%' OR department ILIKE 'sales%'OR join_date BETWEEN '2023-01-01' AND '2023-12-31' OR department IN ('HR', 'Engineering', 'Marketing') OR department IS NOT NULL OR department IS NULL;" | ||
expected = "SELECT * FROM employees WHERE id = ? OR name <> ? OR name != ? OR age < ? OR age <= ? OR salary > ?OR salary >= ? OR department LIKE ? OR department ILIKE ?OR join_date BETWEEN ? AND ? OR department IN (?, ?, ?) OR department IS NOT NULL OR department IS NULL;" | ||
result = commonutils.AnonymizeQueryText(query) | ||
assert.Equal(t, expected, result) | ||
} | ||
|
||
func TestGeneratePlanID(t *testing.T) { | ||
queryID := "query123" | ||
result := commonutils.GeneratePlanID(queryID) | ||
assert.NotNil(t, result) | ||
assert.Contains(t, *result, queryID) | ||
assert.Contains(t, *result, "-") | ||
assert.Contains(t, *result, time.Now().Format(commonutils.TimeFormat)[:8]) // Check date part | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
package commonutils | ||
|
||
import "errors" | ||
|
||
// The maximum number records that can be fetched in a single metrics | ||
const MaxQueryCountThreshold = 30 | ||
|
||
// The maximum number of individual queries that can be fetched in a single metrics | ||
const MaxIndividualQueryCountThreshold = 10 | ||
|
||
// The maximum number of metrics to be published in a single batch | ||
const PublishThreshold = 100 | ||
const RandomIntRange = 1000000 | ||
const TimeFormat = "20060102150405" | ||
|
||
var ErrUnsupportedVersion = errors.New("unsupported PostgreSQL version") | ||
var ErrUnExpectedError = errors.New("unexpected error") | ||
|
||
var ErrInvalidModelType = errors.New("invalid model type") | ||
var ErrNotEligible = errors.New("not Eligible to fetch metrics") | ||
|
||
const PostgresVersion12 = 12 | ||
const PostgresVersion11 = 11 | ||
const PostgresVersion13 = 13 | ||
const PostgresVersion14 = 14 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
before this sleep was not needed, why have we added that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rahulreddy15 please check this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is because before we were running docker compose to bring up the containers within the code.
Now, we are doing it as a command in the Makefile. Also, the postgres containers are being created with test data to simulate all the different kind of queries.
They need a couple moments to start up.