-
Notifications
You must be signed in to change notification settings - Fork 3.2k
[WIP] Improve SQLServer metrics accuracy by reducing the warm-up delay and including all available cumulative records from DB #45227
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
base: main
Are you sure you want to change the base?
Conversation
| // We just send down the metrics corresponding to that single execution as it is. | ||
| // If execution count in DB is not 1 then we send down the delta values. | ||
| execCountInDB := executionCountVal.(int64) | ||
| if execCountInDB != 1 { |
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.
I don't understand why treating 1 as a special case. If a query only runs for once, will this change report this query each time the collector restarts?
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.
@XSAM I have details in the PR description, please check and let me know if you need more info.
…essing a negligable edge case.
Description
SQLServer metrics reporting is improved by reducing the warm-up delay and providing accurate insights sooner.
In the existing implementation, only the
totalElapsedTimemetric is cached during the first collection, while all other metrics are skipped and cached only in the second collection. As a result, delta calculations are not possible at that point and can only be performed starting from the third collection.This PR fixes the issue by caching all metrics starting from the first collection onwards.
Also to get the total of all cumulative metrics records from sys.dm_exec_query_stats, we should move the time period filter from inner query to outer. With that being within the inner query, we were considering only records created within the collection window, resulting in calculation of a wrong total of cumulative records.
Link to tracking issue
Fixes #45228
Testing
Unit tests added. Also ran in internal environment for extended testing.