Skip to content

Commit b27aa03

Browse files
authored
feat: Support comet native log level conf (#2379)
1 parent 9f95e8b commit b27aa03

File tree

5 files changed

+57
-9
lines changed

5 files changed

+57
-9
lines changed

common/src/main/java/org/apache/comet/NativeBase.java

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535

3636
import static org.apache.comet.Constants.LOG_CONF_NAME;
3737
import static org.apache.comet.Constants.LOG_CONF_PATH;
38+
import static org.apache.comet.Constants.LOG_LEVEL_ENV;
3839

3940
/** Base class for JNI bindings. MUST be inherited by all classes that introduce JNI APIs. */
4041
public abstract class NativeBase {
@@ -153,18 +154,25 @@ private static void bundleLoadLibrary() {
153154

154155
private static void initWithLogConf() {
155156
String logConfPath = System.getProperty(LOG_CONF_PATH(), Utils.getConfPath(LOG_CONF_NAME()));
157+
String logLevel = System.getenv(LOG_LEVEL_ENV());
156158

157159
// If both the system property and the environmental variable failed to find a log
158160
// configuration, then fall back to using the deployed default
159161
if (logConfPath == null) {
160162
LOG.info(
161163
"Couldn't locate log file from either COMET_CONF_DIR or comet.log.file.path. "
162-
+ "Using default log configuration which emits to stdout");
164+
+ "Using default log configuration with {} log level which emits to stderr",
165+
logLevel == null ? "INFO" : logLevel);
163166
logConfPath = "";
164167
} else {
168+
// Ignore log level if a log configuration file is specified
169+
if (logLevel != null) {
170+
LOG.warn("Ignoring log level {} because a log configuration file is specified", logLevel);
171+
}
172+
165173
LOG.info("Using {} for native library logging", logConfPath);
166174
}
167-
init(logConfPath);
175+
init(logConfPath, logLevel);
168176
}
169177

170178
private static void cleanupOldTempLibs() {
@@ -283,7 +291,7 @@ private static String resourceName() {
283291
*
284292
* @param logConfPath location to the native log configuration file
285293
*/
286-
static native void init(String logConfPath);
294+
static native void init(String logConfPath, String logLevel);
287295

288296
/**
289297
* Check if a specific feature is enabled in the native library.

common/src/main/scala/org/apache/comet/Constants.scala

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
package org.apache.comet
2121

2222
object Constants {
23+
val COMET_CONF_DIR_ENV = "COMET_CONF_DIR"
2324
val LOG_CONF_PATH = "comet.log.file.path"
2425
val LOG_CONF_NAME = "log4rs.yaml"
26+
val LOG_LEVEL_ENV = "COMET_LOG_LEVEL"
2527
}

common/src/main/scala/org/apache/spark/sql/comet/util/Utils.scala

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,14 @@ import org.apache.spark.sql.types._
3939
import org.apache.spark.sql.vectorized.ColumnarBatch
4040
import org.apache.spark.util.io.{ChunkedByteBuffer, ChunkedByteBufferOutputStream}
4141

42+
import org.apache.comet.Constants.COMET_CONF_DIR_ENV
4243
import org.apache.comet.shims.CometTypeShim
4344
import org.apache.comet.vector.CometVector
4445

4546
object Utils extends CometTypeShim {
4647
def getConfPath(confFileName: String): String = {
4748
sys.env
48-
.get("COMET_CONF_DIR")
49+
.get(COMET_CONF_DIR_ENV)
4950
.map { t => new File(s"$t${File.separator}$confFileName") }
5051
.filter(_.isFile)
5152
.map(_.getAbsolutePath)

docs/source/contributor-guide/debugging.md

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,8 @@ Detecting the debugger
101101

102102
## Verbose debug
103103

104+
### Exception details
105+
104106
By default, Comet outputs the exception details specific for Comet.
105107

106108
```scala
@@ -162,3 +164,29 @@ Note:
162164

163165
- 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)
164166
- The backtrace evaluation comes with performance cost and intended mostly for debugging purposes
167+
168+
### Native log configuration
169+
170+
By default, Comet emits native-side logs at the `INFO` level to `stderr`.
171+
172+
You can use the `COMET_LOG_LEVEL` environment variable to specify the log level. Supported values are: `OFF`, `ERROR`, `WARN`, `INFO`, `DEBUG`, `TRACE`.
173+
174+
For example, to configure native logs at the `DEBUG` level on spark executor:
175+
176+
```
177+
spark.executorEnv.COMET_LOG_LEVEL=DEBUG
178+
```
179+
180+
This produces output like the following:
181+
182+
```
183+
25/09/15 20:17:42 INFO core/src/lib.rs: Comet native library version 0.11.0 initialized
184+
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
185+
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
186+
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
187+
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
188+
...
189+
```
190+
191+
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).
192+
For example, see: [log4rs.yaml](https://github.com/apache/datafusion-comet/blob/main/conf/log4rs.yaml).

native/core/src/lib.rs

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ use jni::{
3030
objects::{JClass, JString},
3131
JNIEnv, JavaVM,
3232
};
33-
use log::{info, LevelFilter};
33+
use log::info;
3434
use log4rs::{
3535
append::console::{ConsoleAppender, Target},
3636
config::{load_config_file, Appender, Deserializers, Root},
@@ -70,6 +70,7 @@ pub extern "system" fn Java_org_apache_comet_NativeBase_init(
7070
e: JNIEnv,
7171
_: JClass,
7272
log_conf_path: JString,
73+
log_level: JString,
7374
) {
7475
// Initialize the error handling to capture panic backtraces
7576
errors::init();
@@ -80,7 +81,11 @@ pub extern "system" fn Java_org_apache_comet_NativeBase_init(
8081
// empty path means there is no custom log4rs config file provided, so fallback to use
8182
// the default configuration
8283
let log_config = if path.is_empty() {
83-
default_logger_config()
84+
let log_level: String = match env.get_string(&log_level) {
85+
Ok(level) => level.into(),
86+
Err(_) => "info".parse().unwrap(),
87+
};
88+
default_logger_config(&log_level)
8489
} else {
8590
load_config_file(path, Deserializers::default())
8691
.map_err(|err| CometError::Config(err.to_string()))
@@ -129,14 +134,18 @@ pub extern "system" fn Java_org_apache_comet_NativeBase_isFeatureEnabled(
129134
})
130135
}
131136

132-
// Creates a default log4rs config, which logs to console with `INFO` level.
133-
fn default_logger_config() -> CometResult<Config> {
137+
// Creates a default log4rs config, which logs to console with log level.
138+
fn default_logger_config(log_level: &str) -> CometResult<Config> {
134139
let console_append = ConsoleAppender::builder()
135140
.target(Target::Stderr)
136141
.encoder(Box::new(PatternEncoder::new(LOG_PATTERN)))
137142
.build();
138143
let appender = Appender::builder().build("console", Box::new(console_append));
139-
let root = Root::builder().appender("console").build(LevelFilter::Info);
144+
let root = Root::builder().appender("console").build(
145+
log_level
146+
.parse()
147+
.map_err(|err| CometError::Config(format!("{err}")))?,
148+
);
140149
Config::builder()
141150
.appender(appender)
142151
.build(root)

0 commit comments

Comments
 (0)