-
Notifications
You must be signed in to change notification settings - Fork 15
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
feat(stackable-telemetry): Add RollingFileAppender #933
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The core change looks good, but I left a few comments / ideas on how we should design the API interface.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @xeniape. Just a couple of comments.
Thanks for all the feedback! Adjusted everything in bfe89a7 |
Co-authored-by: Nick <10092581+NickLarsenNZ@users.noreply.github.com>
pub fn with_file_output( | ||
self, | ||
file_log_settings: FileLogSettings, | ||
) -> TracingBuilder<builder_state::Config> { | ||
TracingBuilder { | ||
service_name: self.service_name, | ||
console_log_settings: self.console_log_settings, | ||
file_log_settings, | ||
otlp_log_settings: self.otlp_log_settings, | ||
otlp_trace_settings: self.otlp_trace_settings, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: #940 changed the API interface of these functions and as such I suggest to also change it here to be in line.
pub fn with_file_output( | |
self, | |
file_log_settings: FileLogSettings, | |
) -> TracingBuilder<builder_state::Config> { | |
TracingBuilder { | |
service_name: self.service_name, | |
console_log_settings: self.console_log_settings, | |
file_log_settings, | |
otlp_log_settings: self.otlp_log_settings, | |
otlp_trace_settings: self.otlp_trace_settings, | |
pub fn with_file_output( | |
self, | |
file_log_settings: impl Into<FileLogSettings>, | |
) -> TracingBuilder<builder_state::Config> { | |
TracingBuilder { | |
service_name: self.service_name, | |
console_log_settings: self.console_log_settings, | |
file_log_settings: file_log_settings.into(), | |
otlp_log_settings: self.otlp_log_settings, | |
otlp_trace_settings: self.otlp_trace_settings, |
Description
Part of stackabletech/issues#639. Adds the RollingFileAppender to the stackable-telemetry crate.
Add RollingFileAppender to stackable-telemetry (as was done in #814).
Definition of Done Checklist
Author
Reviewer
Acceptance