From 9e3c07611cc1becb6e69a07009f8b7957076285b Mon Sep 17 00:00:00 2001 From: Christian Schwarz Date: Wed, 22 Nov 2023 12:08:35 +0100 Subject: [PATCH] logging: support output to stderr (#5896) (part of the getpage benchmarking epic #5771) The plan is to make the benchmarking tool log on stderr and emit results as JSON on stdout. That way, the test suite can simply take captures stdout and json.loads() it, while interactive users of the benchmarking tool have a reasonable experience as well. Existing logging users continue to print to stdout, so, this change should be a no-op functionally and performance-wise. --- control_plane/src/bin/attachment_service.rs | 1 + libs/remote_storage/tests/test_real_azure.rs | 1 + libs/remote_storage/tests/test_real_s3.rs | 1 + libs/utils/src/logging.rs | 15 ++++++++++++++- pageserver/src/bin/pageserver.rs | 6 +++++- pageserver/src/tenant.rs | 1 + safekeeper/src/bin/safekeeper.rs | 1 + storage_broker/src/bin/storage_broker.rs | 1 + 8 files changed, 25 insertions(+), 2 deletions(-) diff --git a/control_plane/src/bin/attachment_service.rs b/control_plane/src/bin/attachment_service.rs index ddd50ff51d..16577e27d6 100644 --- a/control_plane/src/bin/attachment_service.rs +++ b/control_plane/src/bin/attachment_service.rs @@ -286,6 +286,7 @@ async fn main() -> anyhow::Result<()> { logging::init( LogFormat::Plain, logging::TracingErrorLayerEnablement::Disabled, + logging::Output::Stdout, )?; let args = Cli::parse(); diff --git a/libs/remote_storage/tests/test_real_azure.rs b/libs/remote_storage/tests/test_real_azure.rs index 59b92b48fb..b631079bc5 100644 --- a/libs/remote_storage/tests/test_real_azure.rs +++ b/libs/remote_storage/tests/test_real_azure.rs @@ -281,6 +281,7 @@ fn ensure_logging_ready() { utils::logging::init( utils::logging::LogFormat::Test, utils::logging::TracingErrorLayerEnablement::Disabled, + utils::logging::Output::Stdout, ) .expect("logging init failed"); }); diff --git a/libs/remote_storage/tests/test_real_s3.rs b/libs/remote_storage/tests/test_real_s3.rs index e2360b7533..48f00e0106 100644 --- a/libs/remote_storage/tests/test_real_s3.rs +++ b/libs/remote_storage/tests/test_real_s3.rs @@ -210,6 +210,7 @@ fn ensure_logging_ready() { utils::logging::init( utils::logging::LogFormat::Test, utils::logging::TracingErrorLayerEnablement::Disabled, + utils::logging::Output::Stdout, ) .expect("logging init failed"); }); diff --git a/libs/utils/src/logging.rs b/libs/utils/src/logging.rs index 502e02dc71..2f09c2f3ea 100644 --- a/libs/utils/src/logging.rs +++ b/libs/utils/src/logging.rs @@ -66,9 +66,17 @@ pub enum TracingErrorLayerEnablement { EnableWithRustLogFilter, } +/// Where the logging should output to. +#[derive(Clone, Copy)] +pub enum Output { + Stdout, + Stderr, +} + pub fn init( log_format: LogFormat, tracing_error_layer_enablement: TracingErrorLayerEnablement, + output: Output, ) -> anyhow::Result<()> { // We fall back to printing all spans at info-level or above if // the RUST_LOG environment variable is not set. @@ -85,7 +93,12 @@ pub fn init( let log_layer = tracing_subscriber::fmt::layer() .with_target(false) .with_ansi(false) - .with_writer(std::io::stdout); + .with_writer(move || -> Box { + match output { + Output::Stdout => Box::new(std::io::stdout()), + Output::Stderr => Box::new(std::io::stderr()), + } + }); let log_layer = match log_format { LogFormat::Json => log_layer.json().boxed(), LogFormat::Plain => log_layer.boxed(), diff --git a/pageserver/src/bin/pageserver.rs b/pageserver/src/bin/pageserver.rs index 5b0c140d00..f971b0a88d 100644 --- a/pageserver/src/bin/pageserver.rs +++ b/pageserver/src/bin/pageserver.rs @@ -103,7 +103,11 @@ fn main() -> anyhow::Result<()> { } else { TracingErrorLayerEnablement::Disabled }; - logging::init(conf.log_format, tracing_error_layer_enablement)?; + logging::init( + conf.log_format, + tracing_error_layer_enablement, + logging::Output::Stdout, + )?; // mind the order required here: 1. logging, 2. panic_hook, 3. sentry. // disarming this hook on pageserver, because we never tear down tracing. diff --git a/pageserver/src/tenant.rs b/pageserver/src/tenant.rs index 04fe9db76a..507c017532 100644 --- a/pageserver/src/tenant.rs +++ b/pageserver/src/tenant.rs @@ -3504,6 +3504,7 @@ pub(crate) mod harness { // enable it in case the tests exercise code paths that use // debug_assert_current_span_has_tenant_and_timeline_id logging::TracingErrorLayerEnablement::EnableWithRustLogFilter, + logging::Output::Stdout, ) .expect("Failed to init test logging") }); diff --git a/safekeeper/src/bin/safekeeper.rs b/safekeeper/src/bin/safekeeper.rs index 2e54380471..0b5bb22c8b 100644 --- a/safekeeper/src/bin/safekeeper.rs +++ b/safekeeper/src/bin/safekeeper.rs @@ -202,6 +202,7 @@ async fn main() -> anyhow::Result<()> { logging::init( LogFormat::from_config(&args.log_format)?, logging::TracingErrorLayerEnablement::Disabled, + logging::Output::Stdout, )?; logging::replace_panic_hook_with_tracing_panic_hook().forget(); info!("version: {GIT_VERSION}"); diff --git a/storage_broker/src/bin/storage_broker.rs b/storage_broker/src/bin/storage_broker.rs index 5ce24a6a42..9f81ac6cac 100644 --- a/storage_broker/src/bin/storage_broker.rs +++ b/storage_broker/src/bin/storage_broker.rs @@ -434,6 +434,7 @@ async fn main() -> Result<(), Box> { logging::init( LogFormat::from_config(&args.log_format)?, logging::TracingErrorLayerEnablement::Disabled, + logging::Output::Stdout, )?; logging::replace_panic_hook_with_tracing_panic_hook().forget(); // initialize sentry if SENTRY_DSN is provided