Skip to content

Commit 1364569

Browse files
jerblyjerbly
authored
Fix live check error handling (#722)
* Add Bad Rego test policy and fix live check error handling * Fix line endings in yaml test
1 parent 1a8a8c4 commit 1364569

File tree

8 files changed

+167
-45
lines changed

8 files changed

+167
-45
lines changed

‎crates/weaver_forge/src/extensions/util.rs‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -233,9 +233,12 @@ mod tests {
233233

234234
add_filters(&mut env, &config);
235235
let expected_yaml = fs::read_to_string("expected_output/yaml/test.yaml").unwrap();
236-
assert_eq!(
237-
env.render_str("{{ user | toyaml }}", &ctx).unwrap(),
238-
expected_yaml
239-
);
236+
// Normalize line endings for both strings (remove any \r characters)
237+
let normalized_expected = expected_yaml.replace("\r\n", "\n");
238+
let normalized_actual = env
239+
.render_str("{{ user | toyaml }}", &ctx)
240+
.unwrap()
241+
.replace("\r\n", "\n");
242+
assert_eq!(normalized_actual, normalized_expected);
240243
}
241244
}

‎crates/weaver_live_check/data/policies/bad_advice/bad.rego‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
package live_check_advice
2+
3+
import rego.v1
4+
5+
# Causes: "error: use of undefined variable `attribu1te_name` is unsafe"
6+
deny contains make_advice("foo", "violation", attribute_name, "bar") if {
7+
attribute_name := "foo"
8+
not attribu1te_name
9+
}
10+
11+
make_advice(advice_type, advice_level, value, message) := {
12+
"type": "advice",
13+
"advice_type": advice_type,
14+
"advice_level": advice_level,
15+
"value": value,
16+
"message": message,
17+
}

‎crates/weaver_live_check/src/lib.rs‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,11 @@ pub enum SampleRef<'a> {
114114

115115
// Dispatch the live check to the sample type
116116
impl LiveCheckRunner for Sample {
117-
fn run_live_check(&mut self, live_checker: &mut LiveChecker, stats: &mut LiveCheckStatistics) {
117+
fn run_live_check(
118+
&mut self,
119+
live_checker: &mut LiveChecker,
120+
stats: &mut LiveCheckStatistics,
121+
) -> Result<(), Error> {
118122
match self {
119123
Sample::Attribute(attribute) => attribute.run_live_check(live_checker, stats),
120124
Sample::Span(span) => span.run_live_check(live_checker, stats),
@@ -331,5 +335,9 @@ impl LiveCheckStatistics {
331335
/// Samples implement this trait to run live checks on themselves
332336
pub trait LiveCheckRunner {
333337
/// Run the live check
334-
fn run_live_check(&mut self, live_checker: &mut LiveChecker, stats: &mut LiveCheckStatistics);
338+
fn run_live_check(
339+
&mut self,
340+
live_checker: &mut LiveChecker,
341+
stats: &mut LiveCheckStatistics,
342+
) -> Result<(), Error>;
335343
}

‎crates/weaver_live_check/src/live_checker.rs‎

Lines changed: 78 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ mod tests {
153153

154154
let mut stats = LiveCheckStatistics::new(&live_checker.registry);
155155
for sample in &mut samples {
156-
sample.run_live_check(&mut live_checker, &mut stats);
156+
let _ = sample.run_live_check(&mut live_checker, &mut stats);
157157
}
158158
stats.finalize();
159159

@@ -481,7 +481,7 @@ mod tests {
481481

482482
let mut stats = LiveCheckStatistics::new(&live_checker.registry);
483483
for sample in &mut samples {
484-
sample.run_live_check(&mut live_checker, &mut stats);
484+
let _ = sample.run_live_check(&mut live_checker, &mut stats);
485485
}
486486
stats.finalize();
487487

@@ -535,7 +535,7 @@ mod tests {
535535

536536
let mut stats = LiveCheckStatistics::new(&live_checker.registry);
537537
for sample in &mut samples {
538-
sample.run_live_check(&mut live_checker, &mut stats);
538+
let _ = sample.run_live_check(&mut live_checker, &mut stats);
539539
}
540540
stats.finalize();
541541

@@ -548,4 +548,79 @@ mod tests {
548548
assert_eq!(stats.total_entities_by_type.get("resource"), Some(&1));
549549
assert_eq!(stats.total_advisories, 14);
550550
}
551+
552+
#[test]
553+
fn test_bad_custom_rego() {
554+
let registry = ResolvedRegistry {
555+
registry_url: "TEST".to_owned(),
556+
groups: vec![ResolvedGroup {
557+
id: "custom.comprehensive.internal".to_owned(),
558+
r#type: GroupType::Span,
559+
brief: "".to_owned(),
560+
note: "".to_owned(),
561+
prefix: "".to_owned(),
562+
entity_associations: vec![],
563+
extends: None,
564+
stability: Some(Stability::Stable),
565+
deprecated: None,
566+
attributes: vec![Attribute {
567+
name: "custom.string".to_owned(),
568+
r#type: AttributeType::PrimitiveOrArray(PrimitiveOrArrayTypeSpec::String),
569+
examples: Some(Examples::Strings(vec![
570+
"value1".to_owned(),
571+
"value2".to_owned(),
572+
])),
573+
brief: "".to_owned(),
574+
tag: None,
575+
requirement_level: RequirementLevel::Recommended {
576+
text: "".to_owned(),
577+
},
578+
sampling_relevant: None,
579+
note: "".to_owned(),
580+
stability: Some(Stability::Stable),
581+
deprecated: None,
582+
prefix: false,
583+
tags: None,
584+
value: None,
585+
annotations: None,
586+
}],
587+
span_kind: Some(SpanKindSpec::Internal),
588+
events: vec![],
589+
metric_name: None,
590+
instrument: None,
591+
unit: None,
592+
name: None,
593+
lineage: None,
594+
display_name: None,
595+
body: None,
596+
}],
597+
};
598+
599+
let mut samples = vec![Sample::Attribute(
600+
SampleAttribute::try_from("custom.string=hello").unwrap(),
601+
)];
602+
603+
let advisors: Vec<Box<dyn Advisor>> = vec![];
604+
605+
let mut live_checker = LiveChecker::new(registry, advisors);
606+
let rego_advisor = RegoAdvisor::new(
607+
&live_checker,
608+
&Some("data/policies/bad_advice/".into()),
609+
&Some("data/jq/test.jq".into()),
610+
)
611+
.expect("Failed to create Rego advisor");
612+
live_checker.add_advisor(Box::new(rego_advisor));
613+
614+
let mut stats = LiveCheckStatistics::new(&live_checker.registry);
615+
for sample in &mut samples {
616+
// This should fail with: "error: use of undefined variable `attribu1te_name` is unsafe"
617+
618+
let result = sample.run_live_check(&mut live_checker, &mut stats);
619+
assert!(result.is_err());
620+
assert!(result
621+
.unwrap_err()
622+
.to_string()
623+
.contains("use of undefined variable"));
624+
}
625+
}
551626
}

‎crates/weaver_live_check/src/sample_attribute.rs‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ use weaver_checker::violation::{Advice, AdviceLevel};
88
use weaver_semconv::attribute::{AttributeType, PrimitiveOrArrayTypeSpec};
99

1010
use crate::{
11-
live_checker::LiveChecker, LiveCheckResult, LiveCheckRunner, LiveCheckStatistics, SampleRef,
12-
MISSING_ATTRIBUTE_ADVICE_TYPE, TEMPLATE_ATTRIBUTE_ADVICE_TYPE,
11+
live_checker::LiveChecker, Error, LiveCheckResult, LiveCheckRunner, LiveCheckStatistics,
12+
SampleRef, MISSING_ATTRIBUTE_ADVICE_TYPE, TEMPLATE_ATTRIBUTE_ADVICE_TYPE,
1313
};
1414

1515
/// Represents a sample telemetry attribute parsed from any source
@@ -57,7 +57,7 @@ impl<'de> Deserialize<'de> for SampleAttribute {
5757
}
5858

5959
impl TryFrom<&str> for SampleAttribute {
60-
type Error = crate::Error;
60+
type Error = Error;
6161

6262
fn try_from(value: &str) -> Result<Self, Self::Error> {
6363
let trimmed = value.trim();
@@ -162,7 +162,11 @@ impl SampleAttribute {
162162
}
163163

164164
impl LiveCheckRunner for SampleAttribute {
165-
fn run_live_check(&mut self, live_checker: &mut LiveChecker, stats: &mut LiveCheckStatistics) {
165+
fn run_live_check(
166+
&mut self,
167+
live_checker: &mut LiveChecker,
168+
stats: &mut LiveCheckStatistics,
169+
) -> Result<(), Error> {
166170
let mut result = LiveCheckResult::new();
167171
// find the attribute in the registry
168172
let semconv_attribute = {
@@ -196,14 +200,13 @@ impl LiveCheckRunner for SampleAttribute {
196200

197201
// run advisors on the attribute
198202
for advisor in live_checker.advisors.iter_mut() {
199-
if let Ok(advice_list) =
200-
advisor.advise(SampleRef::Attribute(self), semconv_attribute.as_ref(), None)
201-
{
202-
result.add_advice_list(advice_list);
203-
}
203+
let advice_list =
204+
advisor.advise(SampleRef::Attribute(self), semconv_attribute.as_ref(), None)?;
205+
result.add_advice_list(advice_list);
204206
}
205207
self.live_check_result = Some(result);
206208
self.update_stats(stats);
209+
Ok(())
207210
}
208211
}
209212

‎crates/weaver_live_check/src/sample_resource.rs‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55
use serde::{Deserialize, Serialize};
66

77
use crate::{
8-
live_checker::LiveChecker, sample_attribute::SampleAttribute, LiveCheckResult, LiveCheckRunner,
9-
LiveCheckStatistics, SampleRef,
8+
live_checker::LiveChecker, sample_attribute::SampleAttribute, Error, LiveCheckResult,
9+
LiveCheckRunner, LiveCheckStatistics, SampleRef,
1010
};
1111

1212
/// Represents a resource
@@ -20,18 +20,22 @@ pub struct SampleResource {
2020
}
2121

2222
impl LiveCheckRunner for SampleResource {
23-
fn run_live_check(&mut self, live_checker: &mut LiveChecker, stats: &mut LiveCheckStatistics) {
23+
fn run_live_check(
24+
&mut self,
25+
live_checker: &mut LiveChecker,
26+
stats: &mut LiveCheckStatistics,
27+
) -> Result<(), Error> {
2428
let mut result = LiveCheckResult::new();
2529
for advisor in live_checker.advisors.iter_mut() {
26-
if let Ok(advice_list) = advisor.advise(SampleRef::Resource(self), None, None) {
27-
result.add_advice_list(advice_list);
28-
}
30+
let advice_list = advisor.advise(SampleRef::Resource(self), None, None)?;
31+
result.add_advice_list(advice_list);
2932
}
3033
for attribute in &mut self.attributes {
31-
attribute.run_live_check(live_checker, stats);
34+
attribute.run_live_check(live_checker, stats)?;
3235
}
3336
self.live_check_result = Some(result);
3437
stats.inc_entity_count("resource");
3538
stats.maybe_add_live_check_result(self.live_check_result.as_ref());
39+
Ok(())
3640
}
3741
}

‎crates/weaver_live_check/src/sample_span.rs‎

Lines changed: 31 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ use serde::{Deserialize, Serialize};
66
use weaver_semconv::group::SpanKindSpec;
77

88
use crate::{
9-
live_checker::LiveChecker, sample_attribute::SampleAttribute, LiveCheckResult, LiveCheckRunner,
10-
LiveCheckStatistics, SampleRef,
9+
live_checker::LiveChecker, sample_attribute::SampleAttribute, Error, LiveCheckResult,
10+
LiveCheckRunner, LiveCheckStatistics, SampleRef,
1111
};
1212

1313
/// Represents a sample telemetry span parsed from any source
@@ -31,25 +31,29 @@ pub struct SampleSpan {
3131
}
3232

3333
impl LiveCheckRunner for SampleSpan {
34-
fn run_live_check(&mut self, live_checker: &mut LiveChecker, stats: &mut LiveCheckStatistics) {
34+
fn run_live_check(
35+
&mut self,
36+
live_checker: &mut LiveChecker,
37+
stats: &mut LiveCheckStatistics,
38+
) -> Result<(), Error> {
3539
let mut result = LiveCheckResult::new();
3640
for advisor in live_checker.advisors.iter_mut() {
37-
if let Ok(advice_list) = advisor.advise(SampleRef::Span(self), None, None) {
38-
result.add_advice_list(advice_list);
39-
}
41+
let advice_list = advisor.advise(SampleRef::Span(self), None, None)?;
42+
result.add_advice_list(advice_list);
4043
}
4144
for attribute in &mut self.attributes {
42-
attribute.run_live_check(live_checker, stats);
45+
attribute.run_live_check(live_checker, stats)?;
4346
}
4447
for span_event in &mut self.span_events {
45-
span_event.run_live_check(live_checker, stats);
48+
span_event.run_live_check(live_checker, stats)?;
4649
}
4750
for span_link in &mut self.span_links {
48-
span_link.run_live_check(live_checker, stats);
51+
span_link.run_live_check(live_checker, stats)?;
4952
}
5053
self.live_check_result = Some(result);
5154
stats.inc_entity_count("span");
5255
stats.maybe_add_live_check_result(self.live_check_result.as_ref());
56+
Ok(())
5357
}
5458
}
5559

@@ -66,19 +70,23 @@ pub struct SampleSpanEvent {
6670
}
6771

6872
impl LiveCheckRunner for SampleSpanEvent {
69-
fn run_live_check(&mut self, live_checker: &mut LiveChecker, stats: &mut LiveCheckStatistics) {
73+
fn run_live_check(
74+
&mut self,
75+
live_checker: &mut LiveChecker,
76+
stats: &mut LiveCheckStatistics,
77+
) -> Result<(), Error> {
7078
let mut result = LiveCheckResult::new();
7179
for advisor in live_checker.advisors.iter_mut() {
72-
if let Ok(advice_list) = advisor.advise(SampleRef::SpanEvent(self), None, None) {
73-
result.add_advice_list(advice_list);
74-
}
80+
let advice_list = advisor.advise(SampleRef::SpanEvent(self), None, None)?;
81+
result.add_advice_list(advice_list);
7582
}
7683
for attribute in &mut self.attributes {
77-
attribute.run_live_check(live_checker, stats);
84+
attribute.run_live_check(live_checker, stats)?;
7885
}
7986
self.live_check_result = Some(result);
8087
stats.inc_entity_count("span_event");
8188
stats.maybe_add_live_check_result(self.live_check_result.as_ref());
89+
Ok(())
8290
}
8391
}
8492

@@ -93,18 +101,22 @@ pub struct SampleSpanLink {
93101
}
94102

95103
impl LiveCheckRunner for SampleSpanLink {
96-
fn run_live_check(&mut self, live_checker: &mut LiveChecker, stats: &mut LiveCheckStatistics) {
104+
fn run_live_check(
105+
&mut self,
106+
live_checker: &mut LiveChecker,
107+
stats: &mut LiveCheckStatistics,
108+
) -> Result<(), Error> {
97109
let mut result = LiveCheckResult::new();
98110
for advisor in live_checker.advisors.iter_mut() {
99-
if let Ok(advice_list) = advisor.advise(SampleRef::SpanLink(self), None, None) {
100-
result.add_advice_list(advice_list);
101-
}
111+
let advice_list = advisor.advise(SampleRef::SpanLink(self), None, None)?;
112+
result.add_advice_list(advice_list);
102113
}
103114
for attribute in &mut self.attributes {
104-
attribute.run_live_check(live_checker, stats);
115+
attribute.run_live_check(live_checker, stats)?;
105116
}
106117
self.live_check_result = Some(result);
107118
stats.inc_entity_count("span_link");
108119
stats.maybe_add_live_check_result(self.live_check_result.as_ref());
120+
Ok(())
109121
}
110122
}

‎src/registry/live_check.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ pub(crate) fn command(args: &RegistryLiveCheckArgs) -> Result<ExitDirectives, Di
242242
let mut stats = LiveCheckStatistics::new(&live_checker.registry);
243243
let mut samples = Vec::new();
244244
for mut sample in ingester {
245-
sample.run_live_check(&mut live_checker, &mut stats);
245+
sample.run_live_check(&mut live_checker, &mut stats)?;
246246
if report_mode {
247247
samples.push(sample);
248248
} else {

0 commit comments

Comments
 (0)