Skip to content

Skip repeated log entries #937

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

Merged
merged 1 commit into from
Jan 28, 2021

Conversation

daleglass
Copy link
Contributor

This helps with log flooding. Successive repeated log messages will be skipped and counted, the count will be output when a different message is logged.

A new option of 'keep_repeats' has been added to VIRCADIA_LOG_OPTIONS to disable this.

Fixes #934

@daleglass daleglass added enhancement New feature or request needs CR (code review) needs testing (QA) The PR is ready for testing labels Dec 27, 2020
@digisomni digisomni added this to the 2021.1.0 Release milestone Dec 28, 2020
@daleglass daleglass force-pushed the skip_repeated_log_entries branch from 6de94c4 to 189b4ba Compare January 5, 2021 00:10
@ctrlaltdavid ctrlaltdavid added CR Approved At least one code reviewer has approved the PR. rebuild rebuild through the GithubActions and removed needs CR (code review) labels Jan 5, 2021
@ctrlaltdavid ctrlaltdavid added needs CR (code review) and removed CR Approved At least one code reviewer has approved the PR. labels Jan 8, 2021
This helps with log flooding. Successive repeated log messages will be skipped and counted,
the count will be output when a different message is logged.

A new option of 'keep_repeats' has been added to VIRCADIA_LOG_OPTIONS to disable this.
@daleglass daleglass force-pushed the skip_repeated_log_entries branch from 189b4ba to aab5b22 Compare January 9, 2021 18:40
@daleglass daleglass added rebuild rebuild through the GithubActions and removed rebuild rebuild through the GithubActions labels Jan 9, 2021
@digisomni digisomni added rebuild rebuild through the GithubActions and removed rebuild rebuild through the GithubActions labels Jan 14, 2021
@ctrlaltdavid ctrlaltdavid added rebuild rebuild through the GithubActions and removed rebuild rebuild through the GithubActions labels Jan 15, 2021
@ctrlaltdavid ctrlaltdavid removed needs CR (code review) rebuild rebuild through the GithubActions labels Jan 15, 2021
@ctrlaltdavid ctrlaltdavid added the CR Approved At least one code reviewer has approved the PR. label Jan 15, 2021
@digisomni
Copy link
Member

Is there a specific type of log message that I can trigger in order to test this? I used it for a few minutes but didn't really notice anything different even when I did see what looked like "repeated" log entries.

@SilverfishVR
Copy link
Contributor

#474 was implemented specifically to address missing joint log spam, that would be redundant now right?

@daleglass
Copy link
Contributor Author

@digisomni Use Developer/Audio/Stats on Linux. If you don't have a Verdana font, you'll get around 150 messages per second

@SilverfishVR Possibly, not. This PR only happens the case when a message is repeated right after itself. From what I remember from looking at the other mechanism it's subsystem specific and may work better for that case because for instance you can avoid repeats within a subsystem even if something else logs stuff in between.

@digisomni
Copy link
Member

In LogHandler.cpp on line 142 it appears there is functionality for this sort of thing already. Can this be updated to work with/improve that functionality? I found this because I noticed my log was blocking repeated messages from another part of the codebase already.

@digisomni
Copy link
Member

Tested on Mac, there were no font issues so I could not reproduce and trigger this.

@digisomni digisomni added QA Approved The PR has been tested successfully. and removed needs testing (QA) The PR is ready for testing labels Jan 23, 2021
@digisomni digisomni merged commit af8ce1b into vircadia:master Jan 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CR Approved At least one code reviewer has approved the PR. enhancement New feature or request QA Approved The PR has been tested successfully.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Font warning flood
5 participants