Skip to content

Commit

Permalink
feat(resolve): improve error reporting
Browse files Browse the repository at this point in the history
  • Loading branch information
lquerel committed Feb 27, 2024
1 parent bf38910 commit cbb8e7a
Show file tree
Hide file tree
Showing 2 changed files with 101 additions and 73 deletions.
114 changes: 70 additions & 44 deletions crates/weaver_resolver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,38 +47,6 @@ mod tags;
/// All references to semantic conventions will be resolved.
pub struct SchemaResolver {}

/// Different types of unresolved references.
#[derive(Debug)]
pub enum UnresolvedReference {
/// An unresolved attribute reference.
AttributeRef {
/// The id of the group containing the attribute reference.
group_id: String,
/// The unresolved attribute reference.
attribute_ref: String,
/// The provenance of the reference (URL or path).
provenance: String,
},
/// An unresolved `extends` clause reference.
ExtendsRef {
/// The id of the group containing the `extends` clause reference.
group_id: String,
/// The unresolved `extends` clause reference.
extends_ref: String,
/// The provenance of the reference (URL or path).
provenance: String,
},
/// An unresolved `include` reference.
IncludeRef {
/// The id of the group containing the `include` reference.
group_id: String,
/// The unresolved `include` reference.
include_ref: String,
/// The provenance of the reference (URL or path).
provenance: String,
},
}

/// An error that can occur while resolving a telemetry schema.
#[derive(thiserror::Error, Debug)]
pub enum Error {
Expand Down Expand Up @@ -115,13 +83,6 @@ pub enum Error {
error: String,
},

/// Failed to resolve a set of references.
#[error("Failed to resolve the following references {refs:?}")]
UnresolvedReferences {
/// The list of unresolved references.
refs: Vec<UnresolvedReference>,
},

/// Failed to resolve a metric.
#[error("Failed to resolve the metric '{r#ref}'")]
FailToResolveMetric {
Expand All @@ -147,14 +108,53 @@ pub enum Error {
message: String,
},

/// The `any_of` constraints are unsatisfied for a group.
#[error("Some `any_of` constraints are unsatisfied for the group '{group_id}'.\nUnsatisfied `any_of` constraints: {any_of_constraints:#?}")]
UnsatisfiedAnyOfConstraints {
/// An unresolved attribute reference.
#[error("The following attribute reference is not resolved for the group '{group_id}'.\nAttribute reference: {attribute_ref}\nProvenance: {provenance}")]
UnresolvedAttributeRef {
/// The id of the group containing the attribute reference.
group_id: String,
/// The unresolved attribute reference.
attribute_ref: String,
/// The provenance of the reference (URL or path).
provenance: String,
},

/// An unresolved `extends` clause reference.
#[error("The following `extends` clause reference is not resolved for the group '{group_id}'.\n`extends` clause reference: {extends_ref}\nProvenance: {provenance}")]
UnresolvedExtendsRef {
/// The id of the group containing the `extends` clause reference.
group_id: String,
/// The unresolved `extends` clause reference.
extends_ref: String,
/// The provenance of the reference (URL or path).
provenance: String,
},

/// An unresolved `include` reference.
#[error("The following `include` reference is not resolved for the group '{group_id}'.\n`include` reference: {include_ref}\nProvenance: {provenance}")]
UnresolvedIncludeRef {
/// The id of the group containing the `include` reference.
group_id: String,
/// The unresolved `include` reference.
include_ref: String,
/// The provenance of the reference (URL or path).
provenance: String,
},

/// An `any_of` constraint that is not satisfied for a group.
#[error("The following `any_of` constraint is not satisfied for the group '{group_id}'.\n`any_of` constraint: {any_of:#?}\nMissing attributes: {missing_attributes:?}")]
UnsatisfiedAnyOfConstraint {
/// The id of the group containing the unsatisfied `any_of` constraint.
group_id: String,
/// The `any_of` constraints of the group that are not satisfied.
any_of_constraints: Vec<UnsatisfiedAnyOfConstraint>,
/// The `any_of` constraint that is not satisfied.
any_of: Constraint,
/// The detected missing attributes.
missing_attributes: Vec<String>,
},

/// A container for multiple errors.
#[error("{:?}", Error::format_errors(.0))]
CompoundError(Vec<Error>),
}

/// A constraint that is not satisfied and its missing attributes.
Expand All @@ -166,6 +166,32 @@ pub struct UnsatisfiedAnyOfConstraint {
pub missing_attributes: Vec<String>,
}

impl Error {
/// Creates a compound error from a list of errors.
/// Note: All compound errors are flattened.
pub fn compound_error(errors: Vec<Error>) -> Error {
Error::CompoundError(
errors
.into_iter()
.flat_map(|e| match e {
Error::CompoundError(errors) => errors,
e => vec![e],
})
.collect(),
)
}

/// Formats the given errors into a single string.
/// This used to render compound errors.
pub fn format_errors(errors: &[Error]) -> String {
errors
.iter()
.map(|e| e.to_string())
.collect::<Vec<String>>()
.join("\n\n")
}
}

impl SchemaResolver {
/// Loads a telemetry schema from an URL or a file and returns the resolved
/// schema.
Expand Down
60 changes: 31 additions & 29 deletions crates/weaver_resolver/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use crate::constraint::resolve_constraints;
use crate::metrics::resolve_instrument;
use crate::spans::resolve_span_kind;
use crate::stability::resolve_stability;
use crate::{Error, UnresolvedReference, UnsatisfiedAnyOfConstraint};
use crate::{Error, UnsatisfiedAnyOfConstraint};

/// A registry containing unresolved groups.
#[derive(Debug, Deserialize)]
Expand Down Expand Up @@ -84,11 +84,11 @@ pub fn resolve_semconv_registry(
// If the resolution process fails, then we build an error containing all
// the unresolved references.
if !all_refs_resolved {
let mut unresolved_refs = vec![];
let mut errors = vec![];
for group in ureg.groups.iter() {
// Collect unresolved `extends` references.
if let Some(extends) = group.group.extends.as_ref() {
unresolved_refs.push(UnresolvedReference::ExtendsRef {
errors.push(Error::UnresolvedExtendsRef {
group_id: group.group.id.clone(),
extends_ref: extends.clone(),
provenance: group.provenance.clone(),
Expand All @@ -97,7 +97,7 @@ pub fn resolve_semconv_registry(
// Collect unresolved `ref` attributes.
for attr in group.attributes.iter() {
if let AttributeSpec::Ref { r#ref, .. } = &attr.spec {
unresolved_refs.push(UnresolvedReference::AttributeRef {
errors.push(Error::UnresolvedAttributeRef {
group_id: group.group.id.clone(),
attribute_ref: r#ref.clone(),
provenance: group.provenance.clone(),
Expand All @@ -107,18 +107,16 @@ pub fn resolve_semconv_registry(
// Collect unresolved `include` constraints.
for constraint in group.group.constraints.iter() {
if let Some(include) = &constraint.include {
unresolved_refs.push(UnresolvedReference::IncludeRef {
errors.push(Error::UnresolvedIncludeRef {
group_id: group.group.id.clone(),
include_ref: include.clone(),
provenance: group.provenance.clone(),
});
}
}
}
if !unresolved_refs.is_empty() {
return Err(Error::UnresolvedReferences {
refs: unresolved_refs,
});
if !errors.is_empty() {
return Err(Error::CompoundError(errors));
}
}

Expand Down Expand Up @@ -162,14 +160,14 @@ pub fn check_any_of_constraints(
registry: &Registry,
attr_name_index: &[String],
) -> Result<(), Error> {
let mut unresolved_refs = vec![];
let mut errors = vec![];

for group in registry.groups.iter() {
// Build a list of attribute names for the group.
let mut group_attr_names = HashSet::new();
for attr_ref in group.attributes.iter() {
match attr_name_index.get(attr_ref.0 as usize) {
None => unresolved_refs.push(UnresolvedReference::AttributeRef {
None => errors.push(Error::UnresolvedAttributeRef {
group_id: group.id.clone(),
attribute_ref: attr_ref.0.to_string(),
provenance: group.provenance().to_string(),
Expand All @@ -180,17 +178,17 @@ pub fn check_any_of_constraints(
}
}

check_group_any_of_constraints(
if let Err(e) = check_group_any_of_constraints(
group.id.as_ref(),
group_attr_names,
group.constraints.as_ref(),
)?;
) {
errors.push(e);
}
}

if !unresolved_refs.is_empty() {
return Err(Error::UnresolvedReferences {
refs: unresolved_refs,
});
if !errors.is_empty() {
return Err(Error::CompoundError(errors));
}

Ok(())
Expand All @@ -202,7 +200,8 @@ fn check_group_any_of_constraints(
group_attr_names: HashSet<String>,
constraints: &[Constraint],
) -> Result<(), Error> {
let mut unsatisfied_any_of_constraints: HashMap<&Constraint, Vec<String>> = HashMap::new();
let mut unsatisfied_any_of_constraints: HashMap<&Constraint, UnsatisfiedAnyOfConstraint> =
HashMap::new();

for constraint in constraints.iter() {
if constraint.any_of.is_empty() {
Expand All @@ -220,21 +219,24 @@ fn check_group_any_of_constraints(
// constraint.
unsatisfied_any_of_constraints
.entry(constraint)
.or_default()
.or_insert_with(|| UnsatisfiedAnyOfConstraint {
any_of: constraint.clone(),
missing_attributes: vec![],
})
.missing_attributes
.push(attr.clone());
}
}
if !unsatisfied_any_of_constraints.is_empty() {
return Err(Error::UnsatisfiedAnyOfConstraints {
group_id: group_id.to_string(),
any_of_constraints: unsatisfied_any_of_constraints
.into_iter()
.map(|(c, attrs)| UnsatisfiedAnyOfConstraint {
any_of: c.clone(),
missing_attributes: attrs,
})
.collect(),
});
let errors = unsatisfied_any_of_constraints
.into_values()
.map(|v| Error::UnsatisfiedAnyOfConstraint {
group_id: group_id.to_string(),
any_of: v.any_of,
missing_attributes: v.missing_attributes,
})
.collect();
return Err(Error::CompoundError(errors));
}
Ok(())
}
Expand Down

0 comments on commit cbb8e7a

Please sign in to comment.