Skip to content
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.

Sign up for GitHub

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

Jump to bottom

PR Comment changes - Query analysis ohi (#52) - feat: Enhanced query performance monitoring features to track slow queries, examine query execution plans, monitor wait events, and identify blocking sessions #199

Draft
wants to merge 5 commits into
base: epic_db_query_performance_monitoring
Choose a base branch
from
Draft

PR Comment changes - Query analysis ohi (#52) - feat: Enhanced query performance monitoring features to track slow queries, examine query execution plans, monitor wait events, and identify blocking sessions #199

wants to merge 5 commits into from
+3,897 −240

Conversation

abhinav1602
Copy link

@abhinav1602 abhinav1602 commented Jan 15, 2025 •
edited
Loading

feature[DNM]: Query Performance monitoring for NRI-MSSQL providing Enhanced query performance monitoring features to track the following new event Types:
SlowQueryAnalysis
WaitTimeAnalysis
ExecutionPlanAnalysis
BlockedSessionAnalysis

Changes for PR Review:

  • Added SQL Server Version Validation
  • Removed retry mechanisnm
  • Refactor flag EnableQueryMonitoring
  • Update mssql-config.yml.sample file and QueryResponseTimeThreshold

In Progress:

  • Integration tests
  • Unit test cases

Co-authored-by: Anitha Vadla [email protected]
Co-authored-by: Tanush Angural [email protected]

@abhinav1602 @avadla @tanushangural
PR Comment changes - Query analysis ohi (#52)
cac17a6 
feature[DNM]: Query Performance monitoring for NRI-MSSQL providing Enhanced query performance monitoring features to track the following new event Types:

SlowQueryAnalysis
WaitTimeAnalysis
ExecutionPlanAnalysis
BlockedSessionAnalysis

In Progress:
- Integration tests
- Unit test cases
---------
Co-authored-by: Anitha Vadla <[email protected]>
Co-authored-by: tangural <[email protected]>
@abhinav1602 abhinav1602 changed the title PR Comment changes - Query analysis ohi (#52) PR Comment changes - Query analysis ohi (#52) - feat: Enhanced query performance monitoring features to track slow queries, examine query execution plans, monitor wait events, and identify blocking sessions Jan 15, 2025
@rahulreddy15
Copy link

Reopening because merging PR #198 is no longer possible.

@rahulreddy15 rahulreddy15 reopened this Jan 15, 2025
@abhinav1602
Copy link
Author

abhinav1602 commented Jan 15, 2025 •
edited
Loading

Going forward, we'll be using this PR for all the new changes.
We can track the previous review comments and history using this PR link:
#198
All the existing changes and the PR comment(from previous PR) related changes are present in this PR already.

Why this new PR?
Due to a merge issue resulting in a lot of commit in older PR resulting in a tricky situation, we had to get rid of the old branch and create this new one to keep the commit history clean.

sairaj18
sairaj18 reviewed Jan 17, 2025
View reviewed changes
src/args/argument_list.go Outdated Show resolved Hide resolved
@sairaj18
Copy link
Contributor

I think you missed adding the new flags introduced to mssql-config.yml.sample file. Also, including one line helper comment explaining them and example values for them would help whenever someone refer's in future

  • ENABLE_QUERY_PERFORMANCE: true
  • SLOW_QUERY_FETCH_INTERVAL: 15
  • QUERY_RESPONSE_TIME_THRESHOLD: 500
  • QUERY_COUNT_THRESHOLD: 20

sairaj18
sairaj18 reviewed Jan 20, 2025
View reviewed changes
src/queryanalysis/go.sum Outdated Show resolved Hide resolved
@abhinav1602
Query analysis ohi (#59)
6b1158c 
**Changes for PR Review:**
* Added SQL Server Version Validation (#55)
* Removed retry mechanisnm (#56)
* Refactor flag EnableQueryMonitoring (#58)
* Update mssql-config.yml.sample file and QueryResponseTimeThreshold
@abhinav1602
Copy link
Author

I think you missed adding the new flags introduced to mssql-config.yml.sample file. Also, including one line helper comment explaining them and example values for them would help whenever someone refer's in future

  • ENABLE_QUERY_PERFORMANCE: true
  • SLOW_QUERY_FETCH_INTERVAL: 15
  • QUERY_RESPONSE_TIME_THRESHOLD: 500
  • QUERY_COUNT_THRESHOLD: 20

Covered in @avadla's latest commit

abhinav1602 and others added 2 commits January 22, 2025 12:04
@abhinav1602
Query analysis ohi (#63)
02956b5 
PR review comment changes: Add logs and refractor
@rahulreddy15
Integration Tests Setup for MSSQL (#57)
71705dd 
* Integration Tests Setup for MSSQL

* Supported versions only

* Use assert for validating found sample types
rahulreddy15
rahulreddy15 reviewed Jan 22, 2025
View reviewed changes
src/queryanalysis/retrymechanism/retry_mechanism_impl.go Outdated Show resolved Hide resolved
src/queryanalysis/retrymechanism/retry_mechanism.go Outdated Show resolved Hide resolved
src/queryanalysis/retrymechanism/retry_mechanism_impl_test.go Outdated Show resolved Hide resolved
src/queryanalysis/validation/permissions.go Outdated Show resolved Hide resolved
src/queryanalysis/validation/permissions_test.go Outdated Show resolved Hide resolved
src/queryanalysis/validation/sql_server_version.go Outdated Show resolved Hide resolved
src/queryanalysis/validation/validation_check.go Show resolved Hide resolved
src/queryanalysis/utils/utils_test.go Show resolved Hide resolved
src/queryanalysis/validation/sql_server_login.go Outdated Show resolved Hide resolved
src/queryanalysis/query_analysis.go Outdated Show resolved Hide resolved
@abhinav1602
PR comment changes - refractor (#67)
1881c8a 
* Database threadpool configs (#62)
* Close DB connection after use
* Update QueryResponseTimeThreshold value and add comment BatchSize (#64)
* Delete retrymachism (#65)
* Refactor queries as constants (#66)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@avadla avadla avadla left review comments

@tanushangural tanushangural tanushangural left review comments

@sairaj18 sairaj18 sairaj18 left review comments

@rahulreddy15 rahulreddy15 rahulreddy15 left review comments

@paologallinaharbur paologallinaharbur Awaiting requested review from paologallinaharbur

Assignees
No one assigned
Labels
do-not-merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@abhinav1602 @rahulreddy15 @sairaj18 @tanushangural @avadla