diff --git a/src/config.rs b/src/config.rs index f579741..3726ab5 100644 --- a/src/config.rs +++ b/src/config.rs @@ -47,23 +47,6 @@ impl ModuleMatcher { } } -#[cfg_attr(feature = "wasm", derive(tsify::Tsify))] -#[cfg_attr( - feature = "serde", - derive(serde::Serialize, serde::Deserialize), - serde(rename_all = "camelCase") -)] -#[derive(Debug, Clone)] -/// `CallbackConfig` represents details needed when wrapping a function that -/// accepts a callback. These details will be used to construct the -/// `tracingChannel.traceCallback` invocation. -pub struct CallbackConfig { - /// `position` is the ordinal of the callback function within the wrapped - /// function's parameter list. A value of `-1` indicates that the callback - /// is the last parameter in the list. The value is zero based. - pub position: i32, -} - #[cfg_attr( feature = "serde", derive(serde::Serialize, serde::Deserialize), @@ -79,35 +62,16 @@ pub struct InstrumentationConfig { pub channel_name: String, pub module: ModuleMatcher, pub function_query: FunctionQuery, - #[tsify(optional)] - #[serde(default = "InstrumentationConfig::empty_callback_config")] - pub callback_config: CallbackConfig, } impl InstrumentationConfig { #[must_use] pub fn new(channel_name: &str, module: ModuleMatcher, function_query: FunctionQuery) -> Self { - return Self::new_with_callback_config( - channel_name, - module, - function_query, - CallbackConfig { position: -1 }, - ); - } - - #[must_use] - pub fn new_with_callback_config( - channel_name: &str, - module: ModuleMatcher, - function_query: FunctionQuery, - callback_config: CallbackConfig, - ) -> Self { - return Self { + Self { channel_name: channel_name.to_string(), module, function_query, - callback_config, - }; + } } #[must_use] @@ -150,9 +114,4 @@ impl InstrumentationConfig { pub fn matches(&self, module_name: &str, version: &str, file_path: &PathBuf) -> bool { self.module.matches(module_name, version, file_path) } - - #[must_use] - pub fn empty_callback_config() -> CallbackConfig { - return CallbackConfig { position: -1 }; - } } diff --git a/src/function_query.rs b/src/function_query.rs index 9663cf2..6a25e72 100644 --- a/src/function_query.rs +++ b/src/function_query.rs @@ -17,7 +17,12 @@ pub(crate) enum FunctionType { pub enum FunctionKind { Sync, Async, - Callback, + /// `position` is the ordinal of the callback function within the wrapped + /// function's parameter list. A value of `-1` indicates that the callback + /// is the last parameter in the list. The value is zero based. + Callback { + position: i32, + }, } impl FunctionKind { @@ -28,7 +33,7 @@ impl FunctionKind { #[must_use] pub fn is_callback(&self) -> bool { - matches!(self, FunctionKind::Callback) + matches!(self, FunctionKind::Callback { .. }) } #[must_use] @@ -36,7 +41,7 @@ impl FunctionKind { match self { FunctionKind::Sync => "traceSync", FunctionKind::Async => "tracePromise", - FunctionKind::Callback => "traceCallback", + FunctionKind::Callback { .. } => "traceCallback", } } } diff --git a/src/instrumentation.rs b/src/instrumentation.rs index 6375c5b..bb7d135 100644 --- a/src/instrumentation.rs +++ b/src/instrumentation.rs @@ -3,6 +3,7 @@ * This product includes software developed at Datadog (/). Copyright 2025 Datadog, Inc. **/ use crate::config::InstrumentationConfig; +use crate::FunctionKind; use std::path::PathBuf; use swc_core::common::{Span, SyntaxContext}; use swc_core::ecma::{ @@ -403,17 +404,14 @@ fn construct_trace_statement(config: &InstrumentationConfig, channel_name: &str, } let operator = ["tr_ch_apm$", channel_name, ".", config.function_query.kind().tracing_operator()].join(""); - #[allow(clippy::needless_late_init)] - let stmt_str; - if config.function_query.kind().is_callback() == true { - // TODO: figure out how we can pass a `this` argument - let pos = config.callback_config.position.to_string(); - stmt_str = ["return ", &*operator, "(__apm$traced, ", pos.as_str(), ", ", ctx.as_str(), ", this, ...arguments)"].join(""); - } else { - stmt_str = ["return ", &*operator, "(__apm$traced, ", ctx.as_str(), ")"].join(""); - } + let stmt_str = match config.function_query.kind() { + FunctionKind::Callback { position } => { + format!("return {operator}(__apm$traced, {position}, {ctx}, this, ...arguments)") + }, + _ => format!("return {operator}(__apm$traced, {ctx})") + }; - return quote!( + quote!( "$stmt" as Stmt, stmt = Ident::new(Atom::from(stmt_str), Span::default(), SyntaxContext::empty()) ) diff --git a/src/lib.rs b/src/lib.rs index 5d2d704..0394b0f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -14,7 +14,6 @@ #![deny(clippy::correctness)] #![deny(clippy::unwrap_used)] #![allow(clippy::bool_comparison)] -#![allow(clippy::needless_return)] /** * Unless explicitly stated otherwise all files in this repository are licensed under the Apache-2.0 License. diff --git a/tests/callback_cjs/mod.rs b/tests/callback_cjs/mod.rs index acc9513..4cc2cf8 100644 --- a/tests/callback_cjs/mod.rs +++ b/tests/callback_cjs/mod.rs @@ -3,23 +3,22 @@ use orchestrion_js::*; #[test] fn callback_cjs() { - transpile_and_test_with_imports( + transpile_and_test_with_import( file!(), false, - Config::new_single(InstrumentationConfig::new_with_callback_config( + Config::new_single(InstrumentationConfig::new( "foo_ch", ModuleMatcher::new("foo", ">=1.0.0", "index.js").unwrap(), FunctionQuery::FunctionExpression { expression_name: "doWork".to_string(), - kind: FunctionKind::Callback, + kind: FunctionKind::Callback { position: 1 }, index: 0, }, - CallbackConfig { position: 1 }, )), - &[PackageImport { + Some(PackageImport { module_name: "foo".to_string(), module_version: "1.1.1".to_string(), file: "index.js".to_string(), - }], + }), ) } diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 8072b4d..8252bcc 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -27,28 +27,25 @@ pub struct PackageImport { } pub fn transpile_and_test(test_file: &str, mjs: bool, config: Config) { - transpile_and_test_with_imports(test_file, mjs, config, &[]); + transpile_and_test_with_import(test_file, mjs, config, None); } #[rustfmt::skip] -pub fn transpile_and_test_with_imports(test_file: &str, mjs: bool, config: Config, imports: &[PackageImport]) { +pub fn transpile_and_test_with_import(test_file: &str, mjs: bool, config: Config, import: Option) { let test_file = PathBuf::from(test_file); let test_dir = test_file.parent().expect("Couldn't find test directory"); let file_path = PathBuf::from("index.mjs"); let instrumentor = Instrumentor::new(config); - let mut instrumentations; - if imports.len() > 0 { - let import = &imports[0]; - instrumentations = instrumentor.get_matching_instrumentations( + let mut instrumentations = if let Some(import) = import { + instrumentor.get_matching_instrumentations( import.module_name.as_str(), import.module_version.as_str(), &PathBuf::from(&import.file) - ); + ) } else { - instrumentations = - instrumentor.get_matching_instrumentations(TEST_MODULE_NAME, "0.0.1", &file_path); - } + instrumentor.get_matching_instrumentations(TEST_MODULE_NAME, "0.0.1", &file_path) + }; let extension = if mjs { "mjs" } else { "js" }; let instrumentable = test_dir.join(format!("mod.{extension}"));