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

LibRS API to enable rolling log files #8207

Closed

Conversation

aseelegbaria
Copy link
Contributor

Log files can have a max size. upon reaching max size new file will be created and current file will be truncated.
Tracked on [RS5-9271]

@@ -36,6 +36,13 @@ namespace rs2
rs2_reset_logger(&e);
error::handle(e);
}

inline void enable_rolling_files(std::size_t max_size )
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not clear what the unit of measure is for max_size -- is it bytes? KB?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -82,6 +82,9 @@ void rs2_log_to_callback( rs2_log_severity min_severity, rs2_log_callback_ptr ca

void rs2_reset_logger( rs2_error ** error);

void rs2_enable_rolling_files(size_t max_size, rs2_error ** error);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add a /** ... */ description

Copy link
Collaborator

Choose a reason for hiding this comment

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

rs2_enable_rolling_log_file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


void librealsense::enable_rolling_files(std::size_t max_size )
{
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it needs to throw

Copy link
Contributor Author

Choose a reason for hiding this comment

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

why? if so, shouldn't all the other functions also throw?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

void enable_rolling_files(std::size_t max_size)
{
std::string size = std::to_string( max_size/2 );
el::Loggers::addFlag(el::LoggingFlag::StrictLogFileSizeCheck);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please explain -- have you tried without?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Rollout checking happens when Easylogging++ flushes the log file, or, if you have added the flag el::LoggingFlags::StrictLogFileSizeCheck, at each log output.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

std::remove(old_filename);
}

rename(filename, old_filename);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not std::?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

for (int i = 0; i < 100; ++i)
log_all();

std::ifstream log_file(log_filename.c_str(), std::ios::binary);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add reset_logger() before to have it flush everything

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@aseelegbaria
Copy link
Contributor Author

aseelegbaria commented Jan 21, 2021

opened new one: #8212

@aseelegbaria aseelegbaria deleted the rollinglog branch June 3, 2021 16:38
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.

2 participants