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

feat(stackable-telemetry): Add RollingFileAppender #933

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions crates/stackable-telemetry/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@ All notable changes to this project will be documented in this file.
### Added

- Introduce common `Settings` and subscriber specific settings ([#901]).
- Added support for logging to files ([#933]).
Techassi marked this conversation as resolved.
Show resolved Hide resolved

### Changed

- BREAKING: Renamed `TracingBuilder` methods with long names, and prefix with `with_` ([#901]).
- BREAKING: Use the new subscriber settings in the `TracingBuilder` ([#901]).

[#901]: https://github.com/stackabletech/operator-rs/pull/901
[#933]: https://github.com/stackabletech/operator-rs/pull/933

## [0.2.0] - 2024-07-10

Expand Down
1 change: 1 addition & 0 deletions crates/stackable-telemetry/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ snafu.workspace = true
tokio.workspace = true
tower.workspace = true
tracing.workspace = true
tracing-appender.workspace = true
tracing-opentelemetry.workspace = true
tracing-subscriber = { workspace = true, features = ["env-filter"] }

Expand Down
81 changes: 73 additions & 8 deletions crates/stackable-telemetry/src/tracing/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//! This module contains functionality to initialise tracing Subscribers for
//! console output, and OpenTelemetry OTLP export for traces and logs.
//! console output, file output and OpenTelemetry OTLP export for traces and logs.
NickLarsenNZ marked this conversation as resolved.
Show resolved Hide resolved
//!
//! It is intended to be used by the Stackable Data Platform operators and
//! webhooks, but it should be generic enough to be used in any application.
Expand All @@ -16,9 +16,10 @@ use opentelemetry_sdk::{
use opentelemetry_semantic_conventions::resource;
use snafu::{ResultExt as _, Snafu};
use tracing::subscriber::SetGlobalDefaultError;
use tracing_appender::rolling::{RollingFileAppender, Rotation};
use tracing_subscriber::{filter::Directive, layer::SubscriberExt, EnvFilter, Layer, Registry};

use settings::{ConsoleLogSettings, OtlpLogSettings, OtlpTraceSettings};
use settings::{ConsoleLogSettings, FileLogSettings, OtlpLogSettings, OtlpTraceSettings};

pub mod settings;

Expand Down Expand Up @@ -151,6 +152,7 @@ pub enum Error {
pub struct Tracing {
service_name: &'static str,
console_log_settings: ConsoleLogSettings,
file_log_settings: FileLogSettings,
otlp_log_settings: OtlpLogSettings,
otlp_trace_settings: OtlpTraceSettings,
logger_provider: Option<LoggerProvider>,
Expand Down Expand Up @@ -181,6 +183,29 @@ impl Tracing {
layers.push(console_output_layer.boxed());
}

if self.file_log_settings.enabled {
let env_filter_layer = env_filter_builder(
self.file_log_settings.common_settings.environment_variable,
self.file_log_settings.default_level,
);
NickLarsenNZ marked this conversation as resolved.
Show resolved Hide resolved

let file_appender = RollingFileAppender::builder()
.rotation(Rotation::HOURLY)
.filename_prefix(self.service_name.to_string())
.filename_suffix("tracing-rs.json")
.max_log_files(6)
.build(&self.file_log_settings.file_log_dir)
.expect("failed to initialize rolling file appender");
Techassi marked this conversation as resolved.
Show resolved Hide resolved

layers.push(
tracing_subscriber::fmt::layer()
.json()
.with_writer(file_appender)
.with_filter(env_filter_layer)
.boxed(),
);
}

if self.otlp_log_settings.enabled {
let env_filter_layer = env_filter_builder(
self.otlp_log_settings.environment_variable,
Expand Down Expand Up @@ -320,12 +345,6 @@ mod builder_state {
#[derive(Default)]
pub struct PreServiceName;

/// The state before the [`EnvFilter`][1] environment variable name is set.
///
/// [1]: tracing_subscriber::filter::EnvFilter
#[derive(Default)]
pub struct PreEnvVar;

/// The state that allows you to configure the supported [`Subscriber`][1]
/// [`Layer`][2].
///
Expand All @@ -349,6 +368,7 @@ pub struct TracingBuilder<S: BuilderState> {
console_log_settings: ConsoleLogSettings,
otlp_log_settings: OtlpLogSettings,
otlp_trace_settings: OtlpTraceSettings,
file_log_settings: FileLogSettings,

/// Allow the generic to be used (needed for impls).
_marker: std::marker::PhantomData<S>,
Expand Down Expand Up @@ -381,6 +401,26 @@ impl TracingBuilder<builder_state::Config> {
console_log_settings,
otlp_log_settings: self.otlp_log_settings,
otlp_trace_settings: self.otlp_trace_settings,
file_log_settings: self.file_log_settings,
_marker: self._marker,
}
}

/// Enable the file output tracing subscriber and set the default
/// [`LevelFilter`][1] which is overridable through the given environment
/// variable.
///
/// [1]: tracing_subscriber::filter::LevelFilter
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,
Comment on lines +482 to +491
Copy link
Member

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.

Suggested change
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,

_marker: self._marker,
}
}
Expand All @@ -401,6 +441,7 @@ impl TracingBuilder<builder_state::Config> {
console_log_settings: self.console_log_settings,
otlp_log_settings,
otlp_trace_settings: self.otlp_trace_settings,
file_log_settings: self.file_log_settings,
_marker: self._marker,
}
}
Expand All @@ -421,6 +462,7 @@ impl TracingBuilder<builder_state::Config> {
console_log_settings: self.console_log_settings,
otlp_log_settings: self.otlp_log_settings,
otlp_trace_settings,
file_log_settings: self.file_log_settings,
_marker: self._marker,
}
}
Expand All @@ -437,6 +479,7 @@ impl TracingBuilder<builder_state::Config> {
console_log_settings: self.console_log_settings,
otlp_log_settings: self.otlp_log_settings,
otlp_trace_settings: self.otlp_trace_settings,
file_log_settings: self.file_log_settings,
logger_provider: None,
}
}
Expand All @@ -452,6 +495,8 @@ fn env_filter_builder(env_var: &str, default_directive: impl Into<Directive>) ->

#[cfg(test)]
mod test {
use std::path::PathBuf;

use settings::{Build as _, Settings};
use tracing::level_filters::LevelFilter;

Expand Down Expand Up @@ -510,6 +555,15 @@ mod test {
.enabled(true)
.build(),
)
.with_file_output(
Settings::builder()
.with_environment_variable("ABC_FILE")
.with_default_level(LevelFilter::INFO)
.enabled(true)
.file_log_settings_builder()
.with_file_log_dir(String::from("/abc_file_dir"))
.build(),
)
.with_otlp_log_exporter(
Settings::builder()
.with_environment_variable("ABC_OTLP_LOG")
Expand Down Expand Up @@ -537,6 +591,17 @@ mod test {
log_format: Default::default()
}
);
assert_eq!(
trace_guard.file_log_settings,
FileLogSettings {
common_settings: Settings {
enabled: true,
environment_variable: "ABC_FILE",
default_level: LevelFilter::INFO
},
file_log_dir: PathBuf::from("/abc_file_dir")
}
);
assert_eq!(
trace_guard.otlp_log_settings,
OtlpLogSettings {
Expand Down
155 changes: 155 additions & 0 deletions crates/stackable-telemetry/src/tracing/settings/file_log.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
//! File Log Subscriber Settings.

use std::{ops::Deref, path::PathBuf};

use super::{Build, Settings, SettingsBuilder};

/// Configure specific settings for the File Log subscriber.
#[derive(Debug, Default, PartialEq)]
pub struct FileLogSettings {
/// Common subscriber settings that apply to the File Log Subscriber.
pub common_settings: Settings,

/// Path to directory for log files.
pub file_log_dir: PathBuf,
}

impl Deref for FileLogSettings {
type Target = Settings;

fn deref(&self) -> &Self::Target {
&self.common_settings
}
}

/// This trait is only used for the typestate builder and cannot be implemented
/// outside of this crate.
///
/// The only reason it has pub visibility is because it needs to be at least as
/// visible as the types that use it.
#[doc(hidden)]
pub trait BuilderState: private::Sealed {}

/// This private module holds the [`Sealed`][1] trait that is used by the
/// [`BuilderState`], so that it cannot be implemented outside of this crate.
///
/// We impl Sealed for any types that will use the trait that we want to
/// restrict impls on. In this case, the [`BuilderState`] trait.
///
/// [1]: private::Sealed
#[doc(hidden)]
mod private {
use super::*;

pub trait Sealed {}

impl Sealed for builder_state::PreLogDir {}
impl Sealed for builder_state::Config {}
}

/// This module holds the possible states that the builder is in.
///
/// Each state will implement [`BuilderState`] (with no methods), and the
/// Builder struct ([`FileLogSettingsBuilder`][1]) itself will be implemented with
/// each state as a generic parameter.
/// This allows only the methods to be called when the builder is in the
/// applicable state.
///
/// [1]: super::FileLogSettingsBuilder
#[doc(hidden)]
pub mod builder_state {
/// The initial state, before the log directory path is set.
#[derive(Default)]
pub struct PreLogDir;

/// The state that allows you to configure the [`FileLogSettings`][1].
///
/// [1]: super::FileLogSettings
#[derive(Default)]
pub struct Config;
}

// Make the states usable
#[doc(hidden)]
impl BuilderState for builder_state::PreLogDir {}

#[doc(hidden)]
impl BuilderState for builder_state::Config {}

/// For building [`FileLogSettings`].
///
/// <div class="warning">
/// Do not use directly, instead use the [`Settings::builder`] associated function.
/// </div>
pub struct FileLogSettingsBuilder<S: BuilderState> {
pub(crate) common_settings: Settings,
pub(crate) file_log_dir: Option<PathBuf>,
Techassi marked this conversation as resolved.
Show resolved Hide resolved

/// Allow the generic to be used (needed for impls).
_marker: std::marker::PhantomData<S>,
}

impl FileLogSettingsBuilder<builder_state::PreLogDir> {
/// Set the directory for log files.
///
/// A directory is required for using the File Log subscriber.
pub fn with_file_log_dir(self, path: String) -> FileLogSettingsBuilder<builder_state::Config> {
FileLogSettingsBuilder {
common_settings: self.common_settings,
file_log_dir: Some(PathBuf::from(path)),
_marker: std::marker::PhantomData,
}
}
Techassi marked this conversation as resolved.
Show resolved Hide resolved
}

impl FileLogSettingsBuilder<builder_state::Config> {
/// Consumes self and returns a valid [`FileLogSettings`] instance.
pub fn build(self) -> FileLogSettings {
FileLogSettings {
common_settings: self.common_settings,
file_log_dir: self
.file_log_dir
.expect("file_log_dir must be configured at this point"),
}
}
}

/// This implementation is used to turn the common settings builder into the file log specific
/// settings builder via the [`SettingsBuilder::file_log_settings_builder`] function.
impl From<SettingsBuilder> for FileLogSettingsBuilder<builder_state::PreLogDir> {
fn from(value: SettingsBuilder) -> Self {
Self {
common_settings: value.build(),
file_log_dir: None,
_marker: std::marker::PhantomData,
}
}
}

#[cfg(test)]
mod test {
use tracing::level_filters::LevelFilter;

use super::*;

#[test]
fn builds_settings() {
let expected = FileLogSettings {
common_settings: Settings {
environment_variable: "hello",
default_level: LevelFilter::DEBUG,
enabled: true,
},
file_log_dir: PathBuf::from("/logs"),
};
let result = Settings::builder()
.with_environment_variable("hello")
.with_default_level(LevelFilter::DEBUG)
.enabled(true)
.file_log_settings_builder()
.with_file_log_dir(String::from("/logs"))
.build();

assert_eq!(expected, result);
}
}
8 changes: 8 additions & 0 deletions crates/stackable-telemetry/src/tracing/settings/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ use tracing::level_filters::LevelFilter;
pub mod console_log;
pub use console_log::*;

pub mod file_log;
pub use file_log::*;

pub mod otlp_log;
pub use otlp_log::*;

Expand Down Expand Up @@ -105,6 +108,11 @@ impl SettingsBuilder {
self.into()
}

/// Set specific [`FileLogSettings`].
pub fn file_log_settings_builder(self) -> FileLogSettingsBuilder<builder_state::PreLogDir> {
Techassi marked this conversation as resolved.
Show resolved Hide resolved
self.into()
}

/// Set specific [`OtlpLogSettings`].
pub fn otlp_log_settings_builder(self) -> OtlpLogSettingsBuilder {
self.into()
Expand Down
Loading