From d3d88431f3fab17f545b459273d180f433a1b419 Mon Sep 17 00:00:00 2001
From: Florian Klink <flokli@flokli.de>
Date: Mon, 15 Apr 2024 13:52:55 +0300
Subject: feat(tvix/castore/fs): assign read[dir*]/release[dir] ops to parent
 span

When a directory or file is open()'ed, we already put some data into
a lookup table, and subsequent operations then use the returned handle
id.

By also adding the span that's been created during these calls into the
lookup table, we can properly set the span parent for these requests,
nicely connecting the individual operations to the bigger picture.

Change-Id: Ia354842fccdbc7f45c2d3efda3acf058b2dbc48e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11429
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: Brian Olsen <me@griff.name>
---
 tvix/castore/src/fs/mod.rs | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

(limited to 'tvix')

diff --git a/tvix/castore/src/fs/mod.rs b/tvix/castore/src/fs/mod.rs
index 3113d1e384..89a9986202 100644
--- a/tvix/castore/src/fs/mod.rs
+++ b/tvix/castore/src/fs/mod.rs
@@ -102,14 +102,21 @@ pub struct TvixStoreFs<BS, DS, RN> {
     /// This holds all opendir handles (for the root inode)
     /// They point to the rx part of the channel producing the listing.
     #[allow(clippy::type_complexity)]
-    dir_handles:
-        RwLock<HashMap<u64, Arc<Mutex<mpsc::Receiver<(usize, Result<Node, crate::Error>)>>>>>,
+    dir_handles: RwLock<
+        HashMap<
+            u64,
+            (
+                Span,
+                Arc<Mutex<mpsc::Receiver<(usize, Result<Node, crate::Error>)>>>,
+            ),
+        >,
+    >,
 
     next_dir_handle: AtomicU64,
 
     /// This holds all open file handles
     #[allow(clippy::type_complexity)]
-    file_handles: RwLock<HashMap<u64, Arc<Mutex<Box<dyn BlobReader>>>>>,
+    file_handles: RwLock<HashMap<u64, (Span, Arc<Mutex<Box<dyn BlobReader>>>)>>,
 
     next_file_handle: AtomicU64,
 
@@ -422,7 +429,7 @@ where
             debug!("add dir handle {}", dh);
             self.dir_handles
                 .write()
-                .insert(dh, Arc::new(Mutex::new(rx)));
+                .insert(dh, (Span::current(), Arc::new(Mutex::new(rx))));
 
             return Ok((
                 Some(dh),
@@ -433,7 +440,7 @@ where
         Ok((None, OpenOptions::empty()))
     }
 
-    #[tracing::instrument(skip_all, fields(rq.inode = inode, rq.handle = handle, rq.offset = offset))]
+    #[tracing::instrument(skip_all, fields(rq.inode = inode, rq.handle = handle, rq.offset = offset), parent = self.dir_handles.read().get(&handle).and_then(|x| x.0.id()))]
     fn readdir(
         &self,
         _ctx: &Context,
@@ -451,7 +458,7 @@ where
             }
 
             // get the handle from [self.dir_handles]
-            let rx = match self.dir_handles.read().get(&handle) {
+            let (_span, rx) = match self.dir_handles.read().get(&handle) {
                 Some(rx) => rx.clone(),
                 None => {
                     warn!("dir handle {} unknown", handle);
@@ -533,7 +540,7 @@ where
         Ok(())
     }
 
-    #[tracing::instrument(skip_all, fields(rq.inode = inode, rq.handle = handle))]
+    #[tracing::instrument(skip_all, fields(rq.inode = inode, rq.handle = handle), parent = self.dir_handles.read().get(&handle).and_then(|x| x.0.id()))]
     fn readdirplus(
         &self,
         _ctx: &Context,
@@ -554,7 +561,7 @@ where
             }
 
             // get the handle from [self.dir_handles]
-            let rx = match self.dir_handles.read().get(&handle) {
+            let (_span, rx) = match self.dir_handles.read().get(&handle) {
                 Some(rx) => rx.clone(),
                 None => {
                     warn!("dir handle {} unknown", handle);
@@ -657,7 +664,7 @@ where
         Ok(())
     }
 
-    #[tracing::instrument(skip_all, fields(rq.inode = inode, rq.handle = handle))]
+    #[tracing::instrument(skip_all, fields(rq.inode = inode, rq.handle = handle), parent = self.dir_handles.read().get(&handle).and_then(|x| x.0.id()))]
     fn releasedir(
         &self,
         _ctx: &Context,
@@ -728,7 +735,7 @@ where
                         debug!("add file handle {}", fh);
                         self.file_handles
                             .write()
-                            .insert(fh, Arc::new(Mutex::new(blob_reader)));
+                            .insert(fh, (Span::current(), Arc::new(Mutex::new(blob_reader))));
 
                         Ok((
                             Some(fh),
@@ -740,7 +747,7 @@ where
         }
     }
 
-    #[tracing::instrument(skip_all, fields(rq.inode = inode, rq.handle = handle))]
+    #[tracing::instrument(skip_all, fields(rq.inode = inode, rq.handle = handle), parent = self.file_handles.read().get(&handle).and_then(|x| x.0.id()))]
     fn release(
         &self,
         _ctx: &Context,
@@ -763,7 +770,7 @@ where
         Ok(())
     }
 
-    #[tracing::instrument(skip_all, fields(rq.inode = inode, rq.offset = offset, rq.size = size))]
+    #[tracing::instrument(skip_all, fields(rq.inode = inode, rq.offset = offset, rq.size = size), parent = self.file_handles.read().get(&handle).and_then(|x| x.0.id()))]
     fn read(
         &self,
         _ctx: &Context,
@@ -780,7 +787,7 @@ where
         // We need to take out the blob reader from self.file_handles, so we can
         // interact with it in the separate task.
         // On success, we pass it back out of the task, so we can put it back in self.file_handles.
-        let blob_reader = self
+        let (_span, blob_reader) = self
             .file_handles
             .read()
             .get(&handle)
-- 
cgit 1.4.1