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

Make log rotation configurable #1362

Merged
merged 5 commits into from
Feb 10, 2021
Merged

Conversation

lava
Copy link
Member

@lava lava commented Feb 9, 2021

📔 Description

This PR adds minimal configurability to the rotating file sink used for spdlog.

It adds the option vast.log-rotation to turn off rotation, which is required for production use where VAST's log rotation may interfere with log rotation configured in external programs like systemd or logrotated.

The size threshold, number of rotated log files, and whether to use time-based logrotate are not made configurable, since we did not have anyone asking for these features yet.

📝 Checklist

  • All user-facing changes have changelog entries.
  • The changes are reflected on docs.tenzir.com/vast, if necessary.
  • The PR description contains instructions for the reviewer, if necessary.

🎯 Review Instructions

@dominiklohmann
Copy link
Member

The option should probably be named vast.disable-log-rotation with the default being false so it's in line with the other boolean options.

vast.yaml.example Outdated Show resolved Hide resolved
@lava lava force-pushed the story/ch22337/configurable-log-sink branch from dd916f8 to dee43fa Compare February 10, 2021 12:31
@lava lava force-pushed the story/ch22337/configurable-log-sink branch from dee43fa to cec5201 Compare February 10, 2021 12:32
Copy link
Member

@dominiklohmann dominiklohmann left a comment

Choose a reason for hiding this comment

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

Ran this locally and it works as expected. Just had some nits. Please implement before merging.

libvast/vast/detail/settings.hpp Outdated Show resolved Hide resolved
libvast/src/logger.cpp Outdated Show resolved Hide resolved
vast.yaml.example Outdated Show resolved Hide resolved
vast.yaml.example Outdated Show resolved Hide resolved
@lava lava merged commit 9aae4e1 into master Feb 10, 2021
@lava lava deleted the story/ch22337/configurable-log-sink branch February 10, 2021 16:46
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