-
Notifications
You must be signed in to change notification settings - Fork 0
Separate collection interval for Oracle top_query collection #4
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
| topNCollectionErrors := s.collectTopNMetricData(ctx, logs, currentCollectionTime, lookbackTimeCounter) | ||
| if topNCollectionErrors != nil { | ||
| scrapeErrors = append(scrapeErrors, topNCollectionErrors) | ||
| } |
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.
Could we move s.lastExecutionTimestamp = collectionTime here? so the early returned collectTopNMetricData can still update the last execution time.
Currently, if the hits is zero, then the last execution time won't get updated.
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.
Thats a good point. I had multiple thoughts about this too.
I did it this way to make the first collection happen as early as possible, otherwise it would happen only at the end of first 60 seconds. But it's nothing huge. Let me check if there could be some work around.
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.
Hi @XSAM updated as you suggested!
receiver/oracledbreceiver/scraper.go
Outdated
| return int(s.topQueryCollectCfg.CollectionInterval.Seconds()) | ||
| } | ||
|
|
||
| const vsqlRefreshLagSec = 10 * time.Second // Buffer to account for v$sql maximum refresh latency |
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.
Could you explain or update the comment for the buffer design? Why do we need to go back further for 10 seconds.
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.
Done @XSAM
XSAM
left a comment
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.
Looks good!
DBMon - Internal PR
This PR is to introduce the capability in Oracle receiver to have separate collection interval for top_query.
This feature is already existing in sql server.
Only difference in Oracle's implementation is that the
lookbackTimefor each collection is calculated dynamically, and the user cannot provide a value through config. SQL server receiver has an option for this to be configured manually.A
vsqlRefreshLagSecof 10 seconds is added to the lookbackTime to offset the refresh lag in v$sql table( https://docs.oracle.com/en/database/oracle/oracle-database/21/refrn/V-SQL.html#:~:text=they%20are%20updated%20every%205%20seconds )