Skip to content

Commit

Permalink
metrics don't require attribs
Browse files Browse the repository at this point in the history
  • Loading branch information
@jerbly
jerbly committed Nov 30, 2024
1 parent a223cc4 commit 12064c7
Show file tree
Hide file tree
Showing 4 changed files with 212 additions and 25 deletions.
150 changes: 148 additions & 2 deletions crates/weaver_semconv/src/group.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,21 @@ impl GroupSpec {
});
}

// All types, except metric and event, must have extends or attributes or both.
// For backward compatibility, if constraints are set, extends or attributes are not required.
if self.r#type != GroupType::Metric
&& self.r#type != GroupType::Event
&& self.extends.is_none()
&& self.attributes.is_empty()
&& self.constraints.is_empty()
{
errors.push(Error::InvalidGroupMissingExtendsOrAttributes {
path_or_url: path_or_url.to_owned(),
group_id: self.id.clone(),
error: "This group does not contain an extends or attributes field.".to_owned(),
});
}

// Fields span_kind and events are only valid if type is span (the default).
if self.r#type != GroupType::Span {
if self.span_kind.is_some() {
Expand Down Expand Up @@ -415,8 +430,8 @@ mod tests {
use crate::any_value::AnyValueCommonSpec;
use crate::attribute::{BasicRequirementLevelSpec, Examples, RequirementLevel};
use crate::Error::{
CompoundError, InvalidExampleWarning, InvalidGroup, InvalidGroupStability,
InvalidGroupUsesPrefix, InvalidMetric,
CompoundError, InvalidExampleWarning, InvalidGroup, InvalidGroupMissingExtendsOrAttributes,
InvalidGroupStability, InvalidGroupUsesPrefix, InvalidMetric,
};

use super::*;
Expand Down Expand Up @@ -979,6 +994,137 @@ mod tests {
.is_ok());
}

#[test]
fn test_validate_extends_or_attributes() {
let attributes = vec![AttributeSpec::Id {
id: "test".to_owned(),
r#type: AttributeType::PrimitiveOrArray(PrimitiveOrArrayTypeSpec::String),
brief: None,
stability: Some(Stability::Deprecated),
deprecated: Some("true".to_owned()),
examples: Some(Examples::String("test".to_owned())),
tag: None,
requirement_level: Default::default(),
sampling_relevant: None,
note: "".to_owned(),
}];
let mut group = GroupSpec {
id: "test".to_owned(),
r#type: GroupType::AttributeGroup,
brief: "test".to_owned(),
note: "test".to_owned(),
prefix: "".to_owned(),
extends: None,
stability: Some(Stability::Stable),
deprecated: None,
attributes: vec![],
constraints: vec![],
span_kind: None,
events: vec![],
metric_name: None,
instrument: None,
unit: None,
name: None,
display_name: None,
body: None,
};

// Attribute Group must have extends or attributes.
let result = group.validate("<test>").into_result_failing_non_fatal();
assert_eq!(
Err(InvalidGroupMissingExtendsOrAttributes {
path_or_url: "<test>".to_owned(),
group_id: "test".to_owned(),
error: "This group does not contain an extends or attributes field.".to_owned(),
}),
result
);

group.attributes = attributes.clone();
assert!(group
.validate("<test>")
.into_result_failing_non_fatal()
.is_ok());

group.attributes = vec![];
group.extends = Some("test".to_owned());
assert!(group
.validate("<test>")
.into_result_failing_non_fatal()
.is_ok());
group.extends = None;

// Span must have extends or attributes.
group.r#type = GroupType::Span;
let result = group.validate("<test>").into_result_failing_non_fatal();
assert_eq!(
Err(InvalidGroupMissingExtendsOrAttributes {
path_or_url: "<test>".to_owned(),
group_id: "test".to_owned(),
error: "This group does not contain an extends or attributes field.".to_owned(),
}),
result
);

group.attributes = attributes.clone();
assert!(group
.validate("<test>")
.into_result_failing_non_fatal()
.is_ok());

group.attributes = vec![];
group.extends = Some("test".to_owned());
assert!(group
.validate("<test>")
.into_result_failing_non_fatal()
.is_ok());
group.extends = None;

// Resource must have extends or attributes.
group.r#type = GroupType::Resource;
let result = group.validate("<test>").into_result_failing_non_fatal();
assert_eq!(
Err(InvalidGroupMissingExtendsOrAttributes {
path_or_url: "<test>".to_owned(),
group_id: "test".to_owned(),
error: "This group does not contain an extends or attributes field.".to_owned(),
}),
result
);

group.attributes = attributes.clone();
assert!(group
.validate("<test>")
.into_result_failing_non_fatal()
.is_ok());

group.attributes = vec![];
group.extends = Some("test".to_owned());
assert!(group
.validate("<test>")
.into_result_failing_non_fatal()
.is_ok());
group.extends = None;

// Metrics DO NOT need extends or attributes.
group.r#type = GroupType::Metric;
group.metric_name = Some("test".to_owned());
group.instrument = Some(Counter);
group.unit = Some("test".to_owned());
assert!(group
.validate("<test>")
.into_result_failing_non_fatal()
.is_ok());

// Events DO NOT need extends or attributes.
group.r#type = GroupType::Event;
group.name = Some("test".to_owned());
assert!(group
.validate("<test>")
.into_result_failing_non_fatal()
.is_ok());
}

#[test]
fn test_instrumentation_spec() {
assert_eq!(Counter.to_string(), "counter");
Expand Down
12 changes: 12 additions & 0 deletions crates/weaver_semconv/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,18 @@ pub enum Error {
error: String,
},

/// The semantic convention spec contains an invalid group definition. Missing extends or attributes
#[error("Invalid group '{group_id}', missing extends or attributes, detected while resolving '{path_or_url:?}'. {error}")]
#[diagnostic(severity(Warning))]
InvalidGroupMissingExtendsOrAttributes {
/// The path or URL of the semantic convention asset.
path_or_url: String,
/// The group id.
group_id: String,
/// The reason of the error.
error: String,
},

/// The semantic convention asset contains an invalid attribute definition.
#[error("Invalid attribute definition detected while resolving '{path_or_url:?}' (group_id='{group_id}', attribute_id='{attribute_id}'). {error}")]
InvalidAttribute {
Expand Down
11 changes: 8 additions & 3 deletions schemas/semconv-syntax.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,17 @@ All attributes are lower case.
groups ::= semconv
| semconv groups
semconv ::= id [convtype] brief [note] [extends] [stability] [deprecated] [display_name] attributes [specificfields]
semconv ::= id [convtype] brief [note] [extends] [stability] [deprecated] [display_name] [attributes] specificfields
extends_or_attributes ::= (extends | attributes | (extends attributes))
id ::= string
convtype ::= "span" # Default if not specified
| "resource" # see spansfields
| "event" # see eventfields
| "metric" # see metricfields
| "attribute_group" # no specific fields defined
| "attribute_group" # see attribute_group_fields
brief ::= string
note ::= string
Expand Down Expand Up @@ -102,8 +104,11 @@ examples ::= <example_value> {<example_value>}
specificfields ::= spanfields
| eventfields
| metricfields
| attribute_group_fields
attribute_group_fields ::= extends_or_attributes
spanfields ::= [events] [span_kind] stability
spanfields ::= [events] [span_kind] stability extends_or_attributes
eventfields ::= name [body] stability
Expand Down
64 changes: 44 additions & 20 deletions schemas/semconv.schema.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,32 +47,56 @@
"id",
"brief"
],
"anyOf": [
"allOf": [
{
"required": [
"attributes"
]
"if": {
"properties": {
"type": {
"not": {
"anyOf": [
{
"const": "metric"
},
{
"const": "event"
}
]
}
}
}
},
"then": {
"anyOf": [
{
"required": [
"attributes"
]
},
{
"required": [
"extends"
]
}
]
}
},
{
"required": [
"extends"
]
}
],
"if": {
"properties": {
"type": {
"not": {
"const": "attribute_group"
"if": {
"properties": {
"type": {
"not": {
"const": "attribute_group"
}
}
}
},
"then": {
"required": [
"stability"
]
}
}
},
"then": {
"required": [
"stability"
]
},
],
"properties": {
"id": {
"type": "string",
Expand Down

0 comments on commit 12064c7

Please sign in to comment.