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.

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

Add support for automatic removal of old logs #432

Merged
merged 14 commits into from
Nov 1, 2019

Conversation

aesophor
Copy link
Contributor

@aesophor aesophor commented Feb 17, 2019

This PR is in regard to #423. The code has been added and tested , but not yet integrated into glog.

Example usage:

vector<string> overdue_log_names = GetOverdueLogNames("/tmp", 1);

for (const auto& name : overdue_log_names) {
  unlink(name.c_str());
}

GetOverdueLogNames() will check all filenames under log_directory, and return a list of files whose last modified time is over the given days ( calculated with difftime() ), so that we can simply call unlink() on the filenames in the returned vector.

However, I'm not sure when is the right time to call it.

GetOverdueLogNames(string log_directory, int days) will check all
filenames under log_directory, and return a list of files whose last modified time is
over the given days (calculated using difftime()).

So that we can easily for unlink all files stored in the returned vector.
@googlebot
Copy link
Collaborator

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

Googlers can find more info about SignCLA and this PR by following this link.

@aesophor
Copy link
Contributor Author

I signed it!

@googlebot
Copy link
Collaborator

CLAs look good, thanks!

Googlers can find more info about SignCLA and this PR by following this link.

@Steven0917
Copy link

Guess the patch was implemented based on Linux. For Windows user, https://github.com/tronkko/dirent will be needed.

In this commit, at the end of LogFileObject::Write,
it will perform clean up for old logs.

It uses GetLoggingDirectories() and for each file in each
directory, it will check if a file is a log file produced by glog.
If it is, and it is last modified 3 days ago, then it will unlink()
this file. (It will only remove the project's own log files,
it won't remove the logs from other projects.)

Currently it is hardcoded to 3 days, I'll see if this can be
implemented in a more flexible manner.
The log cleaner can be enabled and disabled at any given time.
By default, the log cleaner is disabled.

For example, this will enable the log cleaner and delete
the log files whose last modified time is >= x days
google::EnableLogCleaner(x days);

To disable it, simply call
google::DisableLogCleaner();

Please note that it will only clean up the logs produced for
its own project, the log files from other project will be untouched.
Also replaced the hardcoded overdue days with the correct variable.
src/logging.cc Outdated Show resolved Hide resolved
@aesophor
Copy link
Contributor Author

aesophor commented Aug 21, 2019

Guess the patch was implemented based on Linux. For Windows user, https://github.com/tronkko/dirent will be needed.

Thank you very much for your help! CI has passed on windows!
This is what I've done currently:

To enable log cleaner, in your project (log cleaner is disabled by default).

google::EnableLogCleaner(3);

And then your project will check if there are overdue logs whenever a flush is performed. In this example, the logs whose last modified times are >= 3 will be unlink(). It calls GetLoggingDirectories() to look for overdue logs.

You may also disable it at anytime.

google::DisableLogCleaner();

I try to check if a file is a glog file produced by the project it is linked in carefully. It will only remove a log if its filename matches: <program name>.<hostname>.<user name>.log.<severity level>, so logs from other projects should be untouched.

return log_name_tokens[0] == glog_internal_namespace_::ProgramInvocationShortName()

Edit:
This has been tested on Linux and it works well on my machine.
I'll see if I can test it on Windows, or if anyone is willing to help me I'll be very grateful.

src/logging.cc Outdated Show resolved Hide resolved
Previously the full path to a file is passed into IsGlogLog(),
and then std::string::erase() is used to get the filename part.
If a directory name contains '.', then this function will be unreliable.

Now only the filename it self is passed into IsGlogLog(),
so this problem will be eradicated.
src/logging.cc Outdated Show resolved Hide resolved
Splitting a filename into tokens by '.' causes problems
if the executable's filename contains a dot.

Filename should be matched keyword by keyword in the following
order:
1. program name
2. hostname
3. username
4. "log"
@aesophor
Copy link
Contributor Author

@sergiud Sorry for asking but any chance this patch could be merged?

I've tested them on linux (4.19.66-gentoo) and windows 10, old log files can be removed automatically. This feature is disabled by default.

@sergiud sergiud merged commit a6f7be1 into google:master Nov 1, 2019
@aesophor aesophor deleted the RemoveOldLogs branch November 4, 2019 10:32
@cswuyg
Copy link

cswuyg commented Dec 1, 2019

"It will only remove a log if its filename matches: ...log., so logs from other projects should be untouched."

@aesophor If I use SetLogDestination API to change base_filename_, old logs will not delete automatically. is a bug?

@aesophor
Copy link
Contributor Author

aesophor commented Dec 1, 2019

"It will only remove a log if its filename matches: ...log., so logs from other projects should be untouched."

@aesophor If I use SetLogDestination API to change base_filename_, old logs will not delete automatically. is a bug?

@cswuyg Yes, this is a bug. I'm working on a patch for this.

@aesophor
Copy link
Contributor Author

aesophor commented Dec 1, 2019

@cswuyg Fixed in #502

@sergiud sergiud added this to the 0.5 milestone Mar 30, 2021
@sergiud sergiud mentioned this pull request May 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants