From aa87ddbde560c1475cdf3fb2b0849c72d1577231 Mon Sep 17 00:00:00 2001 From: Christian Schwarz Date: Wed, 22 Nov 2023 08:11:16 +0000 Subject: [PATCH] logging: support output to stderr (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 3205703c7d..f052a1b5ff 100644 --- a/control_plane/src/bin/attachment_service.rs +++ b/control_plane/src/bin/attachment_service.rs @@ -268,6 +268,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 dc07ea7346..1f5d7e2b5b 100644 --- a/pageserver/src/tenant.rs +++ b/pageserver/src/tenant.rs @@ -3494,6 +3494,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