Skip to content

Commit

Permalink
Fix filtering of log messages generated in native code. (cherrypick of
Browse files Browse the repository at this point in the history
…#11313) (#11318)

### Problem

Log messages generated in native code (including those generated by `--files-not-found-behavior=warn`) cannot be filtered using the Python warnings module, meaning that the `--ignore-pants-warnings` option does not currently work for them.

### Solution

Move to a native implementation of warnings filtering, which allows for filtering out log messages generated by Rust as well as Python.

[ci skip-build-wheels]
  • Loading branch information
stuhood authored Dec 16, 2020
1 parent a1632b1 commit c3e521b
Show file tree
Hide file tree
Showing 12 changed files with 60 additions and 25 deletions.
3 changes: 1 addition & 2 deletions src/python/pants/bin/pants_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from pants.base.exception_sink import ExceptionSink
from pants.base.exiter import ExitCode
from pants.bin.remote_pants_runner import RemotePantsRunner
from pants.init.logging import setup_logging, setup_warning_filtering
from pants.init.logging import setup_logging
from pants.init.util import init_workdir
from pants.option.option_value_container import OptionValueContainer
from pants.option.options_bootstrapper import OptionsBootstrapper
Expand Down Expand Up @@ -71,7 +71,6 @@ def run(self, start_time: float) -> ExitCode:
workdir_src = init_workdir(global_bootstrap_options)
ExceptionSink.reset_log_location(workdir_src)

setup_warning_filtering(global_bootstrap_options.ignore_pants_warnings or [])
# We enable logging here, and everything before it will be routed through regular
# Python logging.
setup_logging(global_bootstrap_options, stderr_logging=True)
Expand Down
8 changes: 7 additions & 1 deletion src/python/pants/engine/internals/native.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,16 @@ def init_rust_logging(
use_color: bool,
show_target: bool,
log_levels_by_target: Mapping[str, LogLevel],
message_regex_filters: Iterable[str],
):
log_levels_as_ints = {k: v.level for k, v in log_levels_by_target.items()}
return self.lib.init_logging(
level, log_show_rust_3rdparty, use_color, show_target, log_levels_as_ints
level,
log_show_rust_3rdparty,
use_color,
show_target,
log_levels_as_ints,
tuple(message_regex_filters),
)

def default_cache_path(self) -> str:
Expand Down
25 changes: 14 additions & 11 deletions src/python/pants/init/logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import http.client
import logging
import os
import warnings
from logging import Formatter, Handler, LogRecord, StreamHandler
from typing import Dict, Iterable, Optional, Tuple

Expand All @@ -28,20 +27,18 @@ def init_rust_logger(
use_color: bool,
show_target: bool,
log_levels_by_target: Dict[str, LogLevel] = {},
message_regex_filters: Iterable[str] = (),
) -> None:
Native().init_rust_logging(
log_level.level, log_show_rust_3rdparty, use_color, show_target, log_levels_by_target
log_level.level,
log_show_rust_3rdparty,
use_color,
show_target,
log_levels_by_target,
message_regex_filters,
)


def setup_warning_filtering(warnings_filter_regexes: Iterable[str]) -> None:
"""Sets up regex-based ignores for messages using the Python warnings system."""

warnings.resetwarnings()
for message_regexp in warnings_filter_regexes or ():
warnings.filterwarnings(action="ignore", message=message_regexp)


class NativeHandler(StreamHandler):
"""This class is installed as a Python logging module handler (using the logging.addHandler
method) and proxies logs to the Rust logging infrastructure."""
Expand Down Expand Up @@ -131,9 +128,15 @@ def setup_logging(global_bootstrap_options: OptionValueContainer, stderr_logging
use_color = global_bootstrap_options.colors
show_target = global_bootstrap_options.show_log_target
log_levels_by_target = get_log_levels_by_target(global_bootstrap_options)
message_regex_filters = global_bootstrap_options.ignore_pants_warnings

init_rust_logger(
global_level, log_show_rust_3rdparty, use_color, show_target, log_levels_by_target
global_level,
log_show_rust_3rdparty,
use_color,
show_target,
log_levels_by_target,
message_regex_filters,
)

print_stacktrace = resolve_conflicting_options(
Expand Down
7 changes: 3 additions & 4 deletions src/python/pants/pantsd/pants_daemon.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
from pants.bin.daemon_pants_runner import DaemonPantsRunner
from pants.engine.internals.native import Native
from pants.init.engine_initializer import GraphScheduler
from pants.init.logging import setup_logging, setup_logging_to_file, setup_warning_filtering
from pants.init.logging import setup_logging, setup_logging_to_file
from pants.init.options_initializer import OptionsInitializer
from pants.option.option_value_container import OptionValueContainer
from pants.option.options import Options
Expand Down Expand Up @@ -45,13 +45,12 @@ class RuntimeFailure(Exception):

@classmethod
def create(cls, options_bootstrapper: OptionsBootstrapper) -> "PantsDaemon":

# Any warnings that would be triggered here are re-triggered later per-run of Pants, so we
# silence them.
with warnings.catch_warnings(record=True):
bootstrap_options = options_bootstrapper.bootstrap_options
bootstrap_options_values = bootstrap_options.for_global_scope()

setup_warning_filtering(bootstrap_options_values.ignore_pants_warnings or [])

native = Native()
native.override_thread_logging_destination_to_just_pantsd()

Expand Down
2 changes: 0 additions & 2 deletions src/python/pants/pantsd/pants_daemon_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
from typing_extensions import Protocol

from pants.init.engine_initializer import EngineInitializer, GraphScheduler
from pants.init.logging import setup_warning_filtering
from pants.init.options_initializer import BuildConfigInitializer
from pants.option.option_value_container import OptionValueContainer
from pants.option.options_bootstrapper import OptionsBootstrapper
Expand Down Expand Up @@ -77,7 +76,6 @@ def _init_scheduler(
build_config = BuildConfigInitializer.get(options_bootstrapper)
self._scheduler = EngineInitializer.setup_graph(options_bootstrapper, build_config)
bootstrap_options_values = options_bootstrapper.bootstrap_options.for_global_scope()
setup_warning_filtering(bootstrap_options_values.ignore_pants_warnings or [])

self._services = self._services_constructor(bootstrap_options_values, self._scheduler)
self._fingerprint = options_fingerprint
Expand Down
1 change: 1 addition & 0 deletions src/rust/engine/Cargo.lock

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

1 change: 1 addition & 0 deletions src/rust/engine/logging/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ log = "0.4"
num_enum = "0.4"
parking_lot = "0.11"
tokio = { version = "=0.2.20", features = ["rt-util"] } # TODO: see https://github.com/pantsbuild/pants/issues/10291
regex = "1"
uuid = { version = "0.7", features = ["v4"] }

[build-dependencies]
Expand Down
16 changes: 16 additions & 0 deletions src/rust/engine/logging/src/logger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use std::sync::atomic::{AtomicBool, Ordering};
use lazy_static::lazy_static;
use log::{debug, log, set_logger, set_max_level, LevelFilter, Log, Metadata, Record};
use parking_lot::Mutex;
use regex::Regex;
use tokio::task_local;
use uuid::Uuid;

Expand All @@ -27,6 +28,7 @@ lazy_static! {
pub static ref PANTS_LOGGER: PantsLogger = PantsLogger::new();
}

// TODO: The non-atomic portions of this struct should likely be composed into a single RwLock.
pub struct PantsLogger {
log_file: Mutex<Option<File>>,
global_level: Mutex<RefCell<LevelFilter>>,
Expand All @@ -35,6 +37,7 @@ pub struct PantsLogger {
stderr_handlers: Mutex<HashMap<Uuid, StdioHandler>>,
show_target: AtomicBool,
log_level_filters: Mutex<HashMap<String, log::LevelFilter>>,
message_regex_filters: Mutex<Vec<Regex>>,
}

impl PantsLogger {
Expand All @@ -47,6 +50,7 @@ impl PantsLogger {
stderr_handlers: Mutex::new(HashMap::new()),
show_target: AtomicBool::new(false),
log_level_filters: Mutex::new(HashMap::new()),
message_regex_filters: Mutex::new(Vec::new()),
}
}

Expand All @@ -56,6 +60,7 @@ impl PantsLogger {
use_color: bool,
show_target: bool,
log_levels_by_target: HashMap<String, u64>,
message_regex_filters: Vec<Regex>,
) {
let log_levels_by_target = log_levels_by_target
.iter()
Expand All @@ -82,6 +87,7 @@ impl PantsLogger {
.show_rust_3rdparty_logs
.store(show_rust_3rdparty_logs, Ordering::SeqCst);
*PANTS_LOGGER.log_level_filters.lock() = log_levels_by_target;
*PANTS_LOGGER.message_regex_filters.lock() = message_regex_filters;
PANTS_LOGGER
.show_target
.store(show_target, Ordering::SeqCst);
Expand Down Expand Up @@ -216,6 +222,16 @@ impl Log for PantsLogger {
format!("{} {} {}", time_str, level_marker, record.args())
};

{
let message_regex_filters = self.message_regex_filters.lock();
if message_regex_filters
.iter()
.any(|re| re.is_match(&log_string))
{
return;
}
}

match destination {
Destination::Stderr => {
// We first try to output to all registered handlers. If there are none, or any of them
Expand Down
2 changes: 1 addition & 1 deletion src/rust/engine/process_execution/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ tokio-util = { version = "0.2", features = ["codec"] }
uname = "0.1.1"
uuid = { version = "0.7", features = ["v4"] }
workunit_store = { path = "../workunit_store" }
regex = "1.3.1"
regex = "1"
lazy_static = "1"
parking_lot = "0.11"
itertools = "0.8.0"
Expand Down
13 changes: 12 additions & 1 deletion src/rust/engine/src/externs/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ use hashing::{Digest, EMPTY_DIGEST};
use log::{self, debug, error, warn, Log};
use logging::logger::PANTS_LOGGER;
use logging::{Destination, Logger, PythonLogLevel};
use regex::Regex;
use rule_graph::{self, RuleGraph};
use std::collections::hash_map::HashMap;
use task_executor::Executor;
Expand All @@ -90,7 +91,7 @@ py_module_initializer!(native_engine, |py, m| {
"init_logging",
py_fn!(
py,
init_logging(a: u64, b: bool, c: bool, d: bool, e: PyDict)
init_logging(a: u64, b: bool, c: bool, d: bool, e: PyDict, f: Vec<String>)
),
)?;
m.add(
Expand Down Expand Up @@ -1652,6 +1653,7 @@ fn init_logging(
use_color: bool,
show_target: bool,
log_levels_by_target: PyDict,
message_regex_filters: Vec<String>,
) -> PyUnitResult {
let log_levels_by_target = log_levels_by_target
.items(py)
Expand All @@ -1662,12 +1664,21 @@ fn init_logging(
(k, v)
})
.collect::<HashMap<_, _>>();
let message_regex_filters = message_regex_filters
.iter()
.map(|re| {
Regex::new(re).map_err(|e| {
PyErr::new::<exc::Exception, _>(py, (format!("Failed to parse warning filter: {}", e),))
})
})
.collect::<Result<Vec<Regex>, _>>()?;
Logger::init(
level,
show_rust_3rdparty_logs,
use_color,
show_target,
log_levels_by_target,
message_regex_filters,
);
Ok(None)
}
Expand Down
5 changes: 2 additions & 3 deletions tests/python/pants_test/bin/runner_integration_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,8 @@ def test_warning_filter() -> None:
GLOBAL_SCOPE_CONFIG_SECTION: {
# NB: We do *not* include the exclamation point at the end, which tests that the
# regexps match from the beginning of the warning string, and don't require
# matching the entire string! We also lowercase the message to check that they are
# matched case-insensitively.
"ignore_pants_warnings": ["deprecated: this is a test warning"]
# matching the entire string!
"ignore_pants_warnings": ["DEPRECATED: This is a test warning"]
},
},
)
Expand Down
2 changes: 2 additions & 0 deletions tests/python/pants_test/init/test_logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ def test_file_logging() -> None:
use_color=False,
show_target=False,
log_levels_by_target={},
message_regex_filters=(),
)
logger = logging.getLogger("my_file_logger")
with temporary_dir() as tmpdir:
Expand Down Expand Up @@ -48,6 +49,7 @@ def test_log_filtering_by_rule() -> None:
log_levels_by_target={
"debug_target": LogLevel.DEBUG,
},
message_regex_filters=(),
)
with temporary_dir() as tmpdir:
setup_logging_to_file(LogLevel.INFO, log_dir=tmpdir)
Expand Down

0 comments on commit c3e521b

Please sign in to comment.