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

Boost log to file #172

Merged
merged 23 commits into from
Jul 28, 2020
Merged

Conversation

stephengtuggy
Copy link
Contributor

@stephengtuggy stephengtuggy commented Jul 9, 2020

Thank you for submitting a pull request and becoming a contributor to the Vega Strike Core Engine.

Please answer the following:

Code Changes:

Issues:

Purpose:

  • What is this pull request trying to do? Have Boost logging output go to a file; convert more stdout, stderr, and logging messages to go through Boost.Log; log only fatal errors to the console. (I was going to try to finish converting the entire project, but that appears impractical to do within this PR.)
  • What release is this for? 0.7.0?
  • Is there a project or milestone we should apply this to? 0.7.x?

@stephengtuggy stephengtuggy self-assigned this Jul 9, 2020
}
}

fflush(stderr);
// TODO: Find a way to flush the Boost logs? Console log specifically?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

part of the boost log configuration IIRC

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BenjamenMeyer yes, you can set the log sinks to auto-flush after every line of output. I have done that with the console logger, but my thinking is that that will be too much overhead with the file logger; it will slow down the game too much. So we will need a way to manually flush the file log sink when needed.

Luckily, I now have a way to do that:

VSFileSystem::pFileLogSink->flush();

I just haven't updated this file yet to use it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review, by the way.

@stephengtuggy stephengtuggy linked an issue Jul 27, 2020 that may be closed by this pull request
@stephengtuggy stephengtuggy marked this pull request as ready for review July 27, 2020 03:47
@stephengtuggy
Copy link
Contributor Author

OK, I think this one is ready for review. I was going to try to finish converting the entire project. But other people are working on the same files, of course, and so much is shifting around that that goal seems pretty impractical at this point.

I did get the Boost logging to go to a file by default -- once it has reached the stage of game startup where it is able to. And I believe I converted quite a few of the most common and annoying messages, so they won't show up on the console anymore.

What do you guys think of the log location I chose? $HOME/.vegastrike/logs/. Since this isn't a system program, that seemed like a good location. But maybe I'm wrong.

The file logs include a timestamp in the filename. They rotate at 10 MiB, or at midnight. Logging will stop if free space on the volume drops to 1 GiB.

Any other questions?

@nabaco nabaco changed the title DRAFT: Boost log to file Boost log to file Jul 27, 2020
Copy link
Member

@nabaco nabaco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @stephengtuggy, good job. I think we can merge it, and add some documentation on the week how to use this infrastructure (maybe as art of our coding style) so future developers will know how to use it.

@evertvorster
Copy link
Contributor

@stephengtuggy Thanks for the hard work that you put in.
If you don't mind, I'll also use the .vegastrike/logs directory to keep console logs of the stuff that is not in boost. Currently I am using files in .vegastrike. As you mentioned, this is not system software, so /var/log seems a bit far away.

I am a little concerned about the size of the logs. If you have a rather large /home, this would mean that the files will never be deleted. Is this something you turn on, or is the boost logging on by default?

For the console logs, a backup of the last log is made, and then the new log is written in it's place. This gives us a log rotation of 2, keeping the size of the thing under control. How far back in history do you want to go? It may be worth considering a log rotation scheme of 10, and compressing the backups.

@stephengtuggy
Copy link
Contributor Author

Sorry, one more commit is on the way. Need to make sure to flush the logs before exit, wherever possible.

@stephengtuggy
Copy link
Contributor Author

@stephengtuggy Thanks for the hard work that you put in.

You're welcome. :-)

If you don't mind, I'll also use the .vegastrike/logs directory to keep console logs of the stuff that is not in boost. Currently I am using files in .vegastrike. As you mentioned, this is not system software, so /var/log seems a bit far away.

Sure, that's fine.

I am a little concerned about the size of the logs. If you have a rather large /home, this would mean that the files will never be deleted. Is this something you turn on, or is the boost logging on by default?

The boost logging is on by default, but by default, it only logs warning, error, and fatal messages. This means that during some runs of the game, no log file will be created at all. In others, only one or two messages will be logged.

For the console logs, a backup of the last log is made, and then the new log is written in it's place. This gives us a log rotation of 2, keeping the size of the thing under control. How far back in history do you want to go? It may be worth considering a log rotation scheme of 10, and compressing the backups.

That is worth considering. However, I don't know how to do that with Boost::Log. I'm not actually sure if their system supports that without some customization. Maybe a future PR can deal with that.

@stephengtuggy
Copy link
Contributor Author

There. NOW it's ready.

@stephengtuggy
Copy link
Contributor Author

Hi @stephengtuggy, good job. I think we can merge it, and add some documentation on the week how to use this infrastructure (maybe as art of our coding style) so future developers will know how to use it.

Thanks.

Usage instructions remain the same as before. There is at least a preliminary usage guide on the wiki here.

Copy link
Member

@BenjamenMeyer BenjamenMeyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can merge once a play test has been confirmed

@BenjamenMeyer BenjamenMeyer added this to the 0.7.x milestone Jul 28, 2020
@stephengtuggy
Copy link
Contributor Author

we can merge once a play test has been confirmed

I have play tested it. Do you mean you want someone else to as well?

@BenjamenMeyer BenjamenMeyer merged commit 3555faa into vegastrike:master Jul 28, 2020
@BenjamenMeyer
Copy link
Member

@stephengtuggy it would be nice if we could have a secondary person do play tests...but that's a lot of extra work so as long as we have someone willing to take ownership of the play test and affirm it I'm good.

@evertvorster
Copy link
Contributor

I'll be playing it now, just rebuilding my game engine. :)

@stephengtuggy stephengtuggy deleted the boost_log_to_file branch July 28, 2020 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proposal: Use boost::logs to capture log data
4 participants