Add log de-duplication support to glog #3
Conversation
glog.go
Outdated
| // message. Method will return true if we do, false otherwise. Method is also | ||
| // responsible for noticing a different logging statement is being called and | ||
| // adding any relevant de-duplication log information about our previously | ||
| // de-dupled log statements. As such IT MUST BE CALLED AS PART OF EVERY LOG |
There was a problem hiding this comment.
| // de-dupled log statements. As such IT MUST BE CALLED AS PART OF EVERY LOG | |
| // de-duped log statements. As such IT MUST BE CALLED AS PART OF EVERY LOG |
| // function performant. Thus we must not use a defer unlock here. | ||
| l.lastStatementMu.Lock() | ||
|
|
||
| if l.lastStatementCaller.pc == c.pc { |
There was a problem hiding this comment.
When do we reset the lastStatementCount? If the program counter doesn't change, we will never log the line again? I was sort of the under the impression we would want to reset this after a timeout
There was a problem hiding this comment.
I'm also not positive I understand the structs/fields you have here. There is only a single lastStatementCount field in loggingT, which gets overwritten every time we change program counter. I'd expect there to be a something like a map of program counter to this state. so like
type loggingT struct {
...
logCounts map[uintptr]uint // maps program counter to log count
}It seems like the way this works, the deduping will only happen if we hit the same log line (determined by program counter) sequentially. If there is another log line interspersed with the spammy log, no deduping will happen.
There was a problem hiding this comment.
When do we reset the lastStatementCount?
See line 617 for where that assignment happens (we assign a new caller which includes the pc value).
It seems like the way this works, the deduping will only happen if we hit the same log line (determined by program counter) sequentially
This is precisely how it works. It will de-dupe multiple consecutive log statements. So this helps when one single message is spamming the log file. To do more is a much more complicated lift as you have to define when a log is too noisy and track independent behaviors for each log line (which can get hairy as you might imagine). This in its current form is simple and fast and effective without being too big of a change to an otherwise critical system.
glog.go
Outdated
| // Unsure if needed, but since this was in code previously I am carrying it | ||
| // forward here to avoid any surprise regressions. |
There was a problem hiding this comment.
I would include the original comment that was on the old line
// not a real line number, but acceptable to someDigitsAdds deduplication support to glog so that we can eliminate noisy log sources in code. Will only deduplicate consecutive lines that are noisy.
Adds de-duplication support to glog so that we can eliminate noisy log sources in code. Will only de-duplicate consecutive lines that are noisy.
Includes a flag
logdedupeafterthat will begin de-duplicating after N consecutive log statements. Defaults to 0 (disabled).Test output to demonstrate operation: