Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add more instrumentation for performance tracing #2436

Merged
merged 1 commit into from
Mar 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading