- give span names enough context that you don't need to see the whole…

… namespace to know what they're about. Is this a good idea?

- better formatting for long #[instrument(...)] lines
- .unwrap_or_default() instead of .unwrap_or("")
- .bucket() instead of .bucket.as_deref() wherever possible

aws-s3-transfer-manager/src/operation/download.rs

@@ -59,8 +59,8 @@ impl Download {
         let parent_span_for_tasks = tracing::debug_span!(
             parent: existing_span_for_tasks,
             "download-tasks",
-            bucket = input.bucket.as_deref().unwrap_or(""),
-            key = input.key.as_deref().unwrap_or(""),
+            bucket = input.bucket().unwrap_or_default(),
+            key = input.key().unwrap_or_default(),
         );
         parent_span_for_tasks.follows_from(tracing::Span::current());
 
@@ -69,12 +69,12 @@ impl Download {
             .handle
             .scheduler
             .acquire_permit()
-            .instrument(tracing::debug_span!("acquire-permit"))
+            .instrument(tracing::debug_span!("download-discovery-permit"))
             .await?;
 
         // make initial discovery about the object size, metadata, possibly first chunk
         let mut discovery = discover_obj(&ctx, &input)
-            .instrument(tracing::debug_span!("discovery"))
+            .instrument(tracing::debug_span!("download-discovery"))
             .await?;
         let (comp_tx, comp_rx) = mpsc::channel(concurrency);
 

aws-s3-transfer-manager/src/operation/download/builders.rs

@@ -22,7 +22,10 @@ impl DownloadFluentBuilder {
     }
 
     /// Initiate a download transfer for a single object
-    #[tracing::instrument(skip_all, level="debug", fields(bucket=self.inner.bucket.as_deref().unwrap_or(""), key=self.inner.key.as_deref().unwrap_or("")))]
+    #[tracing::instrument(skip_all, level = "debug", name = "download-initial-send", fields(
+        bucket = self.inner.bucket.as_deref().unwrap_or_default(),
+        key = self.inner.key.as_deref().unwrap_or_default(),
+    ))]
     pub async fn send(self) -> Result<DownloadHandle, crate::error::Error> {
         let input = self.inner.build()?;
         crate::operation::download::Download::orchestrate(self.handle, input, None).await

aws-s3-transfer-manager/src/operation/download/handle.rs

@@ -45,7 +45,7 @@ impl DownloadHandle {
     }
 
     /// Consume the handle and wait for download transfer to complete
-    #[tracing::instrument(skip_all, level = "debug")]
+    #[tracing::instrument(skip_all, level = "debug", name = "download-join")]
     pub async fn join(mut self) -> Result<(), crate::error::Error> {
         self.body.close();
         while let Some(join_result) = self.tasks.join_next().await {

aws-s3-transfer-manager/src/operation/download/service.rs

@@ -75,7 +75,7 @@ async fn download_specific_chunk(
 
     let bytes = body
         .collect()
-        .instrument(tracing::debug_span!("collect-body", seq = seq))
+        .instrument(tracing::debug_span!("download-chunk-collect-body", seq))
         .await
         .map_err(error::from_kind(error::ErrorKind::ChunkFailed))?;
 

aws-s3-transfer-manager/src/operation/download_objects.rs

@@ -47,8 +47,9 @@ impl DownloadObjects {
         let parent_span_for_tasks = tracing::debug_span!(
             parent: None,
             "download-objects-tasks",
-            bucket=input.bucket.as_deref().unwrap_or(""),
-            key_prefix=input.key_prefix.as_deref().unwrap_or(""),
+            bucket = input.bucket().unwrap_or_default(),
+            destination = input.destination().map(|p| p.to_str().unwrap_or_default()).unwrap_or_default(),
+            key_prefix = input.key_prefix().unwrap_or_default(),
         );
         parent_span_for_tasks.follows_from(tracing::Span::current());
 

aws-s3-transfer-manager/src/operation/download_objects/builders.rs

@@ -24,7 +24,11 @@ impl DownloadObjectsFluentBuilder {
     }
 
     /// Initiate a download transfer for multiple objects
-    #[tracing::instrument(skip_all, level="debug", fields(bucket=self.inner.bucket.as_deref().unwrap_or(""), key_prefix=self.inner.key_prefix.as_deref().unwrap_or("")))]
+    #[tracing::instrument(skip_all, level = "debug", name = "download-objects-initial-send", fields(
+        bucket = self.inner.bucket.as_deref().unwrap_or_default(),
+        destination = self.inner.destination.as_deref().map(|p| p.to_str().unwrap_or_default()).unwrap_or_default(),
+        key_prefix = self.inner.key_prefix.as_deref().unwrap_or_default(),
+    ))]
     pub async fn send(self) -> Result<DownloadObjectsHandle, crate::error::Error> {
         let input = self.inner.build()?;
         crate::operation::download_objects::DownloadObjects::orchestrate(self.handle, input).await

aws-s3-transfer-manager/src/operation/download_objects/handle.rs

@@ -21,7 +21,7 @@ pub struct DownloadObjectsHandle {
 
 impl DownloadObjectsHandle {
     /// Consume the handle and wait for download transfer to complete
-    #[tracing::instrument(skip_all, level = "debug")]
+    #[tracing::instrument(skip_all, level = "debug", name = "download-objects-join")]
     pub async fn join(mut self) -> Result<DownloadObjectsOutput, crate::error::Error> {
         // join all tasks
         while let Some(join_result) = self.tasks.join_next().await {

aws-s3-transfer-manager/src/operation/upload/builders.rs

@@ -25,7 +25,10 @@ impl UploadFluentBuilder {
     }
 
     /// Initiate an upload transfer for a single object
-    #[tracing::instrument(skip_all, level="debug", fields(bucket=self.inner.bucket.as_deref().unwrap_or(""), key=self.inner.key.as_deref().unwrap_or("")))]
+    #[tracing::instrument(skip_all, level = "debug", name = "upload-initial-send", fields(
+        bucket = self.inner.bucket.as_deref().unwrap_or_default(),
+        key = self.inner.key.as_deref().unwrap_or_default(),
+    ))]
     pub async fn send(self) -> Result<UploadHandle, crate::error::Error> {
         let input = self.inner.build()?;
         crate::operation::upload::Upload::orchestrate(self.handle, input).await

aws-s3-transfer-manager/src/operation/upload/handle.rs

@@ -37,21 +37,21 @@ impl UploadHandle {
     pub(crate) fn new(ctx: UploadContext) -> Self {
         let parent_span_for_all_tasks = tracing::debug_span!(
             parent: None, "upload-tasks", // TODO: for upload_objects, parent should be upload-objects-tasks
-            bucket = ctx.request.bucket.as_deref().unwrap_or(""),
-            key = ctx.request.key.as_deref().unwrap_or("")
+            bucket = ctx.request.bucket.as_deref().unwrap_or_default(),
+            key = ctx.request.key.as_deref().unwrap_or_default(),
         );
         parent_span_for_all_tasks.follows_from(tracing::Span::current());
 
         // group read tasks together
         let parent_span_for_read_tasks = tracing::debug_span!(
             parent: parent_span_for_all_tasks.clone(),
-            "upload-read-tasks",
+            "upload-read-tasks"
         );
 
         // group upload tasks together
         let parent_span_for_upload_tasks = tracing::debug_span!(
             parent: parent_span_for_all_tasks,
-            "upload-net-tasks",
+            "upload-net-tasks"
         );
 
         Self {
@@ -78,13 +78,13 @@ impl UploadHandle {
     }
 
     /// Consume the handle and wait for upload to complete
-    #[tracing::instrument(skip_all, level = "debug")]
+    #[tracing::instrument(skip_all, level = "debug", name = "upload-join")]
     pub async fn join(self) -> Result<UploadOutput, crate::error::Error> {
         complete_upload(self).await
     }
 
     /// Abort the upload and cancel any in-progress part uploads.
-    #[tracing::instrument(skip_all, level = "debug")]
+    #[tracing::instrument(skip_all, level = "debug", name = "upload-abort")]
     pub async fn abort(&mut self) -> Result<AbortedUpload, crate::error::Error> {
         // TODO(aws-sdk-rust#1159) - handle already completed upload
 

aws-s3-transfer-manager/src/operation/upload/service.rs

@@ -139,7 +139,7 @@ pub(super) async fn read_body(
 ) -> Result<(), error::Error> {
     while let Some(part_data) = part_reader
         .next_part()
-        .instrument(tracing::debug_span!("read-next-part"))
+        .instrument(tracing::debug_span!("upload-read-next-part"))
         .await?
     {
         let req = UploadPartRequest {

aws-s3-transfer-manager/src/operation/upload_objects/builders.rs

@@ -28,7 +28,11 @@ impl UploadObjectsFluentBuilder {
     }
 
     /// Initiate upload of multiple objects
-    #[tracing::instrument(skip_all, level = "debug")]
+    #[tracing::instrument(skip_all, level = "debug", name = "upload-objects-initial-send", fields(
+        bucket = self.inner.bucket.as_deref().unwrap_or_default(),
+        source = self.inner.source.as_deref().map(|p| p.to_str().unwrap_or_default()).unwrap_or_default(),
+        key_prefix = self.inner.key_prefix.as_deref().unwrap_or_default(),
+    ))]
     pub async fn send(self) -> Result<UploadObjectsHandle, UploadObjectsError> {
         // FIXME - Err(UploadObjectsError) instead of .expect()
         let input = self.inner.build().expect("valid input");

aws-s3-transfer-manager/src/operation/upload_objects/handle.rs

@@ -12,7 +12,7 @@ pub struct UploadObjectsHandle {}
 
 impl UploadObjectsHandle {
     /// Consume the handle and wait for the upload to complete
-    #[tracing::instrument(skip_all, level = "debug")]
+    #[tracing::instrument(skip_all, level = "debug", name = "upload-objects-join")]
     pub async fn join(self) -> Result<UploadObjectsOutput, UploadObjectsError> {
         unimplemented!()
     }