Skip to content

Commit

Permalink
track a telemetry event when package discovery yields an invalid package
Browse files Browse the repository at this point in the history
  • Loading branch information
arlyon committed Feb 14, 2024
1 parent ad29683 commit 2746d65
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 5 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion crates/turborepo-lib/src/run/mod.rs
Expand Up @@ -284,7 +284,8 @@ impl Run {

let mut pkg_dep_graph = {
let builder = PackageGraph::builder(&self.base.repo_root, root_package_json.clone())
.with_single_package_mode(self.opts.run_opts.single_package);
.with_single_package_mode(self.opts.run_opts.single_package)
.with_telemetry(Some(run_telemetry.clone()));

#[cfg(feature = "daemon-package-discovery")]
let builder = {
Expand Down
1 change: 1 addition & 0 deletions crates/turborepo-repository/Cargo.toml
Expand Up @@ -27,6 +27,7 @@ turbopath = { workspace = true }
turborepo-graph-utils = { path = "../turborepo-graph-utils" }
turborepo-lockfiles = { workspace = true }
turborepo-scm = { workspace = true }
turborepo-telemetry = { version = "0.1.0", path = "../turborepo-telemetry" }
wax = { workspace = true }
which = { workspace = true }

Expand Down
39 changes: 35 additions & 4 deletions crates/turborepo-repository/src/package_graph/builder.rs
@@ -1,7 +1,7 @@
use std::{
backtrace::Backtrace,
collections::{BTreeMap, HashMap, HashSet},
fmt,
fmt, io,
};

use petgraph::graph::{Graph, NodeIndex};
Expand All @@ -12,6 +12,7 @@ use turbopath::{
};
use turborepo_graph_utils as graph;
use turborepo_lockfiles::Lockfile;
use turborepo_telemetry::events::{generic::GenericEventBuilder, TrackedErrors};

use super::{PackageGraph, WorkspaceInfo, WorkspaceName, WorkspaceNode};
use crate::{
Expand All @@ -20,7 +21,7 @@ use crate::{
PackageDiscoveryBuilder,
},
package_graph::{PackageName, PackageVersion},
package_json::PackageJson,
package_json::{self, PackageJson},
};

pub struct PackageGraphBuilder<'a, T> {
Expand All @@ -29,6 +30,7 @@ pub struct PackageGraphBuilder<'a, T> {
is_single_package: bool,
package_jsons: Option<HashMap<AbsoluteSystemPathBuf, PackageJson>>,
lockfile: Option<Box<dyn Lockfile>>,
telemetry: Option<GenericEventBuilder>,
package_discovery: T,
}

Expand Down Expand Up @@ -75,6 +77,7 @@ impl<'a> PackageGraphBuilder<'a, LocalPackageDiscoveryBuilder> {
is_single_package: false,
package_jsons: None,
lockfile: None,
telemetry: None,
}
}
}
Expand All @@ -94,6 +97,11 @@ impl<'a, P> PackageGraphBuilder<'a, P> {
self
}

pub fn with_telemetry(mut self, telemetry: Option<GenericEventBuilder>) -> Self {
self.telemetry = telemetry;
self
}

#[allow(dead_code)]
pub fn with_lockfile(mut self, lockfile: Option<Box<dyn Lockfile>>) -> Self {
self.lockfile = lockfile;
Expand All @@ -114,6 +122,7 @@ impl<'a, P> PackageGraphBuilder<'a, P> {
package_jsons: self.package_jsons,
lockfile: self.lockfile,
package_discovery: discovery,
telemetry: self.telemetry,
}
}
}
Expand Down Expand Up @@ -151,6 +160,7 @@ struct BuildState<'a, S, T> {
package_jsons: Option<HashMap<AbsoluteSystemPathBuf, PackageJson>>,
state: std::marker::PhantomData<S>,
package_discovery: T,
telemetry: Option<GenericEventBuilder>,
}

// Allows us to perform workspace discovery and parse package jsons
Expand Down Expand Up @@ -197,6 +207,7 @@ where
package_jsons,
lockfile,
package_discovery,
telemetry,
} = builder;
let mut workspaces = HashMap::new();
workspaces.insert(
Expand All @@ -221,6 +232,7 @@ where
package_discovery: CachingPackageDiscovery::new(
package_discovery.build().map_err(Into::into)?,
),
telemetry,
})
}
}
Expand Down Expand Up @@ -272,13 +284,28 @@ impl<'a, T: PackageDiscovery> BuildState<'a, ResolvedPackageManager, T> {
None => {
let mut jsons = HashMap::new();
for path in self.package_discovery.discover_packages().await?.workspaces {
let json = PackageJson::load(&path.package_json)?;
jsons.insert(path.package_json, json);
match PackageJson::load(&path.package_json) {
Ok(json) => {
jsons.insert(path.package_json, json);
}
// if we get here, it could stem from a package watch error, so we should
// fall back to the more expensive local discovery and log a telemetry event
Err(package_json::Error::Io(io))
if io.kind() == io::ErrorKind::NotFound =>
{
if let Some(telemetry) = self.telemetry {
telemetry.track_error(TrackedErrors::InvalidPackageDiscovery);
}
return Err(package_json::Error::Io(io).into());
}
Err(e) => return Err(e.into()),
};
}
Ok::<_, Error>(jsons)
}
}?;

// if package discovery produces
for (path, json) in package_jsons {
match self.add_json(path, json) {
Ok(()) => {}
Expand All @@ -302,6 +329,7 @@ impl<'a, T: PackageDiscovery> BuildState<'a, ResolvedPackageManager, T> {
node_lookup,
lockfile,
package_discovery,
telemetry,
..
} = self;
Ok(BuildState {
Expand All @@ -314,6 +342,7 @@ impl<'a, T: PackageDiscovery> BuildState<'a, ResolvedPackageManager, T> {
package_discovery,
package_jsons: None,
state: std::marker::PhantomData,
telemetry,
})
}

Expand Down Expand Up @@ -439,6 +468,7 @@ impl<'a, T: PackageDiscovery> BuildState<'a, ResolvedWorkspaces, T> {
workspace_graph,
node_lookup,
package_discovery,
telemetry,
..
} = self;
Ok(BuildState {
Expand All @@ -451,6 +481,7 @@ impl<'a, T: PackageDiscovery> BuildState<'a, ResolvedWorkspaces, T> {
package_jsons: None,
state: std::marker::PhantomData,
package_discovery,
telemetry,
})
}
}
Expand Down
6 changes: 6 additions & 0 deletions crates/turborepo-telemetry/src/events/mod.rs
Expand Up @@ -55,6 +55,11 @@ pub enum TrackedErrors {
ErrorFetchingFromCache,
FailedToPipeOutputs,
UnknownChildExit,
/// Yielded when package discovery yields a
/// list of packages that fails downstream.
/// Currently only indicates a package being
/// reported when it does not exist.
InvalidPackageDiscovery,
}

impl Display for TrackedErrors {
Expand All @@ -72,6 +77,7 @@ impl Display for TrackedErrors {
TrackedErrors::ErrorFetchingFromCache => write!(f, "error_fetching_from_cache"),
TrackedErrors::FailedToPipeOutputs => write!(f, "failed_to_pipe_outputs"),
TrackedErrors::UnknownChildExit => write!(f, "unknown_child_exit"),
TrackedErrors::InvalidPackageDiscovery => write!(f, "invalid_package_discovery"),
}
}
}
Expand Down

0 comments on commit 2746d65

Please sign in to comment.