fix build performance tracing

Author: syphar

crates/bin/docs_rs_builder/src/docbuilder/rustwide_builder.rs

@@ -12,6 +12,7 @@ use docs_rs_database::{
     },
     service_config::{ConfigName, get_config, set_config},
 };
+use docs_rs_logging::BUILD_PACKAGE_TRANSACTION_NAME;
 use docs_rs_registry_api::RegistryApi;
 use docs_rs_repository_stats::RepositoryStatsUpdater;
 use docs_rs_rustdoc_json::{
@@ -26,7 +27,9 @@ use docs_rs_types::{
     BuildId, BuildStatus, CrateId, KrateName, ReleaseId, Version,
     doc_coverage::{self, DocCoverage},
 };
-use docs_rs_utils::{RUSTDOC_STATIC_STORAGE_PREFIX, retry, rustc_version::parse_rustc_version};
+use docs_rs_utils::{
+    Handle, RUSTDOC_STATIC_STORAGE_PREFIX, retry, rustc_version::parse_rustc_version,
+};
 use docsrs_metadata::{BuildTargets, DEFAULT_TARGETS, HOST_TARGET, Metadata};
 use regex::Regex;
 use rustwide::{
@@ -43,7 +46,6 @@ use std::{
     sync::Arc,
     time::Instant,
 };
-use tokio::runtime;
 use tracing::{debug, error, info, info_span, instrument, warn};
 
 const USER_AGENT: &str = "docs.rs builder (https://github.com/rust-lang/docs.rs)";
@@ -114,7 +116,7 @@ pub enum PackageKind<'a> {
 pub struct RustwideBuilder {
     workspace: Workspace,
     toolchain: Toolchain,
-    runtime: runtime::Handle,
+    runtime: Handle,
     config: Arc<Config>,
     db: Pool,
     blocking_storage: Arc<Storage>,
@@ -127,7 +129,8 @@ pub struct RustwideBuilder {
 
 impl RustwideBuilder {
     pub fn init(config: Arc<Config>, context: &Context) -> Result<Self> {
-        let toolchain = context.runtime().block_on(async {
+        let runtime: Handle = context.runtime().clone().into();
+        let toolchain = runtime.block_on(async {
             let mut conn = context.pool()?.get_async().await?;
             get_configured_toolchain(&mut conn).await
         })?;
@@ -137,7 +140,7 @@ impl RustwideBuilder {
             toolchain,
             config: config.clone(),
             db: context.pool()?.clone(),
-            runtime: context.runtime().clone(),
+            runtime,
             blocking_storage: context.blocking_storage()?.clone(),
             storage: context.storage()?.clone(),
             registry_api: context.registry_api()?.clone(),
@@ -487,7 +490,7 @@ impl RustwideBuilder {
         )
     }
 
-    #[instrument(name = "docbuilder.build_package", parent = None, skip(self, name), fields(krate=name))]
+    #[instrument(name = BUILD_PACKAGE_TRANSACTION_NAME, parent = None, skip(self, name), fields(krate=name))]
     pub fn build_package(
         &mut self,
         name: &str,
@@ -533,6 +536,7 @@ impl RustwideBuilder {
         }
     }
 
+    #[instrument(skip(self))]
     #[allow(clippy::too_many_arguments)]
     fn build_package_inner(
         &mut self,

crates/lib/docs_rs_logging/src/lib.rs

@@ -8,6 +8,15 @@ use sentry::{
 use std::{env, str::FromStr as _, sync::Arc};
 use tracing_subscriber::{EnvFilter, filter::Directive, prelude::*};
 
+/// defines the transaction name to be used for our rustwide builder.
+///
+/// We want to trace _all_ builds, while we want to apply a
+/// sampling ratio to web requests.
+///
+/// From what I see right now, the transaction name or op is the only way
+/// to distinguish build transactions from web requests.
+pub const BUILD_PACKAGE_TRANSACTION_NAME: &str = "docbuilder.build_package";
+
 pub struct Guard {
     #[allow(dead_code)]
     sentry_guard: Option<sentry::ClientInitGuard>,
@@ -54,8 +63,7 @@ pub fn init() -> anyhow::Result<Guard> {
                 return if sampled { 1.0 } else { 0.0 };
             }
 
-            let op = ctx.operation();
-            if op == "docbuilder.build_package" {
+            if ctx.name() == BUILD_PACKAGE_TRANSACTION_NAME {
                 // record all transactions for builds
                 1.
             } else {

crates/lib/docs_rs_utils/src/lib.rs

@@ -1,7 +1,10 @@
+mod runtime_ext;
 pub mod rustc_version;
 #[cfg(feature = "testing")]
 pub mod testing;
 
+pub use runtime_ext::Handle;
+
 use anyhow::{Context as _, Result};
 use std::fmt;
 use std::{panic, thread, time::Duration};

crates/lib/docs_rs_utils/src/runtime_ext.rs

@@ -0,0 +1,27 @@
+use std::ops::Deref;
+use tokio::runtime;
+use tracing::Instrument as _;
+
+/// Newtype around `tokio::runtime::Handle` that adds
+/// missing integration with tracing spans.
+pub struct Handle(runtime::Handle);
+
+impl Handle {
+    pub fn block_on<F: Future>(&self, future: F) -> F::Output {
+        runtime::Handle::block_on(self, future.in_current_span())
+    }
+}
+
+impl From<runtime::Handle> for Handle {
+    fn from(handle: runtime::Handle) -> Self {
+        Handle(handle)
+    }
+}
+
+impl Deref for Handle {
+    type Target = runtime::Handle;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}