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 log timestamp #455

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

gurki
Copy link

@gurki gurki commented Jun 14, 2021

Adds iso timestamps in front of relevant log messages to allow for more fine-grained benchmarking and monitoring.

@madMAx43v3r
Copy link
Owner

there's a lot of whitespace changes unfortunately...

@gurki
Copy link
Author

gurki commented Jun 15, 2021

Ah, my bad, VSCode auto-remove trailing whitespace on save... I'll revert the spaces.

@gurki gurki force-pushed the add-log-timestamp branch 5 times, most recently from bbfa6bc to 5929588 Compare June 15, 2021 11:05
@gurki gurki force-pushed the add-log-timestamp branch from 5929588 to 5753b1c Compare June 15, 2021 11:06
@@ -375,6 +375,16 @@ namespace Util {
}
}

inline
std::string get_curr_datetime()
Copy link
Owner

Choose a reason for hiding this comment

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

there's already a function for that, see get_date_string_ex()

Copy link
Owner

Choose a reason for hiding this comment

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

you can call that function inside this one

@ruant
Copy link

ruant commented Jun 16, 2021

Maybe add a file option to this PR at the same time?
A helper method that writes the logline to cout and to a file (defined from arguments, maybe a paramter for rolling log files too?)

It's fine and all with using | or >, >> but that breaks SIGINT options with ctrl+c. so you have to use kill -SIGINT instead.

@madMAx43v3r
Copy link
Owner

yeah I would prefer the timestamps to be in a log file, not on the terminal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants