From 449e1ea529f0eef6404a9a709d1852356e987dbf Mon Sep 17 00:00:00 2001 From: simon-id Date: Fri, 13 Jun 2025 03:07:19 +0200 Subject: [PATCH 1/7] add test for arguments mutation --- tests/arguments_mutation/mod.js | 30 ++++++++++++++++++++++++ tests/arguments_mutation/mod.rs | 25 ++++++++++++++++++++ tests/arguments_mutation/test.js | 39 ++++++++++++++++++++++++++++++++ tests/instrumentor_test.rs | 1 + 4 files changed, 95 insertions(+) create mode 100644 tests/arguments_mutation/mod.js create mode 100644 tests/arguments_mutation/mod.rs create mode 100644 tests/arguments_mutation/test.js diff --git a/tests/arguments_mutation/mod.js b/tests/arguments_mutation/mod.js new file mode 100644 index 0000000..f648da1 --- /dev/null +++ b/tests/arguments_mutation/mod.js @@ -0,0 +1,30 @@ +/** + * Unless explicitly stated otherwise all files in this repository are licensed under the Apache-2.0 License. + * This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2025 Datadog, Inc. + **/ +const assert = require('assert'); + +function fetch_simple (url, cb) { + assert.strictEqual(this.this, 'this'); + assert.strictEqual(url, 'https://example.com'); + assert.strictEqual(cb.length, 2); + const result = cb.apply(this, ['arg1', 'arg2']); + assert.strictEqual(result, 'result'); + return 'return'; +} + +function fetch_complex ({ url, tuple: [a = 'a', b = 'b'] }, cb, optional = 'default', ...rest) { + assert.strictEqual(this.this, 'this'); + assert.strictEqual(url, 'https://example.com'); + assert.strictEqual(a, 'a'); + assert.strictEqual(b, 'b'); + assert.strictEqual(cb.length, 2); + assert.strictEqual(optional, 'default'); + assert.deepStrictEqual(rest, []); + const result = cb.apply(this, ['arg1', 'arg2']); + assert.strictEqual(result, 'result'); + return 'return'; +} + + +module.exports = { fetch_simple, fetch_complex }; diff --git a/tests/arguments_mutation/mod.rs b/tests/arguments_mutation/mod.rs new file mode 100644 index 0000000..584a249 --- /dev/null +++ b/tests/arguments_mutation/mod.rs @@ -0,0 +1,25 @@ +use crate::common::*; +use orchestrion_js::*; + +#[test] +fn arguments_mutation() { + transpile_and_test( + file!(), + false, + Config::new( + vec![ + InstrumentationConfig::new( + "fetch_simple", + test_module_matcher(), + FunctionQuery::function_declaration("fetch_simple", FunctionKind::Sync), + ), + InstrumentationConfig::new( + "fetch_complex", + test_module_matcher(), + FunctionQuery::function_declaration("fetch_complex", FunctionKind::Sync), + ), + ], + None, + ), + ); +} diff --git a/tests/arguments_mutation/test.js b/tests/arguments_mutation/test.js new file mode 100644 index 0000000..a3b1455 --- /dev/null +++ b/tests/arguments_mutation/test.js @@ -0,0 +1,39 @@ +/** + * Unless explicitly stated otherwise all files in this repository are licensed under the Apache-2.0 License. + * This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2025 Datadog, Inc. + **/ +const { fetch_simple, fetch_complex } = require('./instrumented.js'); +const assert = require('assert'); +const { tracingChannel } = require('diagnostics_channel'); + +const handler = { + start (message) { + const originalCb = message.arguments[1]; + const wrappedCb = function (a, b) { + assert.strictEqual(this.this, 'this'); + assert.strictEqual(a, 'arg1'); + assert.strictEqual(b, 'arg2'); + arguments[1] = 'arg2_mutated'; + return originalCb.apply(this, arguments); + } + + message.arguments[1] = wrappedCb; + } +}; + +tracingChannel('orchestrion:undici:fetch_simple').subscribe(handler); +tracingChannel('orchestrion:undici:fetch_complex').subscribe(handler); + + +assert.strictEqual(fetch_simple.length, 2); +assert.strictEqual(fetch_complex.length, 2); + +const cb = function (a, b) { + assert.strictEqual(this.this, 'this'); + assert.strictEqual(a, 'arg1'); + assert.strictEqual(b, 'arg2_mutated'); + return 'result'; +}; + +assert.strictEqual(fetch_simple.apply({ this: 'this' }, ['https://example.com', cb]), 'return'); +assert.strictEqual(fetch_complex.apply({ this: 'this' }, [{ url: 'https://example.com' }, cb]), 'return'); diff --git a/tests/instrumentor_test.rs b/tests/instrumentor_test.rs index ec5b9fd..2ba2f83 100644 --- a/tests/instrumentor_test.rs +++ b/tests/instrumentor_test.rs @@ -4,6 +4,7 @@ **/ mod common; +mod arguments_mutation; mod class_method_cjs; mod constructor_cjs; mod constructor_mjs; From 3714811236b12239680ca9232ffbd4531a6cd482 Mon Sep 17 00:00:00 2001 From: simon-id Date: Fri, 13 Jun 2025 15:05:53 +0200 Subject: [PATCH 2/7] fix test --- tests/arguments_mutation/test.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/arguments_mutation/test.js b/tests/arguments_mutation/test.js index a3b1455..42de75b 100644 --- a/tests/arguments_mutation/test.js +++ b/tests/arguments_mutation/test.js @@ -24,7 +24,6 @@ const handler = { tracingChannel('orchestrion:undici:fetch_simple').subscribe(handler); tracingChannel('orchestrion:undici:fetch_complex').subscribe(handler); - assert.strictEqual(fetch_simple.length, 2); assert.strictEqual(fetch_complex.length, 2); @@ -36,4 +35,4 @@ const cb = function (a, b) { }; assert.strictEqual(fetch_simple.apply({ this: 'this' }, ['https://example.com', cb]), 'return'); -assert.strictEqual(fetch_complex.apply({ this: 'this' }, [{ url: 'https://example.com' }, cb]), 'return'); +assert.strictEqual(fetch_complex.apply({ this: 'this' }, [{ url: 'https://example.com', tuple: [] }, cb]), 'return'); From 7f7e3e042ecece0b46dd76845d798b8338b18e7a Mon Sep 17 00:00:00 2001 From: simon-id Date: Fri, 13 Jun 2025 15:41:07 +0200 Subject: [PATCH 3/7] add double wrapping for argument mutation in subscriber --- src/instrumentation.rs | 33 ++++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/src/instrumentation.rs b/src/instrumentation.rs index 7912961..637dc58 100644 --- a/src/instrumentation.rs +++ b/src/instrumentation.rs @@ -8,7 +8,7 @@ use swc_core::common::{Span, SyntaxContext}; use swc_core::ecma::{ ast::{ ArrowExpr, AssignExpr, AssignTarget, BlockStmt, ClassDecl, ClassMethod, Constructor, Expr, - FnDecl, FnExpr, Ident, Lit, MemberProp, MethodProp, Module, ModuleItem, Pat, PropName, + FnDecl, FnExpr, Ident, Lit, MemberProp, MethodProp, Module, ModuleItem, Param, Pat, PropName, Script, SimpleAssignTarget, Stmt, Str, VarDecl, }, atoms::Atom, @@ -49,9 +49,9 @@ impl Instrumentation { self.is_correct_class = false; } - fn new_fn(&self, body: BlockStmt) -> ArrowExpr { + fn new_fn(&self, body: BlockStmt, params: Vec) -> ArrowExpr { ArrowExpr { - params: vec![], + params, body: Box::new(body.into()), is_async: self.config.function_query.kind().is_async(), is_generator: false, @@ -81,7 +81,7 @@ impl Instrumentation { define_channel } - fn insert_tracing(&mut self, body: &mut BlockStmt) { + fn insert_tracing(&mut self, body: &mut BlockStmt, params: Vec) { self.count += 1; let original_stmts = std::mem::take(&mut body.stmts); @@ -93,7 +93,21 @@ impl Instrumentation { ..body.clone() }; - let traced_fn = self.new_fn(original_body); + let original_params: Vec = params.into_iter().map(|p| p.pat).collect(); + + let wrapped_fn = self.new_fn(original_body, original_params); + + let traced_body = BlockStmt { + span: Span::default(), + ctxt: SyntaxContext::empty(), + stmts: vec![ + quote!("const __apm$wrapped = $wrapped;" as Stmt, wrapped: Expr = wrapped_fn.into()), + quote!("return __apm$wrapped.apply(null, __apm$original_args);" as Stmt), + ], + }; + + let traced_fn = self.new_fn(traced_body, vec![]); + let ch_ident = ident!(format!("tr_ch_apm${}", &self.config.channel_name)); let trace_ident = ident!(format!( @@ -103,6 +117,7 @@ impl Instrumentation { )); body.stmts = vec![ + quote!("const __apm$original_args = arguments" as Stmt), quote!("const __apm$traced = $traced;" as Stmt, traced: Expr = traced_fn.into()), quote!( "if (!$ch.hasSubscribers) return __apm$traced();" as Stmt, @@ -168,7 +183,7 @@ impl Instrumentation { && func_expr.function.body.is_some() { if let Some(body) = func_expr.function.body.as_mut() { - self.insert_tracing(body); + self.insert_tracing(body, func_expr.function.params.clone()); } true } else { @@ -206,7 +221,7 @@ impl Instrumentation { && node.function.body.is_some() { if let Some(body) = node.function.body.as_mut() { - self.insert_tracing(body); + self.insert_tracing(body, node.function.params.clone()); } } false @@ -253,7 +268,7 @@ impl Instrumentation { && node.function.body.is_some() { if let Some(body) = node.function.body.as_mut() { - self.insert_tracing(body); + self.insert_tracing(body, node.function.params.clone()); } } true @@ -286,7 +301,7 @@ impl Instrumentation { && node.function.body.is_some() { if let Some(body) = node.function.body.as_mut() { - self.insert_tracing(body); + self.insert_tracing(body, node.function.params.clone()); } } false From a92f7ad9ded09446a54c47d9f11f16d069b3150e Mon Sep 17 00:00:00 2001 From: simon-id Date: Fri, 13 Jun 2025 16:04:03 +0200 Subject: [PATCH 4/7] lint --- src/instrumentation.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/instrumentation.rs b/src/instrumentation.rs index 637dc58..4ca4ac6 100644 --- a/src/instrumentation.rs +++ b/src/instrumentation.rs @@ -108,7 +108,6 @@ impl Instrumentation { let traced_fn = self.new_fn(traced_body, vec![]); - let ch_ident = ident!(format!("tr_ch_apm${}", &self.config.channel_name)); let trace_ident = ident!(format!( "tr_ch_apm${}.{}", From f6253c434cfdf7fe5c34dc24689d68509df70635 Mon Sep 17 00:00:00 2001 From: simon-id Date: Fri, 13 Jun 2025 16:09:21 +0200 Subject: [PATCH 5/7] lint --- src/instrumentation.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/instrumentation.rs b/src/instrumentation.rs index 4ca4ac6..91f6e52 100644 --- a/src/instrumentation.rs +++ b/src/instrumentation.rs @@ -8,8 +8,8 @@ use swc_core::common::{Span, SyntaxContext}; use swc_core::ecma::{ ast::{ ArrowExpr, AssignExpr, AssignTarget, BlockStmt, ClassDecl, ClassMethod, Constructor, Expr, - FnDecl, FnExpr, Ident, Lit, MemberProp, MethodProp, Module, ModuleItem, Param, Pat, PropName, - Script, SimpleAssignTarget, Stmt, Str, VarDecl, + FnDecl, FnExpr, Ident, Lit, MemberProp, MethodProp, Module, ModuleItem, Param, Pat, + PropName, Script, SimpleAssignTarget, Stmt, Str, VarDecl, }, atoms::Atom, }; From c91fd521da548ecf2f359c58fb8219b3c88a8c66 Mon Sep 17 00:00:00 2001 From: simon-id Date: Wed, 16 Jul 2025 11:49:49 +0800 Subject: [PATCH 6/7] use borrow instead of clone --- src/instrumentation.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/instrumentation.rs b/src/instrumentation.rs index 91f6e52..f8703fa 100644 --- a/src/instrumentation.rs +++ b/src/instrumentation.rs @@ -81,7 +81,7 @@ impl Instrumentation { define_channel } - fn insert_tracing(&mut self, body: &mut BlockStmt, params: Vec) { + fn insert_tracing(&mut self, body: &mut BlockStmt, params: &[Param]) { self.count += 1; let original_stmts = std::mem::take(&mut body.stmts); @@ -93,7 +93,7 @@ impl Instrumentation { ..body.clone() }; - let original_params: Vec = params.into_iter().map(|p| p.pat).collect(); + let original_params: Vec = params.into_iter().map(|p| p.pat.clone()).collect(); let wrapped_fn = self.new_fn(original_body, original_params); @@ -182,7 +182,7 @@ impl Instrumentation { && func_expr.function.body.is_some() { if let Some(body) = func_expr.function.body.as_mut() { - self.insert_tracing(body, func_expr.function.params.clone()); + self.insert_tracing(body, &func_expr.function.params); } true } else { @@ -220,7 +220,7 @@ impl Instrumentation { && node.function.body.is_some() { if let Some(body) = node.function.body.as_mut() { - self.insert_tracing(body, node.function.params.clone()); + self.insert_tracing(body, &node.function.params); } } false @@ -267,7 +267,7 @@ impl Instrumentation { && node.function.body.is_some() { if let Some(body) = node.function.body.as_mut() { - self.insert_tracing(body, node.function.params.clone()); + self.insert_tracing(body, &node.function.params); } } true @@ -300,7 +300,7 @@ impl Instrumentation { && node.function.body.is_some() { if let Some(body) = node.function.body.as_mut() { - self.insert_tracing(body, node.function.params.clone()); + self.insert_tracing(body, &node.function.params); } } false From 427bf8a549215fc4eaee66b91c09ba5c9efc94b6 Mon Sep 17 00:00:00 2001 From: simon-id Date: Wed, 16 Jul 2025 11:57:51 +0800 Subject: [PATCH 7/7] change into_iter() to iter() --- src/instrumentation.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/instrumentation.rs b/src/instrumentation.rs index f8703fa..4baa639 100644 --- a/src/instrumentation.rs +++ b/src/instrumentation.rs @@ -93,7 +93,7 @@ impl Instrumentation { ..body.clone() }; - let original_params: Vec = params.into_iter().map(|p| p.pat.clone()).collect(); + let original_params: Vec = params.iter().map(|p| p.pat.clone()).collect(); let wrapped_fn = self.new_fn(original_body, original_params);