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: otel resiliency #26857

Merged
merged 3 commits into from
Nov 14, 2024
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
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ jsonc-parser = { version = "=0.26.2", features = ["serde"] }
lazy-regex = "3"
libc = "0.2.126"
libz-sys = { version = "1.1.20", default-features = false }
log = "0.4.20"
log = { version = "0.4.20", features = ["kv"] }
lsp-types = "=0.97.0" # used by tower-lsp and "proposed" feature is unstable in patch releases
memmem = "0.1.1"
monch = "=0.5.0"
Expand Down
19 changes: 19 additions & 0 deletions cli/args/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ use deno_path_util::normalize_path;
use deno_path_util::url_to_file_path;
use deno_runtime::deno_permissions::PermissionsOptions;
use deno_runtime::deno_permissions::SysDescriptor;
use deno_runtime::ops::otel::OtelConfig;
use log::debug;
use log::Level;
use serde::Deserialize;
Expand Down Expand Up @@ -968,6 +969,24 @@ impl Flags {
args
}

pub fn otel_config(&self) -> Option<OtelConfig> {
if self
.unstable_config
.features
.contains(&String::from("otel"))
{
Some(OtelConfig {
runtime_name: Cow::Borrowed("deno"),
runtime_version: Cow::Borrowed(crate::version::DENO_VERSION_INFO.deno),
deterministic: std::env::var("DENO_UNSTABLE_OTEL_DETERMINISTIC")
.is_ok(),
..Default::default()
})
} else {
None
}
}

/// Extract the paths the config file should be discovered from.
///
/// Returns `None` if the config file should not be auto-discovered.
Expand Down
19 changes: 2 additions & 17 deletions cli/args/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -823,10 +823,8 @@ impl CliOptions {
};
let msg =
format!("DANGER: TLS certificate validation is disabled {}", domains);
#[allow(clippy::print_stderr)]
{
// use eprintln instead of log::warn so this always gets shown
eprintln!("{}", colors::yellow(msg));
log::error!("{}", colors::yellow(msg));
}
}

Expand Down Expand Up @@ -1131,20 +1129,7 @@ impl CliOptions {
}

pub fn otel_config(&self) -> Option<OtelConfig> {
if self
.flags
.unstable_config
.features
.contains(&String::from("otel"))
{
Some(OtelConfig {
runtime_name: Cow::Borrowed("deno"),
runtime_version: Cow::Borrowed(crate::version::DENO_VERSION_INFO.deno),
..Default::default()
})
} else {
None
}
self.flags.otel_config()
}

pub fn env_file_name(&self) -> Option<&String> {
Expand Down
1 change: 1 addition & 0 deletions cli/clippy.toml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
disallowed-methods = [
{ path = "reqwest::Client::new", reason = "create an HttpClient via an HttpClientProvider instead" },
{ path = "std::process::exit", reason = "use deno_runtime::exit instead" },
]
disallowed-types = [
{ path = "reqwest::Client", reason = "use crate::http_util::HttpClient instead" },
Expand Down
2 changes: 1 addition & 1 deletion cli/graph_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ pub fn graph_exit_integrity_errors(graph: &ModuleGraph) {
fn exit_for_integrity_error(err: &ModuleError) {
if let Some(err_message) = enhanced_integrity_error_message(err) {
log::error!("{} {}", colors::red("error:"), err_message);
std::process::exit(10);
deno_runtime::exit(10);
}
}

Expand Down
2 changes: 1 addition & 1 deletion cli/lsp/parent_process_checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ pub fn start(parent_process_id: u32) {
std::thread::sleep(Duration::from_secs(10));

if !is_process_active(parent_process_id) {
std::process::exit(1);
deno_runtime::exit(1);
}
});
}
Expand Down
28 changes: 17 additions & 11 deletions cli/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,18 +350,17 @@ fn setup_panic_hook() {
eprintln!("Args: {:?}", env::args().collect::<Vec<_>>());
eprintln!();
orig_hook(panic_info);
std::process::exit(1);
deno_runtime::exit(1);
}));
}

#[allow(clippy::print_stderr)]
fn exit_with_message(message: &str, code: i32) -> ! {
eprintln!(
log::error!(
"{}: {}",
colors::red_bold("error"),
message.trim_start_matches("error: ")
);
std::process::exit(code);
deno_runtime::exit(code);
}

fn exit_for_error(error: AnyError) -> ! {
Expand All @@ -380,13 +379,12 @@ fn exit_for_error(error: AnyError) -> ! {
exit_with_message(&error_string, error_code);
}

#[allow(clippy::print_stderr)]
pub(crate) fn unstable_exit_cb(feature: &str, api_name: &str) {
eprintln!(
log::error!(
"Unstable API '{api_name}'. The `--unstable-{}` flag must be provided.",
feature
);
std::process::exit(70);
deno_runtime::exit(70);
}

pub fn main() {
Expand Down Expand Up @@ -419,7 +417,7 @@ pub fn main() {
drop(profiler);

match result {
Ok(exit_code) => std::process::exit(exit_code),
Ok(exit_code) => deno_runtime::exit(exit_code),
Err(err) => exit_for_error(err),
}
}
Expand All @@ -433,12 +431,21 @@ fn resolve_flags_and_init(
if err.kind() == clap::error::ErrorKind::DisplayVersion =>
{
// Ignore results to avoid BrokenPipe errors.
util::logger::init(None);
let _ = err.print();
std::process::exit(0);
deno_runtime::exit(0);
}
Err(err) => {
util::logger::init(None);
exit_for_error(AnyError::from(err))
}
Err(err) => exit_for_error(AnyError::from(err)),
};

if let Some(otel_config) = flags.otel_config() {
deno_runtime::ops::otel::init(otel_config)?;
}
util::logger::init(flags.log_level);

// TODO(bartlomieju): remove in Deno v2.5 and hard error then.
if flags.unstable_config.legacy_flag_enabled {
log::warn!(
Expand Down Expand Up @@ -467,7 +474,6 @@ fn resolve_flags_and_init(
deno_core::JsRuntime::init_platform(
None, /* import assertions enabled */ false,
);
util::logger::init(flags.log_level);

Ok(flags)
}
20 changes: 12 additions & 8 deletions cli/mainrt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,23 +40,21 @@ use std::env::current_exe;

use crate::args::Flags;

#[allow(clippy::print_stderr)]
pub(crate) fn unstable_exit_cb(feature: &str, api_name: &str) {
eprintln!(
log::error!(
"Unstable API '{api_name}'. The `--unstable-{}` flag must be provided.",
feature
);
std::process::exit(70);
deno_runtime::exit(70);
}

#[allow(clippy::print_stderr)]
fn exit_with_message(message: &str, code: i32) -> ! {
eprintln!(
log::error!(
"{}: {}",
colors::red_bold("error"),
message.trim_start_matches("error: ")
);
std::process::exit(code);
deno_runtime::exit(code);
}

fn unwrap_or_exit<T>(result: Result<T, AnyError>) -> T {
Expand Down Expand Up @@ -89,13 +87,19 @@ fn main() {
let future = async move {
match standalone {
Ok(Some(data)) => {
if let Some(otel_config) = data.metadata.otel_config.clone() {
deno_runtime::ops::otel::init(otel_config)?;
}
util::logger::init(data.metadata.log_level);
load_env_vars(&data.metadata.env_vars_from_env_file);
let exit_code = standalone::run(data).await?;
std::process::exit(exit_code);
deno_runtime::exit(exit_code);
}
Ok(None) => Ok(()),
Err(err) => Err(err),
Err(err) => {
util::logger::init(None);
Err(err)
}
}
};

Expand Down
2 changes: 1 addition & 1 deletion cli/standalone/binary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,7 @@ impl<'a> DenoCompileBinaryWriter<'a> {
Some(bytes) => bytes,
None => {
log::info!("Download could not be found, aborting");
std::process::exit(1)
deno_runtime::exit(1);
}
};

Expand Down
2 changes: 1 addition & 1 deletion cli/tools/lint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ pub async fn lint(
linter.finish()
};
if !success {
std::process::exit(1);
deno_runtime::exit(1);
}
}

Expand Down
2 changes: 2 additions & 0 deletions cli/tools/test/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1357,6 +1357,7 @@ pub async fn report_tests(
if let Err(err) = reporter.flush_report(&elapsed, &tests, &test_steps) {
eprint!("Test reporter failed to flush: {}", err)
}
#[allow(clippy::disallowed_methods)]
std::process::exit(130);
}
}
Expand Down Expand Up @@ -1642,6 +1643,7 @@ pub async fn run_tests_with_watch(
loop {
signal::ctrl_c().await.unwrap();
if !HAS_TEST_RUN_SIGINT_HANDLER.load(Ordering::Relaxed) {
#[allow(clippy::disallowed_methods)]
std::process::exit(130);
}
}
Expand Down
2 changes: 1 addition & 1 deletion cli/tools/upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ pub async fn upgrade(
let Some(archive_data) = download_package(&client, download_url).await?
else {
log::error!("Download could not be found, aborting");
std::process::exit(1)
deno_runtime::exit(1)
};

log::info!(
Expand Down
3 changes: 1 addition & 2 deletions cli/util/file_watcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ impl DebouncedReceiver {
}
}

#[allow(clippy::print_stderr)]
async fn error_handler<F>(watch_future: F) -> bool
where
F: Future<Output = Result<(), AnyError>>,
Expand All @@ -84,7 +83,7 @@ where
Some(e) => format_js_error(e),
None => format!("{err:?}"),
};
eprintln!(
log::error!(
"{}: {}",
colors::red_bold("error"),
error_string.trim_start_matches("error: ")
Expand Down
1 change: 1 addition & 0 deletions cli/util/logger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ impl log::Log for CliLogger {
// thread's state
DrawThread::hide();
self.0.log(record);
deno_runtime::ops::otel::handle_log(record);
DrawThread::show();
}
}
Expand Down
9 changes: 4 additions & 5 deletions cli/util/v8.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,14 @@ pub fn init_v8_flags(
.skip(1)
.collect::<Vec<_>>();

#[allow(clippy::print_stderr)]
if !unrecognized_v8_flags.is_empty() {
for f in unrecognized_v8_flags {
eprintln!("error: V8 did not recognize flag '{f}'");
log::error!("error: V8 did not recognize flag '{f}'");
}
eprintln!("\nFor a list of V8 flags, use '--v8-flags=--help'");
std::process::exit(1);
log::error!("\nFor a list of V8 flags, use '--v8-flags=--help'");
deno_runtime::exit(1);
}
if v8_flags_includes_help {
std::process::exit(0);
deno_runtime::exit(0);
}
}
5 changes: 2 additions & 3 deletions ext/napi/node_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,6 @@ fn napi_fatal_exception(env: &mut Env, err: napi_value) -> napi_status {
}

#[napi_sym]
#[allow(clippy::print_stderr)]
fn napi_fatal_error(
location: *const c_char,
location_len: usize,
Expand Down Expand Up @@ -173,9 +172,9 @@ fn napi_fatal_error(
};

if let Some(location) = location {
eprintln!("NODE API FATAL ERROR: {} {}", location, message);
log::error!("NODE API FATAL ERROR: {} {}", location, message);
} else {
eprintln!("NODE API FATAL ERROR: {}", message);
log::error!("NODE API FATAL ERROR: {}", message);
}

std::process::abort();
Expand Down
3 changes: 1 addition & 2 deletions ext/napi/uv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@ use deno_core::parking_lot::Mutex;
use std::mem::MaybeUninit;
use std::ptr::addr_of_mut;

#[allow(clippy::print_stderr)]
fn assert_ok(res: c_int) -> c_int {
if res != 0 {
eprintln!("bad result in uv polyfill: {res}");
log::error!("bad result in uv polyfill: {res}");
// don't panic because that might unwind into
// c/c++
std::process::abort();
Expand Down
1 change: 1 addition & 0 deletions runtime/clippy.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,5 @@ disallowed-methods = [
{ path = "std::fs::write", reason = "File system operations should be done using FileSystem trait" },
{ path = "std::path::Path::canonicalize", reason = "File system operations should be done using FileSystem trait" },
{ path = "std::path::Path::exists", reason = "File system operations should be done using FileSystem trait" },
{ path = "std::process::exit", reason = "use deno_runtime::exit instead" },
]
Loading