Skip to content

Commit

Permalink
add more instrumentation for performance tracing
Browse files Browse the repository at this point in the history
  • Loading branch information
syphar committed Mar 1, 2024
1 parent e486388 commit 97c3c70
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 63 deletions.
6 changes: 6 additions & 0 deletions src/storage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -227,6 +230,7 @@ impl AsyncStorage {
})
}

#[instrument]
pub(crate) async fn rustdoc_file_exists(
&self,
name: &str,
Expand Down Expand Up @@ -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,
Expand All @@ -284,6 +289,7 @@ impl AsyncStorage {
Ok(blob)
}

#[instrument]
pub(super) async fn get_range(
&self,
path: &str,
Expand Down
1 change: 1 addition & 0 deletions src/web/crate_details.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ pub struct Release {
}

impl CrateDetails {
#[tracing::instrument(skip(conn))]
pub async fn new(
conn: &mut sqlx::PgConnection,
name: &str,
Expand Down
5 changes: 1 addition & 4 deletions src/web/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,10 +203,6 @@ impl MatchedRelease {
self.release.version
}

fn id(&self) -> i32 {
self.release.id
}

fn version(&self) -> &Version {
&self.release.version
}
Expand All @@ -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,
Expand Down
117 changes: 58 additions & 59 deletions src/web/rustdoc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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([
Expand Down Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -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(),
)
};

Expand Down Expand Up @@ -663,6 +658,7 @@ pub(crate) async fn rustdoc_html_server_handler(
))
}
})
.instrument(info_span!("rewrite html"))
.await?
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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>,
Expand All @@ -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,
Expand Down Expand Up @@ -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>>,
Expand Down

0 comments on commit 97c3c70

Please sign in to comment.