Skip to content

Commit 2013621

Browse files
authored
fixed missing template checks and schema errors (#1055)
1 parent 5f71818 commit 2013621

File tree

2 files changed

+136
-5
lines changed

2 files changed

+136
-5
lines changed

crates/weaver_live_check/src/advice/type_advisor.rs

Lines changed: 132 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ pub struct TypeAdvisor;
2828
trait CheckableAttribute {
2929
fn key(&self) -> &str;
3030
fn requirement_level(&self) -> &RequirementLevel;
31+
fn attribute_type(&self) -> &AttributeType;
3132
}
3233

3334
impl CheckableAttribute for Attribute {
@@ -38,6 +39,10 @@ impl CheckableAttribute for Attribute {
3839
fn requirement_level(&self) -> &RequirementLevel {
3940
&self.requirement_level
4041
}
42+
43+
fn attribute_type(&self) -> &AttributeType {
44+
&self.r#type
45+
}
4146
}
4247

4348
impl CheckableAttribute for MetricAttribute {
@@ -48,6 +53,10 @@ impl CheckableAttribute for MetricAttribute {
4853
fn requirement_level(&self) -> &RequirementLevel {
4954
&self.requirement_level
5055
}
56+
57+
fn attribute_type(&self) -> &AttributeType {
58+
&self.base.r#type
59+
}
5160
}
5261

5362
impl CheckableAttribute for EventAttribute {
@@ -58,6 +67,10 @@ impl CheckableAttribute for EventAttribute {
5867
fn requirement_level(&self) -> &RequirementLevel {
5968
&self.requirement_level
6069
}
70+
71+
fn attribute_type(&self) -> &AttributeType {
72+
&self.base.r#type
73+
}
6174
}
6275

6376
/// Checks if attributes from a resolved group are present in a list of sample attributes
@@ -86,7 +99,23 @@ fn check_attributes<T: CheckableAttribute>(
8699
let mut advice_list = Vec::new();
87100
for semconv_attribute in semconv_attributes {
88101
let key = semconv_attribute.key();
89-
if !attribute_set.contains(key) {
102+
// Check if this is a template attribute
103+
let is_template = matches!(
104+
semconv_attribute.attribute_type(),
105+
AttributeType::Template(_)
106+
);
107+
108+
// For template attributes, check if any sample attribute starts with the template prefix
109+
// For non-template attributes, check for exact match
110+
let is_present = if is_template {
111+
sample_attributes
112+
.iter()
113+
.any(|attr| attr.name.starts_with(key))
114+
} else {
115+
attribute_set.contains(key)
116+
};
117+
118+
if !is_present {
90119
let (advice_type, advice_level, message) = match semconv_attribute.requirement_level() {
91120
RequirementLevel::Basic(BasicRequirementLevelSpec::Required) => (
92121
"required_attribute_not_present".to_owned(),
@@ -512,4 +541,106 @@ mod tests {
512541
let advice = check_attributes(&semconv_attributes, &sample_attributes, &sample);
513542
assert!(advice.is_empty());
514543
}
544+
545+
#[test]
546+
fn test_check_attributes_template_type() {
547+
use weaver_semconv::attribute::{AttributeType, TemplateTypeSpec};
548+
549+
// Create a template attribute like "weaver.finding.context"
550+
let template_attribute = Attribute {
551+
name: "weaver.finding.context".to_owned(),
552+
requirement_level: RequirementLevel::Basic(BasicRequirementLevelSpec::Recommended),
553+
r#type: AttributeType::Template(TemplateTypeSpec::Any),
554+
brief: "Template attribute for context".to_owned(),
555+
examples: None,
556+
tag: None,
557+
stability: None,
558+
deprecated: None,
559+
sampling_relevant: None,
560+
note: "".to_owned(),
561+
prefix: false,
562+
annotations: None,
563+
role: None,
564+
tags: None,
565+
value: None,
566+
};
567+
568+
let semconv_attributes = vec![template_attribute];
569+
570+
// Test 1: Template attribute with matching prefix - should NOT generate advice
571+
let sample_attributes_with_match = vec![
572+
create_sample_attribute("weaver.finding.context.foo"),
573+
create_sample_attribute("weaver.finding.context.bar"),
574+
];
575+
576+
let sample = Sample::Metric(SampleMetric {
577+
name: "test_metric".to_owned(),
578+
unit: "".to_owned(),
579+
data_points: None,
580+
instrument: SampleInstrument::Supported(weaver_semconv::group::InstrumentSpec::Counter),
581+
live_check_result: None,
582+
});
583+
584+
let advice = check_attributes(&semconv_attributes, &sample_attributes_with_match, &sample);
585+
assert!(
586+
advice.is_empty(),
587+
"Expected no advice when template attribute has matching prefixed attributes"
588+
);
589+
590+
// Test 2: Template attribute without matching prefix - SHOULD generate advice
591+
let sample_attributes_without_match = vec![
592+
create_sample_attribute("other.attribute"),
593+
create_sample_attribute("another.attribute"),
594+
];
595+
596+
let advice = check_attributes(
597+
&semconv_attributes,
598+
&sample_attributes_without_match,
599+
&sample,
600+
);
601+
assert_eq!(
602+
advice.len(),
603+
1,
604+
"Expected advice when template attribute has no matching prefixed attributes"
605+
);
606+
assert_eq!(advice[0].id, "recommended_attribute_not_present");
607+
assert_eq!(advice[0].level, FindingLevel::Improvement);
608+
609+
// Test 3: Mix of template and non-template attributes
610+
let regular_attribute = create_test_attribute(
611+
"regular.attr",
612+
RequirementLevel::Basic(BasicRequirementLevelSpec::Required),
613+
);
614+
let mixed_semconv_attributes = vec![
615+
Attribute {
616+
name: "template.attr".to_owned(),
617+
requirement_level: RequirementLevel::Basic(BasicRequirementLevelSpec::Required),
618+
r#type: AttributeType::Template(TemplateTypeSpec::String),
619+
brief: "Template attribute".to_owned(),
620+
examples: None,
621+
tag: None,
622+
stability: None,
623+
deprecated: None,
624+
sampling_relevant: None,
625+
note: "".to_owned(),
626+
prefix: false,
627+
annotations: None,
628+
role: None,
629+
tags: None,
630+
value: None,
631+
},
632+
regular_attribute,
633+
];
634+
635+
let mixed_sample_attributes = vec![
636+
create_sample_attribute("template.attr.key1"), // Matches template
637+
create_sample_attribute("regular.attr"), // Matches regular
638+
];
639+
640+
let advice = check_attributes(&mixed_semconv_attributes, &mixed_sample_attributes, &sample);
641+
assert!(
642+
advice.is_empty(),
643+
"Expected no advice when both template and regular attributes are present"
644+
);
645+
}
515646
}

schemas/semconv.schema.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@
9191
"type": "string",
9292
"description": "The new name of the telemetry object."
9393
},
94-
"note:": {
94+
"note": {
9595
"type": "string",
9696
"description": "A note explaining the reason for the deprecation."
9797
}
@@ -109,7 +109,7 @@
109109
"reason": {
110110
"const": "obsoleted"
111111
},
112-
"note:": {
112+
"note": {
113113
"type": "string",
114114
"description": "A note explaining the reason for the deprecation."
115115
}
@@ -126,7 +126,7 @@
126126
"reason": {
127127
"const": "uncategorized"
128128
},
129-
"note:": {
129+
"note": {
130130
"type": "string",
131131
"description": "A note explaining the reason for the deprecation."
132132
}
@@ -601,7 +601,7 @@
601601
},
602602
"annotations": {
603603
"$ref": "#/$defs/annotations"
604-
}
604+
}
605605
}
606606
}
607607
}

0 commit comments

Comments
 (0)