add more instrumentation for performance tracing

Author: syphar

src/storage/mod.rs

@@ -134,20 +134,23 @@ impl AsyncStorage {
         })
     }
 
+    #[instrument]
     pub(crate) async fn exists(&self, path: &str) -> Result<bool> {
         match &self.backend {
             StorageBackend::Database(db) => db.exists(path).await,
             StorageBackend::S3(s3) => s3.exists(path).await,
         }
     }
 
+    #[instrument]
     pub(crate) async fn get_public_access(&self, path: &str) -> Result<bool> {
         match &self.backend {
             StorageBackend::Database(db) => db.get_public_access(path).await,
             StorageBackend::S3(s3) => s3.get_public_access(path).await,
         }
     }
 
+    #[instrument]
     pub(crate) async fn set_public_access(&self, path: &str, public: bool) -> Result<()> {
         match &self.backend {
             StorageBackend::Database(db) => db.set_public_access(path, public).await,
@@ -227,6 +230,7 @@ impl AsyncStorage {
         })
     }
 
+    #[instrument]
     pub(crate) async fn rustdoc_file_exists(
         &self,
         name: &str,
@@ -272,6 +276,7 @@ impl AsyncStorage {
         }
     }
 
+    #[instrument]
     pub(crate) async fn get(&self, path: &str, max_size: usize) -> Result<Blob> {
         let mut blob = match &self.backend {
             StorageBackend::Database(db) => db.get(path, max_size, None).await,
@@ -284,6 +289,7 @@ impl AsyncStorage {
         Ok(blob)
     }
 
+    #[instrument]
     pub(super) async fn get_range(
         &self,
         path: &str,

src/web/crate_details.rs

@@ -97,6 +97,7 @@ pub struct Release {
 }
 
 impl CrateDetails {
+    #[tracing::instrument(skip(conn))]
     pub async fn new(
         conn: &mut sqlx::PgConnection,
         name: &str,

src/web/mod.rs

@@ -203,10 +203,6 @@ impl MatchedRelease {
         self.release.version
     }
 
-    fn id(&self) -> i32 {
-        self.release.id
-    }
-
     fn version(&self) -> &Version {
         &self.release.version
     }
@@ -232,6 +228,7 @@ impl MatchedRelease {
 /// This function will also check for crates where dashes in the name (`-`) have been replaced with
 /// underscores (`_`) and vice-versa. The return value will indicate whether the crate name has
 /// been matched exactly, or if there has been a "correction" in the name that matched instead.
+#[instrument(skip(conn))]
 async fn match_version(
     conn: &mut sqlx::PgConnection,
     name: &str,

src/web/rustdoc.rs

@@ -34,7 +34,7 @@ use std::{
     collections::{BTreeMap, HashMap},
     sync::Arc,
 };
-use tracing::{debug, error, instrument, trace};
+use tracing::{debug, error, info_span, instrument, trace, Instrument};
 
 static DOC_RUST_LANG_ORG_REDIRECTS: Lazy<HashMap<&str, &str>> = Lazy::new(|| {
     HashMap::from([
@@ -126,7 +126,9 @@ pub(crate) async fn rustdoc_redirector_handler(
             .is_ok()
         {
             rendering_time.step("serve static asset");
-            return try_serve_legacy_toolchain_asset(storage, config, params.name).await;
+            return try_serve_legacy_toolchain_asset(storage, config, params.name)
+                .instrument(info_span!("serve static asset"))
+                .await;
         }
     }
 
@@ -176,84 +178,77 @@ pub(crate) async fn rustdoc_redirector_handler(
         if target.ends_with(".js") {
             // this URL is actually from a crate-internal path, serve it there instead
             rendering_time.step("serve JS for crate");
-            let version = matched_release.into_version();
 
-            let krate = CrateDetails::new(&mut conn, &crate_name, &version, params.version.clone())
-                .await?
-                .ok_or(AxumNope::ResourceNotFound)?;
+            return async {
+                let version = matched_release.into_version();
 
-            rendering_time.step("fetch from storage");
+                let krate =
+                    CrateDetails::new(&mut conn, &crate_name, &version, params.version.clone())
+                        .await?
+                        .ok_or(AxumNope::ResourceNotFound)?;
 
-            match storage
-                .fetch_rustdoc_file(
-                    &crate_name,
-                    &version.to_string(),
-                    krate.latest_build_id.unwrap_or(0),
-                    target,
-                    krate.archive_storage,
-                    Some(&mut rendering_time),
-                )
-                .await
-            {
-                Ok(blob) => return Ok(File(blob).into_response()),
-                Err(err) => {
-                    if !matches!(err.downcast_ref(), Some(AxumNope::ResourceNotFound))
-                        && !matches!(err.downcast_ref(), Some(crate::storage::PathNotFoundError))
-                    {
-                        debug!(?target, ?err, "got error serving file");
-                    }
-                    // FIXME: we sometimes still get requests for toolchain
-                    // specific static assets under the crate/version/ path.
-                    // This is fixed in rustdoc, but pending a rebuild for
-                    // docs that were affected by this bug.
-                    // https://github.com/rust-lang/docs.rs/issues/1979
-                    if target.starts_with("search-") || target.starts_with("settings-") {
-                        return try_serve_legacy_toolchain_asset(storage, config, target).await;
-                    } else {
-                        return Err(err.into());
+                rendering_time.step("fetch from storage");
+
+                match storage
+                    .fetch_rustdoc_file(
+                        &crate_name,
+                        &version.to_string(),
+                        krate.latest_build_id.unwrap_or(0),
+                        target,
+                        krate.archive_storage,
+                        Some(&mut rendering_time),
+                    )
+                    .await
+                {
+                    Ok(blob) => Ok(File(blob).into_response()),
+                    Err(err) => {
+                        if !matches!(err.downcast_ref(), Some(AxumNope::ResourceNotFound))
+                            && !matches!(
+                                err.downcast_ref(),
+                                Some(crate::storage::PathNotFoundError)
+                            )
+                        {
+                            debug!(?target, ?err, "got error serving file");
+                        }
+                        // FIXME: we sometimes still get requests for toolchain
+                        // specific static assets under the crate/version/ path.
+                        // This is fixed in rustdoc, but pending a rebuild for
+                        // docs that were affected by this bug.
+                        // https://github.com/rust-lang/docs.rs/issues/1979
+                        if target.starts_with("search-") || target.starts_with("settings-") {
+                            try_serve_legacy_toolchain_asset(storage, config, target).await
+                        } else {
+                            Err(err.into())
+                        }
                     }
                 }
             }
+            .instrument(info_span!("serve JS for crate"))
+            .await;
         }
     }
 
     let matched_release = matched_release.into_canonical_req_version();
 
-    // get target name and whether it has docs
-    // FIXME: This is a bit inefficient but allowing us to use less code in general
-    rendering_time.step("fetch release doc status");
-    let (target_name, has_docs): (String, bool) = {
-        let row = sqlx::query!(
-            "SELECT
-                 target_name,
-                 rustdoc_status
-             FROM releases
-             WHERE releases.id = $1",
-            matched_release.id(),
-        )
-        .fetch_one(&mut *conn)
-        .await?;
-
-        (row.target_name, row.rustdoc_status)
-    };
-
     let mut target = params.target.as_deref();
-    if target == Some("index.html") || target == Some(&target_name) {
+    if target == Some("index.html") || target == Some(matched_release.target_name()) {
         target = None;
     }
 
-    if has_docs {
+    if matched_release.rustdoc_status() {
         rendering_time.step("redirect to doc");
 
         let url_str = if let Some(target) = target {
             format!(
-                "/{crate_name}/{}/{target}/{target_name}/",
-                matched_release.req_version
+                "/{crate_name}/{}/{target}/{}/",
+                matched_release.req_version,
+                matched_release.target_name(),
             )
         } else {
             format!(
-                "/{crate_name}/{}/{target_name}/",
-                matched_release.req_version
+                "/{crate_name}/{}/{}/",
+                matched_release.req_version,
+                matched_release.target_name(),
             )
         };
 
@@ -663,6 +658,7 @@ pub(crate) async fn rustdoc_html_server_handler(
                 ))
             }
         })
+        .instrument(info_span!("rewrite html"))
         .await?
 }
 
@@ -731,6 +727,7 @@ fn path_for_version(
     (path, query_params)
 }
 
+#[instrument(skip_all)]
 pub(crate) async fn target_redirect_handler(
     Path((name, req_version, req_path)): Path<(String, ReqVersion, String)>,
     mut conn: DbConnection,
@@ -805,7 +802,7 @@ pub(crate) struct BadgeQueryParams {
     version: Option<ReqVersion>,
 }
 
-#[instrument]
+#[instrument(skip_all)]
 pub(crate) async fn badge_handler(
     Path(name): Path<String>,
     Query(query): Query<BadgeQueryParams>,
@@ -823,6 +820,7 @@ pub(crate) async fn badge_handler(
     ))
 }
 
+#[instrument(skip_all)]
 pub(crate) async fn download_handler(
     Path((name, req_version)): Path<(String, ReqVersion)>,
     mut conn: DbConnection,
@@ -866,6 +864,7 @@ pub(crate) async fn download_handler(
 /// Serves shared resources used by rustdoc-generated documentation.
 ///
 /// This serves files from S3, and is pointed to by the `--static-root-path` flag to rustdoc.
+#[instrument(skip_all)]
 pub(crate) async fn static_asset_handler(
     Path(path): Path<String>,
     Extension(storage): Extension<Arc<AsyncStorage>>,