Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix filtering of log messages generated in native code. (cherrypick of #11313) #11318

Merged
merged 2 commits into from
Dec 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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