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 2 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
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
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
22 changes: 19 additions & 3 deletions runtime/js/telemetry.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,14 @@ const {
ReflectApply,
SymbolFor,
Error,
NumberPrototypeToString,
StringPrototypePadStart,
} = primordials;
const { AsyncVariable, setAsyncContext } = core;

const CURRENT = new AsyncVariable();
let TRACING_ENABLED = false;
let DETERMINISTIC = false;

const SPAN_ID_BYTES = 8;
const TRACE_ID_BYTES = 16;
Expand All @@ -45,7 +48,19 @@ const hexSliceLookupTable = (function () {
return table;
})();

let counter = 1;

const INVALID_SPAN_ID = "0000000000000000";
const INVALID_TRACE_ID = "00000000000000000000000000000000";

function generateId(bytes) {
if (DETERMINISTIC) {
return StringPrototypePadStart(
NumberPrototypeToString(counter++, 16),
bytes * 2,
"0",
);
}
let out = "";
for (let i = 0; i < bytes / 4; i += 1) {
const r32 = (MathRandom() * 2 ** 32) >>> 0;
Expand Down Expand Up @@ -112,8 +127,6 @@ function submit(span) {

const now = () => (performance.timeOrigin + performance.now()) / 1000;

const INVALID_SPAN_ID = "0000000000000000";
const INVALID_TRACE_ID = "00000000000000000000000000000000";
const NO_ASYNC_CONTEXT = {};

class Span {
Expand Down Expand Up @@ -362,9 +375,12 @@ const otelConsoleConfig = {

export function bootstrap(config) {
if (config.length === 0) return;
const { 0: consoleConfig } = config;
const { 0: consoleConfig, 1: deterministic } = config;

TRACING_ENABLED = true;
if (deterministic === 1) {
DETERMINISTIC = true;
}

switch (consoleConfig) {
case otelConsoleConfig.capture:
Expand Down
5 changes: 5 additions & 0 deletions runtime/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,11 @@ pub static UNSTABLE_GRANULAR_FLAGS: &[UnstableGranularFlag] = &[
},
];

pub fn exit(code: i32) -> ! {
crate::ops::otel::flush();
std::process::exit(code);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add std::process::exit to disallowed methods in clippy.toml files so someone doesn't accidently use it?

}

#[cfg(test)]
mod test {
use super::*;
Expand Down
4 changes: 1 addition & 3 deletions runtime/ops/os/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,10 +186,8 @@ fn op_get_exit_code(state: &mut OpState) -> i32 {

#[op2(fast)]
fn op_exit(state: &mut OpState) {
crate::ops::otel::otel_drop_state(state);

let code = state.borrow::<ExitCode>().get();
std::process::exit(code)
crate::exit(code)
}

#[op2]
Expand Down
Loading
Loading