feat(components): Make ActionOutcome component-aware (#25684)

GitOrigin-RevId: 4866b0099a837a062a3209fa74a4d0c09ee90493

Author: Sujay Jayakar

crates/application/src/application_function_runner/mod.rs

@@ -305,10 +305,7 @@ impl<RT: Runtime> FunctionRouter<RT> {
             .await?;
 
         let FunctionOutcome::Action(outcome) = outcome else {
-            anyhow::bail!(
-                "Calling an action returned an invalid outcome: {:?}",
-                outcome
-            )
+            anyhow::bail!("Calling an action returned an invalid outcome")
         };
         Ok(outcome)
     }
@@ -1166,7 +1163,7 @@ impl<RT: Runtime> ApplicationFunctionRunner<RT> {
                         identity.into(),
                         self.runtime.clone(),
                         None,
-                    )?,
+                    ),
                     environment: ModuleEnvironment::Invalid,
                     memory_in_mb: 0,
                     execution_time: Duration::from_secs(0),
@@ -1323,10 +1320,9 @@ impl<RT: Runtime> ApplicationFunctionRunner<RT> {
                 )
                 .await;
                 timer.finish();
-                let udf_path = path.as_root_udf_path()?.clone();
                 node_outcome_result.map(|node_outcome| {
                     let outcome = ActionOutcome {
-                        udf_path,
+                        path: path.clone(),
                         arguments: arguments.clone(),
                         identity: tx.inert_identity(),
                         unix_timestamp,
@@ -1360,7 +1356,7 @@ impl<RT: Runtime> ApplicationFunctionRunner<RT> {
                     inert_identity,
                     self.runtime.clone(),
                     udf_server_version,
-                )?;
+                );
                 Ok(ActionCompletion {
                     outcome,
                     execution_time: start.elapsed(),

crates/application/src/function_log.rs

@@ -311,7 +311,8 @@ impl HeapSize for FunctionExecutionPart {
     }
 }
 
-#[derive(Debug, Clone)]
+#[derive(Clone)]
+#[cfg_attr(any(test, feature = "testing"), derive(Debug))]
 pub struct ActionCompletion {
     pub outcome: ActionOutcome,
     pub execution_time: Duration,
@@ -786,7 +787,7 @@ impl<RT: Runtime> FunctionExecutionLog<RT> {
         let unix_timestamp = self.rt.unix_timestamp();
         let completion = ActionCompletion {
             outcome: ActionOutcome {
-                udf_path: path.into_root_udf_path()?,
+                path,
                 arguments,
                 identity,
                 unix_timestamp,
@@ -807,7 +808,19 @@ impl<RT: Runtime> FunctionExecutionLog<RT> {
     }
 
     fn _log_action(&self, completion: ActionCompletion, usage: TrackUsage) {
-        let udf_path = completion.outcome.udf_path.clone();
+        let outcome = completion.outcome;
+        let log_lines = completion.log_lines;
+        let udf_path = match outcome.path.clone().into_root_udf_path() {
+            Ok(udf_path) => udf_path,
+            Err(_) => {
+                tracing::warn!(
+                    "Skipping logging non-root action: {:?}:{:?}",
+                    outcome.path.component,
+                    outcome.path.udf_path
+                );
+                return;
+            },
+        };
         let aggregated = match usage {
             TrackUsage::Track(usage_tracker) => {
                 let usage_stats = usage_tracker.gather_user_stats();
@@ -829,16 +842,13 @@ impl<RT: Runtime> FunctionExecutionLog<RT> {
         if udf_path.is_system() {
             return;
         }
-        let outcome = completion.outcome;
-        let log_lines = completion.log_lines;
-
         let execution = FunctionExecution {
             params: UdfParams::Function {
                 error: match outcome.result {
                     Ok(_) => None,
                     Err(e) => Some(e),
                 },
-                identifier: outcome.udf_path.clone(),
+                identifier: udf_path,
             },
             unix_timestamp: self.rt.unix_timestamp(),
             execution_timestamp: outcome.unix_timestamp,

crates/isolate/src/environment/action/mod.rs

@@ -617,7 +617,7 @@ impl<RT: Runtime> ActionEnvironment<RT> {
             result.as_ref().ok().and_then(|r| r.as_ref().ok()),
         )?;
         let outcome = ActionOutcome {
-            udf_path: path.into_root_udf_path()?,
+            path,
             arguments,
             unix_timestamp: start_unix_timestamp,
             identity: self.identity.into(),

crates/isolate/src/environment/action/outcome.rs

@@ -21,7 +21,6 @@ use pb::{
 #[cfg(any(test, feature = "testing"))]
 use proptest::prelude::*;
 use serde_json::Value as JsonValue;
-use sync_types::CanonicalizedUdfPath;
 use value::ConvexValue;
 
 use super::HttpActionResult;
@@ -34,10 +33,10 @@ use crate::{
     ValidatedPathAndArgs,
 };
 
-#[derive(Debug, Clone)]
-#[cfg_attr(any(test, feature = "testing"), derive(PartialEq))]
+#[derive(Clone)]
+#[cfg_attr(any(test, feature = "testing"), derive(Debug, PartialEq))]
 pub struct ActionOutcome {
-    pub udf_path: CanonicalizedUdfPath,
+    pub path: CanonicalizedComponentFunctionPath,
     pub arguments: ConvexArray,
     pub identity: InertIdentity,
 
@@ -59,16 +58,16 @@ impl ActionOutcome {
         identity: InertIdentity,
         rt: impl Runtime,
         udf_server_version: Option<semver::Version>,
-    ) -> anyhow::Result<Self> {
-        Ok(ActionOutcome {
-            udf_path: path.into_root_udf_path()?,
+    ) -> Self {
+        ActionOutcome {
+            path,
             arguments,
             identity,
             unix_timestamp: rt.unix_timestamp(),
             result: Err(js_error),
             syscall_trace: SyscallTrace::new(),
             udf_server_version,
-        })
+        }
     }
 
     pub(crate) fn from_proto(
@@ -92,7 +91,7 @@ impl ActionOutcome {
         };
         let (path, arguments, udf_server_version) = path_and_args.consume();
         Ok(Self {
-            udf_path: path.into_root_udf_path()?,
+            path,
             arguments,
             identity,
             unix_timestamp: unix_timestamp
@@ -110,7 +109,7 @@ impl TryFrom<ActionOutcome> for ActionOutcomeProto {
 
     fn try_from(
         ActionOutcome {
-            udf_path: _,
+            path: _,
             arguments: _,
             identity: _,
             unix_timestamp,
@@ -142,16 +141,16 @@ impl Arbitrary for ActionOutcome {
     fn arbitrary_with(_args: Self::Parameters) -> Self::Strategy {
         use proptest::prelude::*;
         (
-            any::<CanonicalizedUdfPath>(),
+            any::<CanonicalizedComponentFunctionPath>(),
             any::<ConvexArray>(),
             any::<InertIdentity>(),
             any::<UnixTimestamp>(),
             any::<Result<JsonPackedValue, JsError>>(),
             any::<SyscallTrace>(),
         )
             .prop_map(
-                |(udf_path, arguments, identity, unix_timestamp, result, syscall_trace)| Self {
-                    udf_path,
+                |(path, arguments, identity, unix_timestamp, result, syscall_trace)| Self {
+                    path,
                     arguments,
                     identity,
                     unix_timestamp,
@@ -228,12 +227,12 @@ mod tests {
         #[test]
         fn test_action_udf_outcome_roundtrips(udf_outcome in any::<ActionOutcome>()) {
             let udf_outcome_clone = udf_outcome.clone();
-            let udf_path = udf_outcome.udf_path.clone();
+            let path = udf_outcome.path.clone();
             let arguments = udf_outcome.arguments.clone();
             let version = udf_outcome.udf_server_version.clone();
             let identity = udf_outcome_clone.identity.clone();
             let path_and_args = ValidatedPathAndArgs::new_for_tests(
-                udf_path,
+                path,
                 arguments,
                 version
             );

crates/isolate/src/environment/helpers/outcome.rs

@@ -18,7 +18,8 @@ use crate::{
 /// A `UdfOutcome` represents a successful execution of a developer's function
 /// by our V8 layer. It slightly differs from `UdfExecution`, which is what we
 /// store in memory for logs.
-#[derive(Debug, Clone)]
+#[derive(Clone)]
+#[cfg_attr(any(test, feature = "testing"), derive(Debug))]
 pub enum FunctionOutcome {
     Query(UdfOutcome),
     Mutation(UdfOutcome),

crates/isolate/src/environment/helpers/validation.rs

@@ -330,15 +330,12 @@ impl ValidatedPathAndArgs {
 
     #[cfg(any(test, feature = "testing"))]
     pub fn new_for_tests(
-        udf_path: sync_types::CanonicalizedUdfPath,
+        path: CanonicalizedComponentFunctionPath,
         args: ConvexArray,
         npm_version: Option<Version>,
     ) -> Self {
         Self {
-            path: CanonicalizedComponentFunctionPath {
-                component: ComponentId::Root,
-                udf_path,
-            },
+            path,
             args,
             npm_version,
         }

crates/isolate/src/environment/udf/outcome.rs

@@ -252,6 +252,10 @@ impl UdfOutcome {
 
 #[cfg(test)]
 mod tests {
+    use common::components::{
+        CanonicalizedComponentFunctionPath,
+        ComponentId,
+    };
     use proptest::prelude::*;
 
     use super::{
@@ -273,7 +277,10 @@ mod tests {
             let version = udf_outcome.udf_server_version.clone();
             let identity = udf_outcome_clone.identity.clone();
             let path_and_args = ValidatedPathAndArgs::new_for_tests(
-                udf_path,
+                CanonicalizedComponentFunctionPath {
+                    component: ComponentId::Root,
+                    udf_path,
+                },
                 arguments,
                 version
             );

crates/isolate/src/test_helpers.rs

@@ -17,6 +17,7 @@ use common::{
         IndexMetadata,
     },
     components::{
+        CanonicalizedComponentFunctionPath,
         CanonicalizedComponentModulePath,
         ComponentFunctionPath,
         ComponentId,
@@ -710,7 +711,10 @@ impl<RT: Runtime, P: Persistence + Clone> UdfTest<RT, P> {
 
         let path: UdfPath = udf_path.parse()?;
         let path_and_args = ValidatedPathAndArgs::new_for_tests(
-            path.canonicalize(),
+            CanonicalizedComponentFunctionPath {
+                component: ComponentId::Root,
+                udf_path: path.canonicalize(),
+            },
             ConvexArray::try_from(vec![ConvexValue::Object(args)])?,
             Some(npm_version),
         );
@@ -1032,7 +1036,7 @@ impl<RT: Runtime, P: Persistence + Clone> UdfTest<RT, P> {
                         identity.into(),
                         self.rt.clone(),
                         None,
-                    )?,
+                    ),
                     vec![].into(),
                 ))
             },
@@ -1325,7 +1329,10 @@ pub async fn bogus_udf_request<RT: Runtime>(
     // let (sender, _rx) = oneshot::channel();
     let request = UdfRequest {
         path_and_args: ValidatedPathAndArgs::new_for_tests(
-            "path.js:default".parse()?,
+            CanonicalizedComponentFunctionPath {
+                component: ComponentId::Root,
+                udf_path: "path.js:default".parse()?,
+            },
             ConvexArray::empty(),
             None,
         ),