diff --git a/common/src/main/java/org/apache/comet/NativeBase.java b/common/src/main/java/org/apache/comet/NativeBase.java index 64daad98a9..074a4b1625 100644 --- a/common/src/main/java/org/apache/comet/NativeBase.java +++ b/common/src/main/java/org/apache/comet/NativeBase.java @@ -35,6 +35,7 @@ import static org.apache.comet.Constants.LOG_CONF_NAME; import static org.apache.comet.Constants.LOG_CONF_PATH; +import static org.apache.comet.Constants.LOG_LEVEL_ENV; /** Base class for JNI bindings. MUST be inherited by all classes that introduce JNI APIs. */ public abstract class NativeBase { @@ -153,18 +154,25 @@ private static void bundleLoadLibrary() { private static void initWithLogConf() { String logConfPath = System.getProperty(LOG_CONF_PATH(), Utils.getConfPath(LOG_CONF_NAME())); + String logLevel = System.getenv(LOG_LEVEL_ENV()); // If both the system property and the environmental variable failed to find a log // configuration, then fall back to using the deployed default if (logConfPath == null) { LOG.info( "Couldn't locate log file from either COMET_CONF_DIR or comet.log.file.path. " - + "Using default log configuration which emits to stdout"); + + "Using default log configuration with {} log level which emits to stderr", + logLevel == null ? "INFO" : logLevel); logConfPath = ""; } else { + // Ignore log level if a log configuration file is specified + if (logLevel != null) { + LOG.warn("Ignoring log level {} because a log configuration file is specified", logLevel); + } + LOG.info("Using {} for native library logging", logConfPath); } - init(logConfPath); + init(logConfPath, logLevel); } private static void cleanupOldTempLibs() { @@ -283,7 +291,7 @@ private static String resourceName() { * * @param logConfPath location to the native log configuration file */ - static native void init(String logConfPath); + static native void init(String logConfPath, String logLevel); /** * Check if a specific feature is enabled in the native library. diff --git a/common/src/main/scala/org/apache/comet/Constants.scala b/common/src/main/scala/org/apache/comet/Constants.scala index 83b570fc3a..3bafe52402 100644 --- a/common/src/main/scala/org/apache/comet/Constants.scala +++ b/common/src/main/scala/org/apache/comet/Constants.scala @@ -20,6 +20,8 @@ package org.apache.comet object Constants { + val COMET_CONF_DIR_ENV = "COMET_CONF_DIR" val LOG_CONF_PATH = "comet.log.file.path" val LOG_CONF_NAME = "log4rs.yaml" + val LOG_LEVEL_ENV = "COMET_LOG_LEVEL" } diff --git a/common/src/main/scala/org/apache/spark/sql/comet/util/Utils.scala b/common/src/main/scala/org/apache/spark/sql/comet/util/Utils.scala index a72208db27..da61213912 100644 --- a/common/src/main/scala/org/apache/spark/sql/comet/util/Utils.scala +++ b/common/src/main/scala/org/apache/spark/sql/comet/util/Utils.scala @@ -39,13 +39,14 @@ import org.apache.spark.sql.types._ import org.apache.spark.sql.vectorized.ColumnarBatch import org.apache.spark.util.io.{ChunkedByteBuffer, ChunkedByteBufferOutputStream} +import org.apache.comet.Constants.COMET_CONF_DIR_ENV import org.apache.comet.shims.CometTypeShim import org.apache.comet.vector.CometVector object Utils extends CometTypeShim { def getConfPath(confFileName: String): String = { sys.env - .get("COMET_CONF_DIR") + .get(COMET_CONF_DIR_ENV) .map { t => new File(s"$t${File.separator}$confFileName") } .filter(_.isFile) .map(_.getAbsolutePath) diff --git a/docs/source/contributor-guide/debugging.md b/docs/source/contributor-guide/debugging.md index 7055e2d811..8f83960035 100644 --- a/docs/source/contributor-guide/debugging.md +++ b/docs/source/contributor-guide/debugging.md @@ -101,6 +101,8 @@ Detecting the debugger ## Verbose debug +### Exception details + By default, Comet outputs the exception details specific for Comet. ```scala @@ -162,3 +164,29 @@ Note: - The backtrace coverage in DataFusion is still improving. So there is a chance the error still not covered, if so feel free to file a [ticket](https://github.com/apache/arrow-datafusion/issues) - The backtrace evaluation comes with performance cost and intended mostly for debugging purposes + +### Native log configuration + +By default, Comet emits native-side logs at the `INFO` level to `stderr`. + +You can use the `COMET_LOG_LEVEL` environment variable to specify the log level. Supported values are: `OFF`, `ERROR`, `WARN`, `INFO`, `DEBUG`, `TRACE`. + +For example, to configure native logs at the `DEBUG` level on spark executor: + +``` +spark.executorEnv.COMET_LOG_LEVEL=DEBUG +``` + +This produces output like the following: + +``` +25/09/15 20:17:42 INFO core/src/lib.rs: Comet native library version 0.11.0 initialized +25/09/15 20:17:44 DEBUG /xxx/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/datafusion-execution-49.0.2/src/disk_manager.rs: Created local dirs [TempDir { path: "/private/var/folders/4p/9gtjq1s10fd6frkv9kzy0y740000gn/T/blockmgr-ba524f95-a792-4d79-b49c-276ba324941e/datafusion-qrpApx" }] as DataFusion working directory +25/09/15 20:17:44 DEBUG /xxx/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/datafusion-functions-nested-49.0.2/src/lib.rs: Overwrite existing UDF: array_to_string +25/09/15 20:17:44 DEBUG /xxx/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/datafusion-functions-nested-49.0.2/src/lib.rs: Overwrite existing UDF: string_to_array +25/09/15 20:17:44 DEBUG /xxx/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/datafusion-functions-nested-49.0.2/src/lib.rs: Overwrite existing UDF: range +... +``` + +Additionally, you can place a `log4rs.yaml` configuration file inside the Comet configuration directory specified by the `COMET_CONF_DIR` environment variable to enable more advanced logging configurations. This file uses the [log4rs YAML configuration format](https://docs.rs/log4rs/latest/log4rs/#configuration-via-a-yaml-file). +For example, see: [log4rs.yaml](https://github.com/apache/datafusion-comet/blob/main/conf/log4rs.yaml). diff --git a/native/core/src/lib.rs b/native/core/src/lib.rs index 04c9212101..10ecefad5b 100644 --- a/native/core/src/lib.rs +++ b/native/core/src/lib.rs @@ -30,7 +30,7 @@ use jni::{ objects::{JClass, JString}, JNIEnv, JavaVM, }; -use log::{info, LevelFilter}; +use log::info; use log4rs::{ append::console::{ConsoleAppender, Target}, config::{load_config_file, Appender, Deserializers, Root}, @@ -70,6 +70,7 @@ pub extern "system" fn Java_org_apache_comet_NativeBase_init( e: JNIEnv, _: JClass, log_conf_path: JString, + log_level: JString, ) { // Initialize the error handling to capture panic backtraces errors::init(); @@ -80,7 +81,11 @@ pub extern "system" fn Java_org_apache_comet_NativeBase_init( // empty path means there is no custom log4rs config file provided, so fallback to use // the default configuration let log_config = if path.is_empty() { - default_logger_config() + let log_level: String = match env.get_string(&log_level) { + Ok(level) => level.into(), + Err(_) => "info".parse().unwrap(), + }; + default_logger_config(&log_level) } else { load_config_file(path, Deserializers::default()) .map_err(|err| CometError::Config(err.to_string())) @@ -129,14 +134,18 @@ pub extern "system" fn Java_org_apache_comet_NativeBase_isFeatureEnabled( }) } -// Creates a default log4rs config, which logs to console with `INFO` level. -fn default_logger_config() -> CometResult { +// Creates a default log4rs config, which logs to console with log level. +fn default_logger_config(log_level: &str) -> CometResult { let console_append = ConsoleAppender::builder() .target(Target::Stderr) .encoder(Box::new(PatternEncoder::new(LOG_PATTERN))) .build(); let appender = Appender::builder().build("console", Box::new(console_append)); - let root = Root::builder().appender("console").build(LevelFilter::Info); + let root = Root::builder().appender("console").build( + log_level + .parse() + .map_err(|err| CometError::Config(format!("{err}")))?, + ); Config::builder() .appender(appender) .build(root)