Skip to content

Commit

Permalink
Add instrumentation support for develop (#2019)
Browse files Browse the repository at this point in the history
* Add instrumentation support for develop

* Fix build without default features

---------

Co-authored-by: messense <[email protected]>
  • Loading branch information
konstin and messense authored Apr 2, 2024
1 parent 25be7f9 commit a52843a
Show file tree
Hide file tree
Showing 9 changed files with 39 additions and 9 deletions.
4 changes: 4 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,10 @@ faster-tests = []
# Deprecated features, keep it now for compatibility
human-panic = []

[profile.profiling]
inherits = "release"
debug = true

# Without this, compressing the .gz archive becomes notably slow for debug builds
[profile.dev.package.miniz_oxide]
opt-level = 3
Expand Down
2 changes: 2 additions & 0 deletions src/build_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use std::fmt::{Display, Formatter};
use std::io;
use std::path::{Path, PathBuf};
use std::str::FromStr;
use tracing::instrument;

/// The way the rust code is used in the wheel
#[derive(Clone, Debug, PartialEq, Eq)]
Expand Down Expand Up @@ -199,6 +200,7 @@ pub type BuiltWheelMetadata = (PathBuf, String);
impl BuildContext {
/// Checks which kind of bindings we have (pyo3/rust-cypthon or cffi or bin) and calls the
/// correct builder.
#[instrument(skip_all)]
pub fn build_wheels(&self) -> Result<Vec<BuiltWheelMetadata>> {
use itertools::Itertools;

Expand Down
3 changes: 2 additions & 1 deletion src/build_options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use std::collections::{HashMap, HashSet};
use std::env;
use std::ops::{Deref, DerefMut};
use std::path::PathBuf;
use tracing::debug;
use tracing::{debug, instrument};

// This is used for BridgeModel::Bindings("pyo3-ffi") and BridgeModel::Bindings("pyo3").
// These should be treated almost identically but must be correctly identified
Expand Down Expand Up @@ -481,6 +481,7 @@ impl BuildOptions {
}

/// Tries to fill the missing metadata for a BuildContext by querying cargo and python
#[instrument(skip_all)]
pub fn into_build_context(
self,
release: bool,
Expand Down
3 changes: 2 additions & 1 deletion src/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use std::io::{BufReader, Read};
use std::path::{Path, PathBuf};
use std::process::{Command, Stdio};
use std::str;
use tracing::{debug, trace};
use tracing::{debug, instrument, trace};

/// The first version of pyo3 that supports building Windows abi3 wheel
/// without `PYO3_NO_PYTHON` environment variable
Expand Down Expand Up @@ -577,6 +577,7 @@ fn compile_target(
/// to import the module with error if it's missing or named incorrectly
///
/// Currently the check is only run on linux, macOS and Windows
#[instrument(skip_all)]
pub fn warn_missing_py_init(artifact: &Path, module_name: &str) -> Result<()> {
let py_init = format!("PyInit_{module_name}");
let mut fd = File::open(artifact)?;
Expand Down
4 changes: 4 additions & 0 deletions src/develop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use std::path::Path;
use std::path::PathBuf;
use std::process::Command;
use tempfile::TempDir;
use tracing::instrument;
use url::Url;

/// Install the crate as module in the current virtualenv
Expand Down Expand Up @@ -76,6 +77,7 @@ fn make_pip_command(python_path: &Path, pip_path: Option<&Path>) -> Command {
}
}

#[instrument(skip_all)]
fn install_dependencies(
build_context: &BuildContext,
extras: &[String],
Expand Down Expand Up @@ -121,6 +123,7 @@ fn install_dependencies(
Ok(())
}

#[instrument(skip_all, fields(wheel_filename = %wheel_filename.display()))]
fn pip_install_wheel(
build_context: &BuildContext,
python: &Path,
Expand Down Expand Up @@ -166,6 +169,7 @@ fn pip_install_wheel(
/// When a maturin package is installed using `pip install -e`, pip takes care of writing the
/// correct URL, however when a maturin package is installed with `maturin develop`, the URL is
/// set to the path to the temporary wheel file created during installation.
#[instrument(skip_all)]
fn fix_direct_url(
build_context: &BuildContext,
python: &Path,
Expand Down
23 changes: 19 additions & 4 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ use maturin::{generate_json_schema, GenerateJsonSchemaOptions};
use maturin::{upload_ui, PublishOpt};
use std::env;
use std::path::PathBuf;
use tracing::debug;
use tracing::{debug, instrument};
#[cfg(feature = "log")]
use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt};

#[derive(Debug, Parser)]
#[command(
Expand Down Expand Up @@ -304,10 +306,8 @@ fn pep517(subcommand: Pep517Command) -> Result<()> {
Ok(())
}

#[instrument]
fn run() -> Result<()> {
#[cfg(feature = "log")]
tracing_subscriber::fmt::init();

#[cfg(feature = "zig")]
{
// Allow symlink `maturin` to `ar` to invoke `zig ar`
Expand Down Expand Up @@ -464,6 +464,21 @@ fn main() {
#[cfg(not(debug_assertions))]
setup_panic_hook();

#[cfg(feature = "log")]
{
let logger = tracing_subscriber::fmt::layer()
// Avoid showing all the details from the spans
.compact()
// Log the timing of each span
.with_span_events(tracing_subscriber::fmt::format::FmtSpan::CLOSE);

tracing_subscriber::registry()
// `RUST_LOG` support
.with(tracing_subscriber::EnvFilter::from_default_env())
.with(logger)
.init();
}

if let Err(e) = run() {
eprintln!("💥 maturin failed");
for cause in e.chain() {
Expand Down
3 changes: 2 additions & 1 deletion src/module_writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use std::path::{Path, PathBuf};
use std::process::{Command, Output};
use std::str;
use tempfile::{tempdir, TempDir};
use tracing::debug;
use tracing::{debug, instrument};
use zip::{self, DateTime, ZipWriter};

/// Allows writing the module to a wheel or add it directly to the virtualenv
Expand Down Expand Up @@ -706,6 +706,7 @@ fn handle_cffi_call_result(

/// Copies the shared library into the module, which is the only extra file needed with bindings
#[allow(clippy::too_many_arguments)]
#[instrument(skip_all)]
pub fn write_bindings_module(
writer: &mut impl ModuleWriter,
project_layout: &ProjectLayout,
Expand Down
3 changes: 2 additions & 1 deletion src/project_layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::collections::HashSet;
use std::env;
use std::io;
use std::path::{Path, PathBuf};
use tracing::debug;
use tracing::{debug, instrument};

const PYPROJECT_TOML: &str = "pyproject.toml";

Expand Down Expand Up @@ -312,6 +312,7 @@ impl ProjectResolver {
}
}

#[instrument(skip_all)]
fn resolve_cargo_metadata(
manifest_path: &Path,
cargo_options: &CargoOptions,
Expand Down
3 changes: 2 additions & 1 deletion src/python_interpreter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use std::ops::Deref;
use std::path::{Path, PathBuf};
use std::process::{Command, Stdio};
use std::str::{self, FromStr};
use tracing::debug;
use tracing::{debug, instrument};

mod config;

Expand Down Expand Up @@ -573,6 +573,7 @@ impl PythonInterpreter {

/// Checks whether the given command is a python interpreter and returns a
/// [PythonInterpreter] if that is the case
#[instrument(skip_all, fields(executable = %executable.as_ref().display()))]
pub fn check_executable(
executable: impl AsRef<Path>,
target: &Target,
Expand Down

0 comments on commit a52843a

Please sign in to comment.