From 09c562a1a885c123bf6eb17095f88b679ac64c41 Mon Sep 17 00:00:00 2001 From: neuronull Date: Tue, 11 Jul 2023 10:59:14 -0600 Subject: [PATCH 01/32] add fix and small refactor --- src/components/validation/resources/event.rs | 57 ++- src/components/validation/resources/http.rs | 4 +- src/components/validation/resources/mod.rs | 2 +- src/components/validation/runner/io.rs | 6 +- src/components/validation/runner/mod.rs | 2 +- src/components/validation/runner/telemetry.rs | 44 +- .../validators/component_spec/mod.rs | 14 +- .../validators/component_spec/sources.rs | 375 ++++++------------ 8 files changed, 222 insertions(+), 282 deletions(-) diff --git a/src/components/validation/resources/event.rs b/src/components/validation/resources/event.rs index 21c38e58bfad5..fe61f782bafbd 100644 --- a/src/components/validation/resources/event.rs +++ b/src/components/validation/resources/event.rs @@ -1,10 +1,12 @@ use serde::Deserialize; +use snafu::Snafu; use vector_core::event::{Event, LogEvent}; -/// An event used in a test case. +/// An raw test case event for deserialization from yaml file. +/// This is an intermediary step to TestEvent. #[derive(Clone, Debug, Deserialize)] #[serde(untagged)] -pub enum TestEvent { +pub enum RawTestEvent { /// The event is used, as-is, without modification. Passthrough(EventData), @@ -20,12 +22,51 @@ pub enum TestEvent { Modified { modified: bool, event: EventData }, } -impl TestEvent { - pub fn into_event(self) -> Event { - match self { - Self::Passthrough(event) => event.into_event(), - Self::Modified { event, .. } => event.into_event(), - } +/// An event used in a test case. +#[derive(Clone, Debug, Deserialize)] +#[serde(try_from = "RawTestEvent")] +#[serde(untagged)] +pub enum TestEvent { + /// The event is used, as-is, without modification. + Passthrough(Event), + + /// The event is potentially modified by the external resource. + /// + /// The modification made is dependent on the external resource, but this mode is made available + /// for when a test case wants to exercise the failure path, but cannot cause a failure simply + /// by constructing the event in a certain way i.e. adding an invalid field, or removing a + /// required field, or using an invalid field value, and so on. + /// + /// For transforms and sinks, generally, the only way to cause an error is if the event itself + /// is malformed in some way, which can be achieved without this test event variant. + Modified { modified: bool, event: Event }, +} + +// impl TestEvent { +// pub fn into_event(self) -> Event { +// match self { +// Self::Passthrough(event) => event.into_event(), +// Self::Modified { event, .. } => event.into_event(), +// } +// } +// } + +#[derive(Clone, Debug, Eq, PartialEq, Snafu)] +pub enum RawTestEventParseError {} + +impl TryFrom for TestEvent { + type Error = RawTestEventParseError; + + fn try_from(other: RawTestEvent) -> Result { + Ok(match other { + RawTestEvent::Passthrough(event_data) => { + TestEvent::Passthrough(event_data.into_event()) + } + RawTestEvent::Modified { modified, event } => TestEvent::Modified { + modified, + event: event.into_event(), + }, + }) } } diff --git a/src/components/validation/resources/http.rs b/src/components/validation/resources/http.rs index 8fa066b6f3f4a..64ca6e04fd4bd 100644 --- a/src/components/validation/resources/http.rs +++ b/src/components/validation/resources/http.rs @@ -410,7 +410,7 @@ pub fn encode_test_event( TestEvent::Passthrough(event) => { // Encode the event normally. encoder - .encode(event.into_event(), buf) + .encode(event, buf) .expect("should not fail to encode input event"); } TestEvent::Modified { event, .. } => { @@ -431,7 +431,7 @@ pub fn encode_test_event( }; alt_encoder - .encode(event.into_event(), buf) + .encode(event, buf) .expect("should not fail to encode input event"); } } diff --git a/src/components/validation/resources/mod.rs b/src/components/validation/resources/mod.rs index 2b9fc3c542ccb..f382e3b2c6277 100644 --- a/src/components/validation/resources/mod.rs +++ b/src/components/validation/resources/mod.rs @@ -13,7 +13,7 @@ use vector_core::{config::DataType, event::Event}; use crate::codecs::{Decoder, DecodingConfig, Encoder, EncodingConfig, EncodingConfigWithFraming}; -pub use self::event::{EventData, TestEvent}; +pub use self::event::{RawTestEvent, TestEvent}; pub use self::http::{encode_test_event, HttpResourceConfig}; use super::sync::{Configuring, TaskCoordinator}; diff --git a/src/components/validation/runner/io.rs b/src/components/validation/runner/io.rs index c454ba433c847..d2df83e754f6e 100644 --- a/src/components/validation/runner/io.rs +++ b/src/components/validation/runner/io.rs @@ -99,8 +99,12 @@ impl InputEdge { started.mark_as_done(); while let Some(test_event) = rx.recv().await { + let event = match test_event { + TestEvent::Passthrough(e) => e, + TestEvent::Modified { modified: _, event } => event, + }; let request = PushEventsRequest { - events: vec![test_event.into_event().into()], + events: vec![event.into()], }; if let Err(e) = client.push_events(request).await { diff --git a/src/components/validation/runner/mod.rs b/src/components/validation/runner/mod.rs index 2c2a066b17806..d2d483e48bd0a 100644 --- a/src/components/validation/runner/mod.rs +++ b/src/components/validation/runner/mod.rs @@ -271,7 +271,7 @@ impl Runner { // like the aforementioned unit tests, switch to any improved mechanism we come up with // in the future to make these tests more deterministic and waste less time waiting // around if we can avoid it. - tokio::time::sleep(Duration::from_secs(1)).await; + tokio::time::sleep(Duration::from_secs(2)).await; let input_events = test_case.events.clone(); let input_driver = tokio::spawn(async move { diff --git a/src/components/validation/runner/telemetry.rs b/src/components/validation/runner/telemetry.rs index 9ef813838f1dc..6b5bb784291ff 100644 --- a/src/components/validation/runner/telemetry.rs +++ b/src/components/validation/runner/telemetry.rs @@ -23,6 +23,8 @@ const INTERNAL_LOGS_KEY: &str = "_telemetry_logs"; const INTERNAL_METRICS_KEY: &str = "_telemetry_metrics"; const VECTOR_SINK_KEY: &str = "_telemetry_out"; +const SHUTDOWN_TICKS: u8 = 2; + // The metrics event to monitor for before shutting down a telemetry collector. const INTERNAL_METRICS_SHUTDOWN_EVENT: &str = "component_received_events_total"; @@ -113,28 +115,40 @@ impl Telemetry { let mut events_seen = 0; let current_time = chrono::Utc::now(); + let timeout = tokio::time::sleep(Duration::from_secs(10)); + tokio::pin!(timeout); + loop { - match &rx.recv().await { - None => break 'outer, - Some(telemetry_event) => { - telemetry_events.push(telemetry_event.clone()); - if let Event::Metric(metric) = telemetry_event { - if let Some(tags) = metric.tags() { - if metric.name() == INTERNAL_METRICS_SHUTDOWN_EVENT && - tags.get("component_name") == Some(INTERNAL_LOGS_KEY) && - metric.data().timestamp().unwrap() > ¤t_time { - debug!("Telemetry: processed one component_received_events_total event."); - - events_seen += 1; - if events_seen == 2 { - break 'outer; + select! { + d = rx.recv() => { + match d { + None => break, + Some(telemetry_event) => { + telemetry_events.push(telemetry_event.clone()); + if let Event::Metric(metric) = telemetry_event { + if let Some(tags) = metric.tags() { + if metric.name() == INTERNAL_METRICS_SHUTDOWN_EVENT && + tags.get("component_name") == Some(INTERNAL_LOGS_KEY) && + metric.data().timestamp().unwrap() > ¤t_time { + debug!("Telemetry: processed one component_received_events_total event."); + + events_seen += 1; + if events_seen == SHUTDOWN_TICKS { + break; + } + } } } } } - } + }, + _ = &mut timeout => break, } } + if events_seen != SHUTDOWN_TICKS { + panic!("did not receive {SHUTDOWN_TICKS} events while waiting for shutdown! found {events_seen}"); + } + break 'outer; }, maybe_telemetry_event = rx.recv() => match maybe_telemetry_event { None => break, diff --git a/src/components/validation/validators/component_spec/mod.rs b/src/components/validation/validators/component_spec/mod.rs index e566403d95bf9..fe080f684d96c 100644 --- a/src/components/validation/validators/component_spec/mod.rs +++ b/src/components/validation/validators/component_spec/mod.rs @@ -8,7 +8,7 @@ use crate::components::validation::{ use super::Validator; -use self::sources::{validate_sources, SourceMetrics}; +use self::sources::{validate_sources, SourceMetricType}; /// Validates that the component meets the requirements of the [Component Specification][component_spec]. /// @@ -128,7 +128,7 @@ fn validate_telemetry( fn filter_events_by_metric_and_component<'a>( telemetry_events: &'a [Event], - metric: SourceMetrics, + metric_type: &SourceMetricType, component_name: &'a str, ) -> Result, Vec> { let metrics: Vec<&Metric> = telemetry_events @@ -141,7 +141,7 @@ fn filter_events_by_metric_and_component<'a>( } }) .filter(|&m| { - if m.name() == metric.to_string() { + if m.name() == metric_type.to_string() { if let Some(tags) = m.tags() { if tags.get("component_name").unwrap_or("") == component_name { return true; @@ -153,10 +153,14 @@ fn filter_events_by_metric_and_component<'a>( }) .collect(); - debug!("{}: {} metrics found.", metric.to_string(), metrics.len(),); + debug!( + "{}: {} metrics found.", + metric_type.to_string(), + metrics.len(), + ); if metrics.is_empty() { - return Err(vec![format!("{}: no metrics were emitted.", metric)]); + return Err(vec![format!("{}: no metrics were emitted.", metric_type)]); } Ok(metrics) diff --git a/src/components/validation/validators/component_spec/sources.rs b/src/components/validation/validators/component_spec/sources.rs index c25b217a399e4..d4dbc5283ccb6 100644 --- a/src/components/validation/validators/component_spec/sources.rs +++ b/src/components/validation/validators/component_spec/sources.rs @@ -1,7 +1,6 @@ use std::fmt::{Display, Formatter}; use bytes::BytesMut; -use vector_common::json_size::JsonSize; use vector_core::event::{Event, MetricKind}; use vector_core::EstimatedJsonEncodedSizeOf; @@ -11,7 +10,7 @@ use super::filter_events_by_metric_and_component; const TEST_SOURCE_NAME: &str = "test_source"; -pub enum SourceMetrics { +pub enum SourceMetricType { EventsReceived, EventsReceivedBytes, ReceivedBytesTotal, @@ -19,18 +18,24 @@ pub enum SourceMetrics { SentEventBytesTotal, } -impl SourceMetrics { +impl SourceMetricType { const fn name(&self) -> &'static str { match self { - SourceMetrics::EventsReceived => "component_received_events_total", - SourceMetrics::EventsReceivedBytes => "component_received_event_bytes_total", - SourceMetrics::ReceivedBytesTotal => "component_received_bytes_total", - SourceMetrics::SentEventsTotal => "component_sent_events_total", - SourceMetrics::SentEventBytesTotal => "component_sent_event_bytes_total", + SourceMetricType::EventsReceived => "component_received_events_total", + SourceMetricType::EventsReceivedBytes => "component_received_event_bytes_total", + SourceMetricType::ReceivedBytesTotal => "component_received_bytes_total", + SourceMetricType::SentEventsTotal => "component_sent_events_total", + SourceMetricType::SentEventBytesTotal => "component_sent_event_bytes_total", } } } +impl Display for SourceMetricType { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.name()) + } +} + pub fn validate_sources( configuration: &ValidationConfiguration, inputs: &[TestEvent], @@ -62,63 +67,68 @@ pub fn validate_sources( } } -impl Display for SourceMetrics { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", self.name()) - } -} +fn sum_counters( + metric_name: &SourceMetricType, + metrics: &[&vector_core::event::Metric], +) -> Result> { + let mut sum: f64 = 0.0; + let mut errs = Vec::new(); -fn validate_component_received_events_total( - _configuration: &ValidationConfiguration, - inputs: &[TestEvent], - _outputs: &[Event], - telemetry_events: &[Event], -) -> Result, Vec> { - let mut errs: Vec = Vec::new(); - - let metrics = filter_events_by_metric_and_component( - telemetry_events, - SourceMetrics::EventsReceived, - TEST_SOURCE_NAME, - )?; - - let mut events = 0; for m in metrics { match m.value() { vector_core::event::MetricValue::Counter { value } => { if let MetricKind::Absolute = m.data().kind { - events = *value as i32 + sum = *value; } else { - events += *value as i32 + sum += *value; } } - _ => errs.push(format!( - "{}: metric value is not a counter", - SourceMetrics::EventsReceived, - )), + _ => errs.push(format!("{}: metric value is not a counter", metric_name,)), } } + if errs.is_empty() { + Ok(sum) + } else { + Err(errs) + } +} + +fn validate_events_total( + inputs: &[TestEvent], + telemetry_events: &[Event], + metric_type: &SourceMetricType, + passthrough: bool, +) -> Result, Vec> { + let mut errs: Vec = Vec::new(); + + let metrics = + filter_events_by_metric_and_component(telemetry_events, metric_type, TEST_SOURCE_NAME); + + let events: i32 = sum_counters(metric_type, &metrics)? as i32; + let expected_events = inputs.iter().fold(0, |acc, i| { - if let TestEvent::Passthrough(_) = i { - return acc + 1; + if passthrough { + if let TestEvent::Passthrough(_) = i { + return acc + 1; + } + } else { + if let TestEvent::Modified { .. } = i { + return acc + 1; + } } acc }); debug!( "{}: {} events, {} expected events.", - SourceMetrics::EventsReceived, - events, - expected_events, + metric_type, events, expected_events, ); if events != expected_events { errs.push(format!( "{}: expected {} events, but received {}", - SourceMetrics::EventsReceived, - expected_events, - events + metric_type, expected_events, events )); } @@ -126,66 +136,30 @@ fn validate_component_received_events_total( return Err(errs); } - Ok(vec![format!( - "{}: {}", - SourceMetrics::EventsReceived, - events, - )]) + Ok(vec![format!("{}: {}", metric_type, events,)]) } -fn validate_component_received_event_bytes_total( - _configuration: &ValidationConfiguration, - inputs: &[TestEvent], - _outputs: &[Event], +fn validate_bytes_total( telemetry_events: &[Event], + metric_type: &SourceMetricType, + expected_bytes: usize, ) -> Result, Vec> { let mut errs: Vec = Vec::new(); - let metrics = filter_events_by_metric_and_component( - telemetry_events, - SourceMetrics::EventsReceivedBytes, - TEST_SOURCE_NAME, - )?; - - let mut metric_bytes: f64 = 0.0; - for m in metrics { - match m.value() { - vector_core::event::MetricValue::Counter { value } => { - if let MetricKind::Absolute = m.data().kind { - metric_bytes = *value - } else { - metric_bytes += value - } - } - _ => errs.push(format!( - "{}: metric value is not a counter", - SourceMetrics::EventsReceivedBytes, - )), - } - } - - let expected_bytes = inputs.iter().fold(JsonSize::new(0), |acc, i| { - if let TestEvent::Passthrough(_) = i { - let size = vec![i.clone().into_event()].estimated_json_encoded_size_of(); - return acc + size; - } + let metrics = + filter_events_by_metric_and_component(telemetry_events, metric_type, TEST_SOURCE_NAME)?; - acc - }); + let metric_bytes = sum_counters(metric_type, &metrics)?; debug!( "{}: {} bytes, {} expected bytes.", - SourceMetrics::EventsReceivedBytes, - metric_bytes, - expected_bytes, + metric_type, metric_bytes, expected_bytes, ); - if JsonSize::new(metric_bytes as usize) != expected_bytes { + if metric_bytes != expected_bytes as f64 { errs.push(format!( "{}: expected {} bytes, but received {}", - SourceMetrics::EventsReceivedBytes, - expected_bytes, - metric_bytes + metric_type, expected_bytes, metric_bytes )); } @@ -193,83 +167,82 @@ fn validate_component_received_event_bytes_total( return Err(errs); } - Ok(vec![format!( - "{}: {}", - SourceMetrics::EventsReceivedBytes, - metric_bytes, - )]) + Ok(vec![format!("{}: {}", metric_type, metric_bytes,)]) } -fn validate_component_received_bytes_total( - configuration: &ValidationConfiguration, +fn validate_component_received_events_total( + _configuration: &ValidationConfiguration, inputs: &[TestEvent], _outputs: &[Event], telemetry_events: &[Event], ) -> Result, Vec> { - let mut errs: Vec = Vec::new(); - - let metrics = filter_events_by_metric_and_component( + validate_events_total( + inputs, telemetry_events, - SourceMetrics::ReceivedBytesTotal, - TEST_SOURCE_NAME, - )?; + &SourceMetricType::EventsReceived, + true, + ) +} - let mut metric_bytes: f64 = 0.0; - for m in metrics { - match m.value() { - vector_core::event::MetricValue::Counter { value } => { - if let MetricKind::Absolute = m.data().kind { - metric_bytes = *value - } else { - metric_bytes += value - } +fn validate_component_received_event_bytes_total( + _configuration: &ValidationConfiguration, + inputs: &[TestEvent], + _outputs: &[Event], + telemetry_events: &[Event], +) -> Result, Vec> { + let expected_bytes = inputs.iter().fold(0, |acc, i| { + if let TestEvent::Passthrough(e) = i { + match e { + Event::Log(log_event) => info!("event bytes total. test event: {:?}", log_event), + Event::Metric(_) => todo!(), + Event::Trace(_) => todo!(), } - _ => errs.push(format!( - "{}: metric value is not a counter", - SourceMetrics::ReceivedBytesTotal, - )), + let size = vec![e.clone()].estimated_json_encoded_size_of(); + return acc + size; } - } + acc + }); + + validate_bytes_total( + telemetry_events, + &SourceMetricType::EventsReceivedBytes, + expected_bytes, + ) +} + +fn validate_component_received_bytes_total( + configuration: &ValidationConfiguration, + inputs: &[TestEvent], + _outputs: &[Event], + telemetry_events: &[Event], +) -> Result, Vec> { let mut expected_bytes = 0; if let Some(c) = &configuration.external_resource { let mut encoder = c.codec.into_encoder(); for i in inputs { + let event = match i { + TestEvent::Passthrough(e) => e, + TestEvent::Modified { modified: _, event } => event, + }; + match event { + Event::Log(log_event) => { + info!(" received bytes total. test event: {:?}", log_event) + } + Event::Metric(_) => todo!(), + Event::Trace(_) => todo!(), + } let mut buffer = BytesMut::new(); encode_test_event(&mut encoder, &mut buffer, i.clone()); expected_bytes += buffer.len() } } - debug!( - "{}: {} bytes, expected at least {} bytes.", - SourceMetrics::ReceivedBytesTotal, - metric_bytes, + validate_bytes_total( + telemetry_events, + &SourceMetricType::ReceivedBytesTotal, expected_bytes, - ); - - // We'll just establish a lower bound because we can't guarantee that the - // source will receive an exact number of bytes, since we can't synchronize - // with its internal logic. For example, some sources push or pull metrics - // on a schedule (http_client). - if metric_bytes < expected_bytes as f64 { - errs.push(format!( - "{}: expected at least {} bytes, but received {}", - SourceMetrics::ReceivedBytesTotal, - expected_bytes, - metric_bytes - )); - } - - if !errs.is_empty() { - return Err(errs); - } - - Ok(vec![format!( - "{}: {}", - SourceMetrics::ReceivedBytesTotal, - metric_bytes, - )]) + ) } fn validate_component_sent_events_total( @@ -278,63 +251,12 @@ fn validate_component_sent_events_total( _outputs: &[Event], telemetry_events: &[Event], ) -> Result, Vec> { - let mut errs: Vec = Vec::new(); - - let metrics = filter_events_by_metric_and_component( + validate_events_total( + inputs, telemetry_events, - SourceMetrics::SentEventsTotal, - TEST_SOURCE_NAME, - )?; - - let mut events = 0; - for m in metrics { - match m.value() { - vector_core::event::MetricValue::Counter { value } => { - if let MetricKind::Absolute = m.data().kind { - events = *value as i32 - } else { - events += *value as i32 - } - } - _ => errs.push(format!( - "{}: metric value is not a counter", - SourceMetrics::SentEventsTotal, - )), - } - } - - let expected_events = inputs.iter().fold(0, |acc, i| { - if let TestEvent::Passthrough(_) = i { - return acc + 1; - } - acc - }); - - debug!( - "{}: {} events, {} expected events.", - SourceMetrics::SentEventsTotal, - events, - expected_events, - ); - - if events != expected_events { - errs.push(format!( - "{}: expected {} events, but received {}", - SourceMetrics::SentEventsTotal, - inputs.len(), - events - )); - } - - if !errs.is_empty() { - return Err(errs); - } - - Ok(vec![format!( - "{}: {}", - SourceMetrics::SentEventsTotal, - events, - )]) + &SourceMetricType::SentEventsTotal, + true, + ) } fn validate_component_sent_event_bytes_total( @@ -343,59 +265,14 @@ fn validate_component_sent_event_bytes_total( outputs: &[Event], telemetry_events: &[Event], ) -> Result, Vec> { - let mut errs: Vec = Vec::new(); - - let metrics = filter_events_by_metric_and_component( - telemetry_events, - SourceMetrics::SentEventBytesTotal, - TEST_SOURCE_NAME, - )?; - - let mut metric_bytes: f64 = 0.0; - for m in metrics { - match m.value() { - vector_core::event::MetricValue::Counter { value } => { - if let MetricKind::Absolute = m.data().kind { - metric_bytes = *value - } else { - metric_bytes += value - } - } - _ => errs.push(format!( - "{}: metric value is not a counter", - SourceMetrics::SentEventBytesTotal, - )), - } - } - - let mut expected_bytes = JsonSize::zero(); + let mut expected_bytes = 0; for e in outputs { expected_bytes += vec![e].estimated_json_encoded_size_of(); } - debug!( - "{}: {} bytes, {} expected bytes.", - SourceMetrics::SentEventBytesTotal, - metric_bytes, + validate_bytes_total( + telemetry_events, + &SourceMetricType::SentEventBytesTotal, expected_bytes, - ); - - if JsonSize::new(metric_bytes as usize) != expected_bytes { - errs.push(format!( - "{}: expected {} bytes, but received {}.", - SourceMetrics::SentEventBytesTotal, - expected_bytes, - metric_bytes - )); - } - - if !errs.is_empty() { - return Err(errs); - } - - Ok(vec![format!( - "{}: {}", - SourceMetrics::SentEventBytesTotal, - metric_bytes, - )]) + ) } From 1e0e6e7bddb5bad5c22f2792e217056b54c67713 Mon Sep 17 00:00:00 2001 From: neuronull Date: Tue, 11 Jul 2023 11:13:06 -0600 Subject: [PATCH 02/32] fix compilation errors --- .../validators/component_spec/mod.rs | 18 +++++------------- .../validators/component_spec/sources.rs | 6 +++--- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/src/components/validation/validators/component_spec/mod.rs b/src/components/validation/validators/component_spec/mod.rs index fe080f684d96c..88122822f2c0f 100644 --- a/src/components/validation/validators/component_spec/mod.rs +++ b/src/components/validation/validators/component_spec/mod.rs @@ -128,9 +128,9 @@ fn validate_telemetry( fn filter_events_by_metric_and_component<'a>( telemetry_events: &'a [Event], - metric_type: &SourceMetricType, + metric: &SourceMetricType, component_name: &'a str, -) -> Result, Vec> { +) -> Vec<&'a Metric> { let metrics: Vec<&Metric> = telemetry_events .iter() .flat_map(|e| { @@ -141,7 +141,7 @@ fn filter_events_by_metric_and_component<'a>( } }) .filter(|&m| { - if m.name() == metric_type.to_string() { + if m.name() == metric.to_string() { if let Some(tags) = m.tags() { if tags.get("component_name").unwrap_or("") == component_name { return true; @@ -153,15 +153,7 @@ fn filter_events_by_metric_and_component<'a>( }) .collect(); - debug!( - "{}: {} metrics found.", - metric_type.to_string(), - metrics.len(), - ); + debug!("{}: {} metrics found.", metric.to_string(), metrics.len(),); - if metrics.is_empty() { - return Err(vec![format!("{}: no metrics were emitted.", metric_type)]); - } - - Ok(metrics) + metrics } diff --git a/src/components/validation/validators/component_spec/sources.rs b/src/components/validation/validators/component_spec/sources.rs index d4dbc5283ccb6..c6bea95fe9205 100644 --- a/src/components/validation/validators/component_spec/sources.rs +++ b/src/components/validation/validators/component_spec/sources.rs @@ -147,7 +147,7 @@ fn validate_bytes_total( let mut errs: Vec = Vec::new(); let metrics = - filter_events_by_metric_and_component(telemetry_events, metric_type, TEST_SOURCE_NAME)?; + filter_events_by_metric_and_component(telemetry_events, metric_type, TEST_SOURCE_NAME); let metric_bytes = sum_counters(metric_type, &metrics)?; @@ -197,7 +197,7 @@ fn validate_component_received_event_bytes_total( Event::Metric(_) => todo!(), Event::Trace(_) => todo!(), } - let size = vec![e.clone()].estimated_json_encoded_size_of(); + let size = vec![e.clone()].estimated_json_encoded_size_of().get(); return acc + size; } @@ -267,7 +267,7 @@ fn validate_component_sent_event_bytes_total( ) -> Result, Vec> { let mut expected_bytes = 0; for e in outputs { - expected_bytes += vec![e].estimated_json_encoded_size_of(); + expected_bytes += vec![e].estimated_json_encoded_size_of().get(); } validate_bytes_total( From 63a9581205218677067a2bb78dbd546f444605dc Mon Sep 17 00:00:00 2001 From: neuronull Date: Tue, 11 Jul 2023 12:51:49 -0600 Subject: [PATCH 03/32] 3 ticks --- src/components/validation/runner/telemetry.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/validation/runner/telemetry.rs b/src/components/validation/runner/telemetry.rs index 6b5bb784291ff..3df924af3ea4f 100644 --- a/src/components/validation/runner/telemetry.rs +++ b/src/components/validation/runner/telemetry.rs @@ -23,7 +23,7 @@ const INTERNAL_LOGS_KEY: &str = "_telemetry_logs"; const INTERNAL_METRICS_KEY: &str = "_telemetry_metrics"; const VECTOR_SINK_KEY: &str = "_telemetry_out"; -const SHUTDOWN_TICKS: u8 = 2; +const SHUTDOWN_TICKS: u8 = 3; // The metrics event to monitor for before shutting down a telemetry collector. const INTERNAL_METRICS_SHUTDOWN_EVENT: &str = "component_received_events_total"; @@ -115,7 +115,7 @@ impl Telemetry { let mut events_seen = 0; let current_time = chrono::Utc::now(); - let timeout = tokio::time::sleep(Duration::from_secs(10)); + let timeout = tokio::time::sleep(Duration::from_secs(5)); tokio::pin!(timeout); loop { From c6af43eb13ab1596132aedee4095aa65f9a9bbc2 Mon Sep 17 00:00:00 2001 From: neuronull Date: Wed, 12 Jul 2023 11:15:08 -0600 Subject: [PATCH 04/32] dont compute expected metrics in validator --- src/components/validation/mod.rs | 13 ++ src/components/validation/resources/event.rs | 77 ++++++--- src/components/validation/resources/http.rs | 53 +----- src/components/validation/resources/mod.rs | 4 +- src/components/validation/runner/mod.rs | 159 +++++++++++++++--- .../validators/component_spec/mod.rs | 20 +-- .../validators/component_spec/sources.rs | 128 +++++--------- src/components/validation/validators/mod.rs | 5 +- 8 files changed, 255 insertions(+), 204 deletions(-) diff --git a/src/components/validation/mod.rs b/src/components/validation/mod.rs index 7457cd6548ad9..dd335140b97f5 100644 --- a/src/components/validation/mod.rs +++ b/src/components/validation/mod.rs @@ -170,6 +170,19 @@ macro_rules! register_validatable_component { }; } +/// Input and Output runners populate this structure as they send and receive events. +/// The structure is passed into the validator to use as the expected values for the +/// metrics that the components under test actually output. +#[derive(Default)] +pub struct RunnerMetrics { + pub received_events_total: u64, + pub received_event_bytes_total: u64, + pub received_bytes_total: u64, + pub sent_bytes_total: u64, // a reciprocal for received_bytes_total + pub sent_event_bytes_total: u64, + pub sent_event_total: u64, +} + #[cfg(all(test, feature = "component-validation-tests"))] mod tests { use std::{ diff --git a/src/components/validation/resources/event.rs b/src/components/validation/resources/event.rs index fe61f782bafbd..49bb8420b4944 100644 --- a/src/components/validation/resources/event.rs +++ b/src/components/validation/resources/event.rs @@ -1,8 +1,16 @@ +use bytes::BytesMut; use serde::Deserialize; use snafu::Snafu; +use tokio_util::codec::Encoder as _; + +use crate::codecs::Encoder; +use codecs::{ + encoding, JsonSerializer, LengthDelimitedEncoder, LogfmtSerializer, MetricTagValues, + NewlineDelimitedEncoder, +}; use vector_core::event::{Event, LogEvent}; -/// An raw test case event for deserialization from yaml file. +/// An test case event for deserialization from yaml file. /// This is an intermediary step to TestEvent. #[derive(Clone, Debug, Deserialize)] #[serde(untagged)] @@ -22,6 +30,22 @@ pub enum RawTestEvent { Modified { modified: bool, event: EventData }, } +#[derive(Clone, Debug, Deserialize)] +#[serde(untagged)] +pub enum EventData { + /// A log event. + Log(String), +} + +impl EventData { + /// Converts this event data into an `Event`. + pub fn into_event(self) -> Event { + match self { + Self::Log(message) => Event::Log(LogEvent::from_bytes_legacy(&message.into())), + } + } +} + /// An event used in a test case. #[derive(Clone, Debug, Deserialize)] #[serde(try_from = "RawTestEvent")] @@ -42,15 +66,6 @@ pub enum TestEvent { Modified { modified: bool, event: Event }, } -// impl TestEvent { -// pub fn into_event(self) -> Event { -// match self { -// Self::Passthrough(event) => event.into_event(), -// Self::Modified { event, .. } => event.into_event(), -// } -// } -// } - #[derive(Clone, Debug, Eq, PartialEq, Snafu)] pub enum RawTestEventParseError {} @@ -70,18 +85,38 @@ impl TryFrom for TestEvent { } } -#[derive(Clone, Debug, Deserialize)] -#[serde(untagged)] -pub enum EventData { - /// A log event. - Log(String), -} +pub fn encode_test_event( + encoder: &mut Encoder, + buf: &mut BytesMut, + event: TestEvent, +) { + match event { + TestEvent::Passthrough(event) => { + // Encode the event normally. + encoder + .encode(event, buf) + .expect("should not fail to encode input event"); + } + TestEvent::Modified { event, .. } => { + // This is a little fragile, but we check what serializer this encoder uses, and based + // on `Serializer::supports_json`, we choose an opposing codec. For example, if the + // encoder supports JSON, we'll use a serializer that doesn't support JSON, and vise + // versa. + let mut alt_encoder = if encoder.serializer().supports_json() { + Encoder::::new( + LengthDelimitedEncoder::new().into(), + LogfmtSerializer::new().into(), + ) + } else { + Encoder::::new( + NewlineDelimitedEncoder::new().into(), + JsonSerializer::new(MetricTagValues::default()).into(), + ) + }; -impl EventData { - /// Converts this event data into an `Event`. - pub fn into_event(self) -> Event { - match self { - Self::Log(message) => Event::Log(LogEvent::from_bytes_legacy(&message.into())), + alt_encoder + .encode(event, buf) + .expect("should not fail to encode input event"); } } } diff --git a/src/components/validation/resources/http.rs b/src/components/validation/resources/http.rs index 64ca6e04fd4bd..5b88234788381 100644 --- a/src/components/validation/resources/http.rs +++ b/src/components/validation/resources/http.rs @@ -1,5 +1,6 @@ use std::{ collections::VecDeque, + future::Future, net::{IpAddr, SocketAddr}, str::FromStr, sync::Arc, @@ -11,26 +12,18 @@ use axum::{ Router, }; use bytes::BytesMut; -use codecs::{ - encoding, JsonSerializer, LengthDelimitedEncoder, LogfmtSerializer, MetricTagValues, - NewlineDelimitedEncoder, -}; use http::{Method, Request, StatusCode, Uri}; use hyper::{Body, Client, Server}; -use std::future::Future; use tokio::{ select, sync::{mpsc, oneshot, Mutex, Notify}, }; -use tokio_util::codec::{Decoder, Encoder as _}; -use vector_core::event::Event; +use tokio_util::codec::Decoder; -use crate::{ - codecs::Encoder, - components::validation::sync::{Configuring, TaskCoordinator}, -}; +use crate::components::validation::sync::{Configuring, TaskCoordinator}; +use vector_core::event::Event; -use super::{ResourceCodec, ResourceDirection, TestEvent}; +use super::{encode_test_event, ResourceCodec, ResourceDirection, TestEvent}; /// An HTTP resource. #[derive(Clone)] @@ -400,39 +393,3 @@ fn socketaddr_from_uri(uri: &Uri) -> SocketAddr { SocketAddr::from((uri_host, uri_port)) } - -pub fn encode_test_event( - encoder: &mut Encoder, - buf: &mut BytesMut, - event: TestEvent, -) { - match event { - TestEvent::Passthrough(event) => { - // Encode the event normally. - encoder - .encode(event, buf) - .expect("should not fail to encode input event"); - } - TestEvent::Modified { event, .. } => { - // This is a little fragile, but we check what serializer this encoder uses, and based - // on `Serializer::supports_json`, we choose an opposing codec. For example, if the - // encoder supports JSON, we'll use a serializer that doesn't support JSON, and vise - // versa. - let mut alt_encoder = if encoder.serializer().supports_json() { - Encoder::::new( - LengthDelimitedEncoder::new().into(), - LogfmtSerializer::new().into(), - ) - } else { - Encoder::::new( - NewlineDelimitedEncoder::new().into(), - JsonSerializer::new(MetricTagValues::default()).into(), - ) - }; - - alt_encoder - .encode(event, buf) - .expect("should not fail to encode input event"); - } - } -} diff --git a/src/components/validation/resources/mod.rs b/src/components/validation/resources/mod.rs index f382e3b2c6277..f957bedc47954 100644 --- a/src/components/validation/resources/mod.rs +++ b/src/components/validation/resources/mod.rs @@ -13,8 +13,8 @@ use vector_core::{config::DataType, event::Event}; use crate::codecs::{Decoder, DecodingConfig, Encoder, EncodingConfig, EncodingConfigWithFraming}; -pub use self::event::{RawTestEvent, TestEvent}; -pub use self::http::{encode_test_event, HttpResourceConfig}; +pub use self::event::{encode_test_event, RawTestEvent, TestEvent}; +pub use self::http::HttpResourceConfig; use super::sync::{Configuring, TaskCoordinator}; diff --git a/src/components/validation/runner/mod.rs b/src/components/validation/runner/mod.rs index d2d483e48bd0a..3f13b20543557 100644 --- a/src/components/validation/runner/mod.rs +++ b/src/components/validation/runner/mod.rs @@ -2,18 +2,34 @@ pub mod config; mod io; mod telemetry; -use std::{collections::HashMap, path::PathBuf, time::Duration}; +use std::{ + collections::HashMap, + path::PathBuf, + sync::{Arc, Mutex}, + time::Duration, +}; + +use bytes::BytesMut; +use tokio::{ + runtime::Builder, + select, + sync::mpsc::{self, Receiver, Sender}, + task::JoinHandle, +}; +use tokio_util::codec::Encoder as _; -use tokio::{runtime::Builder, select, sync::mpsc}; -use vector_core::event::Event; +use codecs::encoding; +use vector_core::{event::Event, EstimatedJsonEncodedSizeOf}; use crate::{ - components::validation::TestCase, + codecs::Encoder, + components::validation::{RunnerMetrics, TestCase}, config::{ConfigBuilder, ConfigDiff}, topology, }; use super::{ + encode_test_event, sync::{Configuring, TaskCoordinator}, ComponentType, TestCaseExpectation, TestEvent, ValidationConfiguration, Validator, }; @@ -216,6 +232,11 @@ impl Runner { debug!("Component topology configuration built and telemetry collector spawned."); + // Create the data structure that the input and output runners will use to store + // their receivent/sent metrics. This is then shared with the Validator for comparison + // against the actual metrics output by the component under test. + let runner_metrics = Arc::new(Mutex::new(RunnerMetrics::default())); + // After that, we'll build the external resource necessary for this component, if any. // Once that's done, we build the input event/output event sender and receiver based on // whatever we spawned for an external resource. @@ -226,13 +247,13 @@ impl Runner { // For example, if we're validating a source, we would have added a filler sink for our // controlled output edge, which means we then need a server task listening for the // events sent by that sink. - let (runner_input, runner_output) = build_external_resource( + let (runner_input, runner_output, maybe_runner_encoder) = build_external_resource( &self.configuration, &input_task_coordinator, &output_task_coordinator, ); let input_tx = runner_input.into_sender(controlled_edges.input); - let mut output_rx = runner_output.into_receiver(controlled_edges.output); + let output_rx = runner_output.into_receiver(controlled_edges.output); debug!("External resource (if any) and controlled edges built and spawned."); // Now with any external resource spawned, as well as any tasks for handling controlled @@ -273,23 +294,18 @@ impl Runner { // around if we can avoid it. tokio::time::sleep(Duration::from_secs(2)).await; - let input_events = test_case.events.clone(); - let input_driver = tokio::spawn(async move { - for input_event in input_events { - input_tx - .send(input_event) - .await - .expect("input channel should not be closed"); - } - }); + let input_driver = spawn_input_driver( + test_case.events.clone(), + input_tx, + &runner_metrics, + maybe_runner_encoder.as_ref().map(|encoder| encoder.clone()), + ); - let output_driver = tokio::spawn(async move { - let mut output_events = Vec::new(); - while let Some(output_event) = output_rx.recv().await { - output_events.push(output_event); - } - output_events - }); + let output_driver = spawn_output_driver( + output_rx, + &runner_metrics, + maybe_runner_encoder.as_ref().map(|encoder| encoder.clone()), + ); // At this point, the component topology is running, and all input/output/telemetry // tasks are running as well. Our input driver should be sending (or will have already @@ -334,12 +350,12 @@ impl Runner { .values() .map(|validator| { validator.check_validation( - self.configuration.clone(), component_type, expectation, &input_events, &output_events, &telemetry_events, + &runner_metrics.lock().unwrap(), ) }) .collect(); @@ -397,9 +413,12 @@ fn build_external_resource( configuration: &ValidationConfiguration, input_task_coordinator: &TaskCoordinator, output_task_coordinator: &TaskCoordinator, -) -> (RunnerInput, RunnerOutput) { +) -> (RunnerInput, RunnerOutput, Option>) { let component_type = configuration.component_type(); let maybe_external_resource = configuration.external_resource(); + let maybe_encoder = maybe_external_resource + .as_ref() + .map(|resource| resource.codec.into_encoder()); match component_type { ComponentType::Source => { // As an external resource for a source, we create a channel that the validation runner @@ -411,11 +430,15 @@ fn build_external_resource( maybe_external_resource.expect("a source must always have an external resource"); resource.spawn_as_input(rx, input_task_coordinator); - (RunnerInput::External(tx), RunnerOutput::Controlled) + ( + RunnerInput::External(tx), + RunnerOutput::Controlled, + maybe_encoder, + ) } ComponentType::Transform => { // Transforms have no external resources. - (RunnerInput::Controlled, RunnerOutput::Controlled) + (RunnerInput::Controlled, RunnerOutput::Controlled, None) } ComponentType::Sink => { // As an external resource for a sink, we create a channel that the validation runner @@ -427,7 +450,11 @@ fn build_external_resource( maybe_external_resource.expect("a sink must always have an external resource"); resource.spawn_as_output(tx, output_task_coordinator); - (RunnerInput::Controlled, RunnerOutput::External(rx)) + ( + RunnerInput::Controlled, + RunnerOutput::External(rx), + maybe_encoder, + ) } } } @@ -483,6 +510,84 @@ fn spawn_component_topology( }); } +fn spawn_input_driver( + input_events: Vec, + input_tx: Sender, + runner_metrics: &Arc>, + mut maybe_encoder: Option>, +) -> JoinHandle<()> { + let input_runner_metrics = Arc::clone(&runner_metrics); + + tokio::spawn(async move { + for input_event in input_events { + input_tx + .send(input_event.clone()) + .await + .expect("input channel should not be closed"); + + // Update the runner metrics for the sent event. This will later + // be used in the Validators, as the "expected" case. + let mut input_runner_metrics = input_runner_metrics.lock().unwrap(); + + if let Some(mut encoder) = maybe_encoder.as_mut() { + let mut buffer = BytesMut::new(); + encode_test_event(&mut encoder, &mut buffer, input_event.clone()); + + input_runner_metrics.sent_bytes_total += buffer.len() as u64; + } + + let (modified, event) = match input_event { + TestEvent::Passthrough(event) => (false, event), + TestEvent::Modified { modified, event } => (modified, event), + }; + + // account for failure case + if !modified { + input_runner_metrics.sent_event_total += 1; + + input_runner_metrics.sent_event_bytes_total += + vec![event].estimated_json_encoded_size_of().get() as u64; + } + } + }) +} + +fn spawn_output_driver( + mut output_rx: Receiver, + runner_metrics: &Arc>, + maybe_encoder: Option>, +) -> JoinHandle> { + let output_runner_metrics = Arc::clone(&runner_metrics); + + tokio::spawn(async move { + let mut output_events = Vec::new(); + while let Some(output_event) = output_rx.recv().await { + output_events.push(output_event.clone()); + + // Update the runner metrics for the received event. This will later + // be used in the Validators, as the "expected" case. + let mut output_runner_metrics = output_runner_metrics.lock().unwrap(); + + output_runner_metrics.received_events_total += 1; + output_runner_metrics.received_event_bytes_total += vec![output_event.clone()] + .estimated_json_encoded_size_of() + .get() as u64; + + if let Some(encoder) = maybe_encoder.as_ref() { + let mut buffer = BytesMut::new(); + //encode_test_event(&mut encoder, &mut buffer, output_event); + encoder + .clone() + .encode(output_event, &mut buffer) + .expect("should not fail to encode output event"); + + output_runner_metrics.received_bytes_total += buffer.len() as u64; + } + } + output_events + }) +} + fn initialize_test_environment() { // Make sure our metrics recorder is installed and in test mode. This is necessary for // proper internal telemetry collect when running the component topology, even though it's diff --git a/src/components/validation/validators/component_spec/mod.rs b/src/components/validation/validators/component_spec/mod.rs index 88122822f2c0f..36aa2d4452b05 100644 --- a/src/components/validation/validators/component_spec/mod.rs +++ b/src/components/validation/validators/component_spec/mod.rs @@ -2,9 +2,7 @@ mod sources; use vector_core::event::{Event, Metric}; -use crate::components::validation::{ - ComponentType, TestCaseExpectation, TestEvent, ValidationConfiguration, -}; +use crate::components::validation::{ComponentType, RunnerMetrics, TestCaseExpectation, TestEvent}; use super::Validator; @@ -28,12 +26,12 @@ impl Validator for ComponentSpecValidator { fn check_validation( &self, - configuration: ValidationConfiguration, component_type: ComponentType, expectation: TestCaseExpectation, inputs: &[TestEvent], outputs: &[Event], telemetry_events: &[Event], + runner_metrics: &RunnerMetrics, ) -> Result, Vec> { for input in inputs { debug!("Validator observed input event: {:?}", input); @@ -84,13 +82,7 @@ impl Validator for ComponentSpecValidator { format!("received {} telemetry events", telemetry_events.len()), ]; - let out = validate_telemetry( - configuration, - component_type, - inputs, - outputs, - telemetry_events, - )?; + let out = validate_telemetry(component_type, telemetry_events, runner_metrics)?; run_out.extend(out); Ok(run_out) @@ -98,18 +90,16 @@ impl Validator for ComponentSpecValidator { } fn validate_telemetry( - configuration: ValidationConfiguration, component_type: ComponentType, - inputs: &[TestEvent], - outputs: &[Event], telemetry_events: &[Event], + runner_metrics: &RunnerMetrics, ) -> Result, Vec> { let mut out: Vec = Vec::new(); let mut errs: Vec = Vec::new(); match component_type { ComponentType::Source => { - let result = validate_sources(&configuration, inputs, outputs, telemetry_events); + let result = validate_sources(telemetry_events, runner_metrics); match result { Ok(o) => out.extend(o), Err(e) => errs.extend(e), diff --git a/src/components/validation/validators/component_spec/sources.rs b/src/components/validation/validators/component_spec/sources.rs index c6bea95fe9205..6298e241a8cde 100644 --- a/src/components/validation/validators/component_spec/sources.rs +++ b/src/components/validation/validators/component_spec/sources.rs @@ -1,10 +1,8 @@ use std::fmt::{Display, Formatter}; -use bytes::BytesMut; use vector_core::event::{Event, MetricKind}; -use vector_core::EstimatedJsonEncodedSizeOf; -use crate::components::validation::{encode_test_event, TestEvent, ValidationConfiguration}; +use crate::components::validation::RunnerMetrics; use super::filter_events_by_metric_and_component; @@ -37,10 +35,8 @@ impl Display for SourceMetricType { } pub fn validate_sources( - configuration: &ValidationConfiguration, - inputs: &[TestEvent], - outputs: &[Event], telemetry_events: &[Event], + runner_metrics: &RunnerMetrics, ) -> Result, Vec> { let mut out: Vec = Vec::new(); let mut errs: Vec = Vec::new(); @@ -54,7 +50,7 @@ pub fn validate_sources( ]; for v in validations.iter() { - match v(configuration, inputs, outputs, telemetry_events) { + match v(telemetry_events, runner_metrics) { Err(e) => errs.extend(e), Ok(m) => out.extend(m), } @@ -95,40 +91,26 @@ fn sum_counters( } fn validate_events_total( - inputs: &[TestEvent], telemetry_events: &[Event], metric_type: &SourceMetricType, - passthrough: bool, + expected_events: u64, ) -> Result, Vec> { let mut errs: Vec = Vec::new(); let metrics = filter_events_by_metric_and_component(telemetry_events, metric_type, TEST_SOURCE_NAME); - let events: i32 = sum_counters(metric_type, &metrics)? as i32; - - let expected_events = inputs.iter().fold(0, |acc, i| { - if passthrough { - if let TestEvent::Passthrough(_) = i { - return acc + 1; - } - } else { - if let TestEvent::Modified { .. } = i { - return acc + 1; - } - } - acc - }); + let actual_events: u64 = sum_counters(metric_type, &metrics)? as u64; debug!( "{}: {} events, {} expected events.", - metric_type, events, expected_events, + metric_type, actual_events, expected_events, ); - if events != expected_events { + if actual_events != expected_events { errs.push(format!( "{}: expected {} events, but received {}", - metric_type, expected_events, events + metric_type, expected_events, actual_events )); } @@ -136,30 +118,30 @@ fn validate_events_total( return Err(errs); } - Ok(vec![format!("{}: {}", metric_type, events,)]) + Ok(vec![format!("{}: {}", metric_type, actual_events)]) } fn validate_bytes_total( telemetry_events: &[Event], metric_type: &SourceMetricType, - expected_bytes: usize, + expected_bytes: u64, ) -> Result, Vec> { let mut errs: Vec = Vec::new(); let metrics = filter_events_by_metric_and_component(telemetry_events, metric_type, TEST_SOURCE_NAME); - let metric_bytes = sum_counters(metric_type, &metrics)?; + let actual_bytes: u64 = sum_counters(metric_type, &metrics)? as u64; debug!( "{}: {} bytes, {} expected bytes.", - metric_type, metric_bytes, expected_bytes, + metric_type, actual_bytes, expected_bytes, ); - if metric_bytes != expected_bytes as f64 { + if actual_bytes != expected_bytes { errs.push(format!( "{}: expected {} bytes, but received {}", - metric_type, expected_bytes, metric_bytes + metric_type, expected_bytes, actual_bytes )); } @@ -167,42 +149,31 @@ fn validate_bytes_total( return Err(errs); } - Ok(vec![format!("{}: {}", metric_type, metric_bytes,)]) + Ok(vec![format!("{}: {}", metric_type, actual_bytes)]) } fn validate_component_received_events_total( - _configuration: &ValidationConfiguration, - inputs: &[TestEvent], - _outputs: &[Event], telemetry_events: &[Event], + runner_metrics: &RunnerMetrics, ) -> Result, Vec> { + // The reciprocal metric for events received is events sent, + // so the expected value is what the input runner sent. + let expected_events = runner_metrics.sent_event_total; + validate_events_total( - inputs, telemetry_events, &SourceMetricType::EventsReceived, - true, + expected_events, ) } fn validate_component_received_event_bytes_total( - _configuration: &ValidationConfiguration, - inputs: &[TestEvent], - _outputs: &[Event], telemetry_events: &[Event], + runner_metrics: &RunnerMetrics, ) -> Result, Vec> { - let expected_bytes = inputs.iter().fold(0, |acc, i| { - if let TestEvent::Passthrough(e) = i { - match e { - Event::Log(log_event) => info!("event bytes total. test event: {:?}", log_event), - Event::Metric(_) => todo!(), - Event::Trace(_) => todo!(), - } - let size = vec![e.clone()].estimated_json_encoded_size_of().get(); - return acc + size; - } - - acc - }); + // The reciprocal metric for received_event_bytes is sent_event_bytes, + // so the expected value is what the input runner sent. + let expected_bytes = runner_metrics.sent_event_bytes_total; validate_bytes_total( telemetry_events, @@ -212,31 +183,12 @@ fn validate_component_received_event_bytes_total( } fn validate_component_received_bytes_total( - configuration: &ValidationConfiguration, - inputs: &[TestEvent], - _outputs: &[Event], telemetry_events: &[Event], + runner_metrics: &RunnerMetrics, ) -> Result, Vec> { - let mut expected_bytes = 0; - if let Some(c) = &configuration.external_resource { - let mut encoder = c.codec.into_encoder(); - for i in inputs { - let event = match i { - TestEvent::Passthrough(e) => e, - TestEvent::Modified { modified: _, event } => event, - }; - match event { - Event::Log(log_event) => { - info!(" received bytes total. test event: {:?}", log_event) - } - Event::Metric(_) => todo!(), - Event::Trace(_) => todo!(), - } - let mut buffer = BytesMut::new(); - encode_test_event(&mut encoder, &mut buffer, i.clone()); - expected_bytes += buffer.len() - } - } + // The reciprocal metric for received_bytes is sent_bytes, + // so the expected value is what the input runner sent. + let expected_bytes = runner_metrics.sent_bytes_total; validate_bytes_total( telemetry_events, @@ -246,29 +198,27 @@ fn validate_component_received_bytes_total( } fn validate_component_sent_events_total( - _configuration: &ValidationConfiguration, - inputs: &[TestEvent], - _outputs: &[Event], telemetry_events: &[Event], + runner_metrics: &RunnerMetrics, ) -> Result, Vec> { + // The reciprocal metric for events sent is events received, + // so the expected value is what the output runner received. + let expected_events = runner_metrics.received_events_total; + validate_events_total( - inputs, telemetry_events, &SourceMetricType::SentEventsTotal, - true, + expected_events, ) } fn validate_component_sent_event_bytes_total( - _configuration: &ValidationConfiguration, - _inputs: &[TestEvent], - outputs: &[Event], telemetry_events: &[Event], + runner_metrics: &RunnerMetrics, ) -> Result, Vec> { - let mut expected_bytes = 0; - for e in outputs { - expected_bytes += vec![e].estimated_json_encoded_size_of().get(); - } + // The reciprocal metric for sent_event_bytes is received_event_bytes, + // so the expected value is what the output runner received. + let expected_bytes = runner_metrics.received_event_bytes_total; validate_bytes_total( telemetry_events, diff --git a/src/components/validation/validators/mod.rs b/src/components/validation/validators/mod.rs index 8cb4c8945b16b..e563488d7f416 100644 --- a/src/components/validation/validators/mod.rs +++ b/src/components/validation/validators/mod.rs @@ -1,9 +1,10 @@ mod component_spec; + pub use self::component_spec::ComponentSpecValidator; use vector_core::event::Event; -use super::{ComponentType, TestCaseExpectation, TestEvent, ValidationConfiguration}; +use super::{ComponentType, RunnerMetrics, TestCaseExpectation, TestEvent}; /// A component validator. /// @@ -19,12 +20,12 @@ pub trait Validator { /// provided as well. fn check_validation( &self, - configuration: ValidationConfiguration, component_type: ComponentType, expectation: TestCaseExpectation, inputs: &[TestEvent], outputs: &[Event], telemetry_events: &[Event], + runner_metrics: &RunnerMetrics, ) -> Result, Vec>; } From 55a35183c5fde21b61b3fb6ef0aeb5c99171f188 Mon Sep 17 00:00:00 2001 From: neuronull Date: Wed, 12 Jul 2023 11:49:23 -0600 Subject: [PATCH 05/32] cleanup --- src/components/validation/resources/event.rs | 16 +++++++++++++++- src/components/validation/resources/mod.rs | 2 +- src/components/validation/runner/io.rs | 6 +----- src/components/validation/runner/mod.rs | 2 +- src/components/validation/runner/telemetry.rs | 2 +- .../validators/component_spec/sources.rs | 8 ++++---- 6 files changed, 23 insertions(+), 13 deletions(-) diff --git a/src/components/validation/resources/event.rs b/src/components/validation/resources/event.rs index 49bb8420b4944..eb49ee31d1db3 100644 --- a/src/components/validation/resources/event.rs +++ b/src/components/validation/resources/event.rs @@ -10,7 +10,7 @@ use codecs::{ }; use vector_core::event::{Event, LogEvent}; -/// An test case event for deserialization from yaml file. +/// A test case event for deserialization from yaml file. /// This is an intermediary step to TestEvent. #[derive(Clone, Debug, Deserialize)] #[serde(untagged)] @@ -47,6 +47,11 @@ impl EventData { } /// An event used in a test case. +/// It is important to have created the event with all fields, immediately after deserializing from the +/// test case definition yaml file. This ensures that the event data we are using in the expected/actual +/// metrics collection is based on the same event. Namely, one issue that can arise from creating the event +/// from the event data twice (once for the expected and once for actual), it can result in a timestamp in +/// the event which may or may not have the same millisecond precision as it's counterpart. #[derive(Clone, Debug, Deserialize)] #[serde(try_from = "RawTestEvent")] #[serde(untagged)] @@ -66,6 +71,15 @@ pub enum TestEvent { Modified { modified: bool, event: Event }, } +impl TestEvent { + pub fn into_event(self) -> Event { + match self { + Self::Passthrough(event) => event, + Self::Modified { event, .. } => event, + } + } +} + #[derive(Clone, Debug, Eq, PartialEq, Snafu)] pub enum RawTestEventParseError {} diff --git a/src/components/validation/resources/mod.rs b/src/components/validation/resources/mod.rs index f957bedc47954..22e59018da427 100644 --- a/src/components/validation/resources/mod.rs +++ b/src/components/validation/resources/mod.rs @@ -13,7 +13,7 @@ use vector_core::{config::DataType, event::Event}; use crate::codecs::{Decoder, DecodingConfig, Encoder, EncodingConfig, EncodingConfigWithFraming}; -pub use self::event::{encode_test_event, RawTestEvent, TestEvent}; +pub use self::event::{encode_test_event, TestEvent}; pub use self::http::HttpResourceConfig; use super::sync::{Configuring, TaskCoordinator}; diff --git a/src/components/validation/runner/io.rs b/src/components/validation/runner/io.rs index d2df83e754f6e..c454ba433c847 100644 --- a/src/components/validation/runner/io.rs +++ b/src/components/validation/runner/io.rs @@ -99,12 +99,8 @@ impl InputEdge { started.mark_as_done(); while let Some(test_event) = rx.recv().await { - let event = match test_event { - TestEvent::Passthrough(e) => e, - TestEvent::Modified { modified: _, event } => event, - }; let request = PushEventsRequest { - events: vec![event.into()], + events: vec![test_event.into_event().into()], }; if let Err(e) = client.push_events(request).await { diff --git a/src/components/validation/runner/mod.rs b/src/components/validation/runner/mod.rs index 3f13b20543557..be0157fea8336 100644 --- a/src/components/validation/runner/mod.rs +++ b/src/components/validation/runner/mod.rs @@ -233,7 +233,7 @@ impl Runner { debug!("Component topology configuration built and telemetry collector spawned."); // Create the data structure that the input and output runners will use to store - // their receivent/sent metrics. This is then shared with the Validator for comparison + // their received/sent metrics. This is then shared with the Validator for comparison // against the actual metrics output by the component under test. let runner_metrics = Arc::new(Mutex::new(RunnerMetrics::default())); diff --git a/src/components/validation/runner/telemetry.rs b/src/components/validation/runner/telemetry.rs index 3df924af3ea4f..96772f4902c22 100644 --- a/src/components/validation/runner/telemetry.rs +++ b/src/components/validation/runner/telemetry.rs @@ -146,7 +146,7 @@ impl Telemetry { } } if events_seen != SHUTDOWN_TICKS { - panic!("did not receive {SHUTDOWN_TICKS} events while waiting for shutdown! found {events_seen}"); + panic!("did not receive {SHUTDOWN_TICKS} events while waiting for shutdown! Only found {events_seen}!"); } break 'outer; }, diff --git a/src/components/validation/validators/component_spec/sources.rs b/src/components/validation/validators/component_spec/sources.rs index 6298e241a8cde..04b8aa138dd98 100644 --- a/src/components/validation/validators/component_spec/sources.rs +++ b/src/components/validation/validators/component_spec/sources.rs @@ -66,7 +66,7 @@ pub fn validate_sources( fn sum_counters( metric_name: &SourceMetricType, metrics: &[&vector_core::event::Metric], -) -> Result> { +) -> Result> { let mut sum: f64 = 0.0; let mut errs = Vec::new(); @@ -84,7 +84,7 @@ fn sum_counters( } if errs.is_empty() { - Ok(sum) + Ok(sum as u64) } else { Err(errs) } @@ -100,7 +100,7 @@ fn validate_events_total( let metrics = filter_events_by_metric_and_component(telemetry_events, metric_type, TEST_SOURCE_NAME); - let actual_events: u64 = sum_counters(metric_type, &metrics)? as u64; + let actual_events = sum_counters(metric_type, &metrics)?; debug!( "{}: {} events, {} expected events.", @@ -131,7 +131,7 @@ fn validate_bytes_total( let metrics = filter_events_by_metric_and_component(telemetry_events, metric_type, TEST_SOURCE_NAME); - let actual_bytes: u64 = sum_counters(metric_type, &metrics)? as u64; + let actual_bytes = sum_counters(metric_type, &metrics)?; debug!( "{}: {} bytes, {} expected bytes.", From 8ec87b3a25d87707f112538ffef1108d9857e922 Mon Sep 17 00:00:00 2001 From: neuronull Date: Wed, 12 Jul 2023 11:54:08 -0600 Subject: [PATCH 06/32] cleanup --- src/components/validation/runner/mod.rs | 1 - src/components/validation/runner/telemetry.rs | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/components/validation/runner/mod.rs b/src/components/validation/runner/mod.rs index be0157fea8336..73beba89bf1a5 100644 --- a/src/components/validation/runner/mod.rs +++ b/src/components/validation/runner/mod.rs @@ -575,7 +575,6 @@ fn spawn_output_driver( if let Some(encoder) = maybe_encoder.as_ref() { let mut buffer = BytesMut::new(); - //encode_test_event(&mut encoder, &mut buffer, output_event); encoder .clone() .encode(output_event, &mut buffer) diff --git a/src/components/validation/runner/telemetry.rs b/src/components/validation/runner/telemetry.rs index 96772f4902c22..7238b39160628 100644 --- a/src/components/validation/runner/telemetry.rs +++ b/src/components/validation/runner/telemetry.rs @@ -146,7 +146,7 @@ impl Telemetry { } } if events_seen != SHUTDOWN_TICKS { - panic!("did not receive {SHUTDOWN_TICKS} events while waiting for shutdown! Only found {events_seen}!"); + panic!("Did not receive {SHUTDOWN_TICKS} events while waiting for shutdown! Only received {events_seen}!"); } break 'outer; }, From 4b3b721ea7e3732f0cd75a5d1f1c3af76d04f5fa Mon Sep 17 00:00:00 2001 From: neuronull Date: Wed, 12 Jul 2023 12:14:22 -0600 Subject: [PATCH 07/32] clippy --- src/components/validation/resources/event.rs | 1 + src/components/validation/runner/mod.rs | 12 ++++++------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/components/validation/resources/event.rs b/src/components/validation/resources/event.rs index eb49ee31d1db3..beb9c244c2fbd 100644 --- a/src/components/validation/resources/event.rs +++ b/src/components/validation/resources/event.rs @@ -72,6 +72,7 @@ pub enum TestEvent { } impl TestEvent { + #[allow(clippy::missing_const_for_fn)] // const cannot run destructor pub fn into_event(self) -> Event { match self { Self::Passthrough(event) => event, diff --git a/src/components/validation/runner/mod.rs b/src/components/validation/runner/mod.rs index 73beba89bf1a5..d52a15926b8a0 100644 --- a/src/components/validation/runner/mod.rs +++ b/src/components/validation/runner/mod.rs @@ -298,13 +298,13 @@ impl Runner { test_case.events.clone(), input_tx, &runner_metrics, - maybe_runner_encoder.as_ref().map(|encoder| encoder.clone()), + maybe_runner_encoder.as_ref().cloned(), ); let output_driver = spawn_output_driver( output_rx, &runner_metrics, - maybe_runner_encoder.as_ref().map(|encoder| encoder.clone()), + maybe_runner_encoder.as_ref().cloned(), ); // At this point, the component topology is running, and all input/output/telemetry @@ -516,7 +516,7 @@ fn spawn_input_driver( runner_metrics: &Arc>, mut maybe_encoder: Option>, ) -> JoinHandle<()> { - let input_runner_metrics = Arc::clone(&runner_metrics); + let input_runner_metrics = Arc::clone(runner_metrics); tokio::spawn(async move { for input_event in input_events { @@ -529,9 +529,9 @@ fn spawn_input_driver( // be used in the Validators, as the "expected" case. let mut input_runner_metrics = input_runner_metrics.lock().unwrap(); - if let Some(mut encoder) = maybe_encoder.as_mut() { + if let Some(encoder) = maybe_encoder.as_mut() { let mut buffer = BytesMut::new(); - encode_test_event(&mut encoder, &mut buffer, input_event.clone()); + encode_test_event(encoder, &mut buffer, input_event.clone()); input_runner_metrics.sent_bytes_total += buffer.len() as u64; } @@ -557,7 +557,7 @@ fn spawn_output_driver( runner_metrics: &Arc>, maybe_encoder: Option>, ) -> JoinHandle> { - let output_runner_metrics = Arc::clone(&runner_metrics); + let output_runner_metrics = Arc::clone(runner_metrics); tokio::spawn(async move { let mut output_events = Vec::new(); From f9854bf53cd59f4233b6b02cb0604abec449ae7a Mon Sep 17 00:00:00 2001 From: neuronull Date: Wed, 12 Jul 2023 14:09:34 -0600 Subject: [PATCH 08/32] feedback tz: sent_eventssssss --- src/components/validation/mod.rs | 2 +- src/components/validation/runner/mod.rs | 2 +- src/components/validation/validators/component_spec/sources.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/validation/mod.rs b/src/components/validation/mod.rs index dd335140b97f5..90723ff416a50 100644 --- a/src/components/validation/mod.rs +++ b/src/components/validation/mod.rs @@ -180,7 +180,7 @@ pub struct RunnerMetrics { pub received_bytes_total: u64, pub sent_bytes_total: u64, // a reciprocal for received_bytes_total pub sent_event_bytes_total: u64, - pub sent_event_total: u64, + pub sent_events_total: u64, } #[cfg(all(test, feature = "component-validation-tests"))] diff --git a/src/components/validation/runner/mod.rs b/src/components/validation/runner/mod.rs index d52a15926b8a0..7c4024e6dda22 100644 --- a/src/components/validation/runner/mod.rs +++ b/src/components/validation/runner/mod.rs @@ -543,7 +543,7 @@ fn spawn_input_driver( // account for failure case if !modified { - input_runner_metrics.sent_event_total += 1; + input_runner_metrics.sent_events_total += 1; input_runner_metrics.sent_event_bytes_total += vec![event].estimated_json_encoded_size_of().get() as u64; diff --git a/src/components/validation/validators/component_spec/sources.rs b/src/components/validation/validators/component_spec/sources.rs index 04b8aa138dd98..6c1a3dca88399 100644 --- a/src/components/validation/validators/component_spec/sources.rs +++ b/src/components/validation/validators/component_spec/sources.rs @@ -158,7 +158,7 @@ fn validate_component_received_events_total( ) -> Result, Vec> { // The reciprocal metric for events received is events sent, // so the expected value is what the input runner sent. - let expected_events = runner_metrics.sent_event_total; + let expected_events = runner_metrics.sent_events_total; validate_events_total( telemetry_events, From 0577ee62f4b7b06d4d1f07e0adcdf25715a89a87 Mon Sep 17 00:00:00 2001 From: neuronull Date: Wed, 12 Jul 2023 14:57:44 -0600 Subject: [PATCH 09/32] feedback tz: fix telemetry shutdown finishing logic --- src/components/validation/resources/http.rs | 16 ++++---- src/components/validation/resources/mod.rs | 2 +- src/components/validation/runner/io.rs | 31 +++++++------- src/components/validation/runner/mod.rs | 41 ++++++++++--------- src/components/validation/runner/telemetry.rs | 41 +++++++------------ 5 files changed, 61 insertions(+), 70 deletions(-) diff --git a/src/components/validation/resources/http.rs b/src/components/validation/resources/http.rs index 5b88234788381..44e2b7ba7579b 100644 --- a/src/components/validation/resources/http.rs +++ b/src/components/validation/resources/http.rs @@ -60,7 +60,7 @@ impl HttpResourceConfig { self, direction: ResourceDirection, codec: ResourceCodec, - output_tx: mpsc::Sender, + output_tx: mpsc::Sender>, task_coordinator: &TaskCoordinator, ) { match direction { @@ -223,7 +223,7 @@ fn spawn_input_http_client( fn spawn_output_http_server( config: HttpResourceConfig, codec: ResourceCodec, - output_tx: mpsc::Sender, + output_tx: mpsc::Sender>, task_coordinator: &TaskCoordinator, ) { // This HTTP server will wait for events to be sent by a sink, and collect them and send them on @@ -245,12 +245,10 @@ fn spawn_output_http_server( loop { match decoder.decode_eof(&mut body) { Ok(Some((events, _byte_size))) => { - for event in events { - output_tx - .send(event) - .await - .expect("should not fail to send output event"); - } + output_tx + .send(events.to_vec()) + .await + .expect("should not fail to send output event"); } Ok(None) => return StatusCode::OK.into_response(), Err(_) => return StatusCode::INTERNAL_SERVER_ERROR.into_response(), @@ -283,7 +281,7 @@ fn spawn_output_http_server( fn spawn_output_http_client( _config: HttpResourceConfig, _codec: ResourceCodec, - _output_tx: mpsc::Sender, + _output_tx: mpsc::Sender>, _task_coordinator: &TaskCoordinator, ) { // TODO: The `prometheus_exporter` sink is the only sink that exposes an HTTP server which must be diff --git a/src/components/validation/resources/mod.rs b/src/components/validation/resources/mod.rs index 22e59018da427..a22d6fc324dbd 100644 --- a/src/components/validation/resources/mod.rs +++ b/src/components/validation/resources/mod.rs @@ -308,7 +308,7 @@ impl ExternalResource { /// Spawns this resource for use as an output for a sink. pub fn spawn_as_output( self, - output_tx: mpsc::Sender, + output_tx: mpsc::Sender>, task_coordinator: &TaskCoordinator, ) { match self.definition { diff --git a/src/components/validation/runner/io.rs b/src/components/validation/runner/io.rs index c454ba433c847..55e4fca1eaecf 100644 --- a/src/components/validation/runner/io.rs +++ b/src/components/validation/runner/io.rs @@ -27,11 +27,11 @@ use crate::{ #[derive(Clone)] pub struct EventForwardService { - tx: mpsc::Sender, + tx: mpsc::Sender>, } -impl From> for EventForwardService { - fn from(tx: mpsc::Sender) -> Self { +impl From>> for EventForwardService { + fn from(tx: mpsc::Sender>) -> Self { Self { tx } } } @@ -42,14 +42,17 @@ impl VectorService for EventForwardService { &self, request: tonic::Request, ) -> Result, Status> { - let events = request.into_inner().events.into_iter().map(Event::from); - - for event in events { - self.tx - .send(event) - .await - .expect("event forward rx should not close first"); - } + let events = request + .into_inner() + .events + .into_iter() + .map(Event::from) + .collect(); + + self.tx + .send(events) + .await + .expect("event forward rx should not close first"); Ok(tonic::Response::new(PushEventsResponse {})) } @@ -74,7 +77,7 @@ pub struct InputEdge { pub struct OutputEdge { listen_addr: GrpcAddress, service: VectorServer, - rx: mpsc::Receiver, + rx: mpsc::Receiver>, } impl InputEdge { @@ -129,7 +132,7 @@ impl OutputEdge { pub fn spawn_output_server( self, task_coordinator: &TaskCoordinator, - ) -> mpsc::Receiver { + ) -> mpsc::Receiver> { spawn_grpc_server(self.listen_addr, self.service, task_coordinator); self.rx } @@ -184,5 +187,5 @@ pub fn spawn_grpc_server( pub struct ControlledEdges { pub input: Option>, - pub output: Option>, + pub output: Option>>, } diff --git a/src/components/validation/runner/mod.rs b/src/components/validation/runner/mod.rs index 7c4024e6dda22..b4bcd72de75bf 100644 --- a/src/components/validation/runner/mod.rs +++ b/src/components/validation/runner/mod.rs @@ -89,7 +89,7 @@ pub enum RunnerOutput { /// external resource pulls output events from the sink. /// /// Only sinks have external inputs. - External(mpsc::Receiver), + External(mpsc::Receiver>), /// The component uses a "controlled" edge for its output. /// @@ -109,8 +109,8 @@ impl RunnerOutput { /// this function will panic, as one or the other must be provided. pub fn into_receiver( self, - controlled_edge: Option>, - ) -> mpsc::Receiver { + controlled_edge: Option>>, + ) -> mpsc::Receiver> { match (self, controlled_edge) { (Self::External(_), Some(_)) => panic!("Runner output declared as external resource, but controlled output edge was also specified."), (Self::Controlled, None) => panic!("Runner output declared as controlled, but no controlled output edge was specified."), @@ -553,7 +553,7 @@ fn spawn_input_driver( } fn spawn_output_driver( - mut output_rx: Receiver, + mut output_rx: Receiver>, runner_metrics: &Arc>, maybe_encoder: Option>, ) -> JoinHandle> { @@ -561,26 +561,29 @@ fn spawn_output_driver( tokio::spawn(async move { let mut output_events = Vec::new(); - while let Some(output_event) = output_rx.recv().await { - output_events.push(output_event.clone()); + while let Some(events) = output_rx.recv().await { + output_events.extend(events.clone()); // Update the runner metrics for the received event. This will later // be used in the Validators, as the "expected" case. let mut output_runner_metrics = output_runner_metrics.lock().unwrap(); - output_runner_metrics.received_events_total += 1; - output_runner_metrics.received_event_bytes_total += vec![output_event.clone()] - .estimated_json_encoded_size_of() - .get() as u64; - - if let Some(encoder) = maybe_encoder.as_ref() { - let mut buffer = BytesMut::new(); - encoder - .clone() - .encode(output_event, &mut buffer) - .expect("should not fail to encode output event"); - - output_runner_metrics.received_bytes_total += buffer.len() as u64; + for output_event in events { + output_runner_metrics.received_events_total += 1; + output_runner_metrics.received_event_bytes_total += vec![output_event.clone()] + .estimated_json_encoded_size_of() + .get() + as u64; + + if let Some(encoder) = maybe_encoder.as_ref() { + let mut buffer = BytesMut::new(); + encoder + .clone() + .encode(output_event, &mut buffer) + .expect("should not fail to encode output event"); + + output_runner_metrics.received_bytes_total += buffer.len() as u64; + } } } output_events diff --git a/src/components/validation/runner/telemetry.rs b/src/components/validation/runner/telemetry.rs index 7238b39160628..f69dc40217a92 100644 --- a/src/components/validation/runner/telemetry.rs +++ b/src/components/validation/runner/telemetry.rs @@ -23,16 +23,13 @@ const INTERNAL_LOGS_KEY: &str = "_telemetry_logs"; const INTERNAL_METRICS_KEY: &str = "_telemetry_metrics"; const VECTOR_SINK_KEY: &str = "_telemetry_out"; -const SHUTDOWN_TICKS: u8 = 3; - -// The metrics event to monitor for before shutting down a telemetry collector. -const INTERNAL_METRICS_SHUTDOWN_EVENT: &str = "component_received_events_total"; +const SHUTDOWN_TICKS: u8 = 2; /// Telemetry collector for a component under validation. pub struct Telemetry { listen_addr: GrpcAddress, service: VectorServer, - rx: mpsc::Receiver, + rx: mpsc::Receiver>, } impl Telemetry { @@ -100,20 +97,19 @@ impl Telemetry { select! { _ = telemetry_shutdown_handle.wait() => { // After we receive the shutdown signal, we need to wait - // for two event emissions from the internal_metrics + // for two batches of event emissions from the internal_metrics // source. This is to ensure that we've received all the // events from the components that we're testing. // // We need exactly two because the internal_metrics // source does not emit component events until after the // component_received_events_total metric has been - // emitted. Thus, two events ensure that all component + // emitted. Thus, two batches ensure that all component // events have been emitted. debug!("Telemetry: waiting for final internal_metrics events before shutting down."); - let mut events_seen = 0; - let current_time = chrono::Utc::now(); + let mut batches_received = 0; let timeout = tokio::time::sleep(Duration::from_secs(5)); tokio::pin!(timeout); @@ -123,21 +119,12 @@ impl Telemetry { d = rx.recv() => { match d { None => break, - Some(telemetry_event) => { - telemetry_events.push(telemetry_event.clone()); - if let Event::Metric(metric) = telemetry_event { - if let Some(tags) = metric.tags() { - if metric.name() == INTERNAL_METRICS_SHUTDOWN_EVENT && - tags.get("component_name") == Some(INTERNAL_LOGS_KEY) && - metric.data().timestamp().unwrap() > ¤t_time { - debug!("Telemetry: processed one component_received_events_total event."); - - events_seen += 1; - if events_seen == SHUTDOWN_TICKS { - break; - } - } - } + Some(telemetry_event_batch) => { + telemetry_events.extend(telemetry_event_batch); + debug!("Telemetry: processed one batch of internal_metrics."); + batches_received += 1; + if batches_received == SHUTDOWN_TICKS { + break; } } } @@ -145,14 +132,14 @@ impl Telemetry { _ = &mut timeout => break, } } - if events_seen != SHUTDOWN_TICKS { - panic!("Did not receive {SHUTDOWN_TICKS} events while waiting for shutdown! Only received {events_seen}!"); + if batches_received != SHUTDOWN_TICKS { + panic!("Did not receive {SHUTDOWN_TICKS} events while waiting for shutdown! Only received {batches_received}!"); } break 'outer; }, maybe_telemetry_event = rx.recv() => match maybe_telemetry_event { None => break, - Some(telemetry_event) => telemetry_events.push(telemetry_event), + Some(telemetry_event_batch) => telemetry_events.extend(telemetry_event_batch), }, } } From 51e9ab47ce1d86aba933a2946c84b8ea141352ac Mon Sep 17 00:00:00 2001 From: neuronull Date: Thu, 13 Jul 2023 14:01:10 -0600 Subject: [PATCH 10/32] 3 ticks --- src/components/validation/runner/telemetry.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/validation/runner/telemetry.rs b/src/components/validation/runner/telemetry.rs index f69dc40217a92..76e8c29f62a9f 100644 --- a/src/components/validation/runner/telemetry.rs +++ b/src/components/validation/runner/telemetry.rs @@ -23,7 +23,7 @@ const INTERNAL_LOGS_KEY: &str = "_telemetry_logs"; const INTERNAL_METRICS_KEY: &str = "_telemetry_metrics"; const VECTOR_SINK_KEY: &str = "_telemetry_out"; -const SHUTDOWN_TICKS: u8 = 2; +const SHUTDOWN_TICKS: u8 = 3; /// Telemetry collector for a component under validation. pub struct Telemetry { From e8cdf11f111b74f8fcfe3e1a090ad130ab1cb216 Mon Sep 17 00:00:00 2001 From: neuronull Date: Thu, 13 Jul 2023 16:58:00 -0600 Subject: [PATCH 11/32] small reorg to add sinks --- src/components/validation/mod.rs | 8 + src/components/validation/runner/config.rs | 7 +- .../validators/component_spec/mod.rs | 109 +++++++++++++- .../validators/component_spec/sinks.rs | 127 ++++++++++++++++ .../validators/component_spec/sources.rs | 140 ++---------------- src/components/validation/validators/mod.rs | 30 ++++ 6 files changed, 283 insertions(+), 138 deletions(-) create mode 100644 src/components/validation/validators/component_spec/sinks.rs diff --git a/src/components/validation/mod.rs b/src/components/validation/mod.rs index 90723ff416a50..a824de0c057d6 100644 --- a/src/components/validation/mod.rs +++ b/src/components/validation/mod.rs @@ -14,6 +14,14 @@ pub use self::runner::*; pub use self::test_case::{TestCase, TestCaseExpectation}; pub use self::validators::*; +pub mod component_names { + pub const TEST_SOURCE_NAME: &str = "test_source"; + pub const TEST_SINK_NAME: &str = "test_sink"; + pub const TEST_TRANSFORM_NAME: &str = "test_transform"; + pub const TEST_INPUT_SOURCE_NAME: &str = "input_source"; + pub const TEST_OUTPUT_SINK_NAME: &str = "output_sink"; +} + /// Component types that can be validated. // TODO: We should centralize this in `vector-common` or something, where both this code and the // configuration schema stuff (namely the proc macros that use this) can share it. diff --git a/src/components/validation/runner/config.rs b/src/components/validation/runner/config.rs index c087fd8bf809a..cab721450db3d 100644 --- a/src/components/validation/runner/config.rs +++ b/src/components/validation/runner/config.rs @@ -1,5 +1,6 @@ use crate::{ components::validation::{ + component_names::*, sync::{Configuring, TaskCoordinator}, util::GrpcAddress, ComponentConfiguration, ComponentType, ValidationConfiguration, @@ -21,12 +22,6 @@ pub struct TopologyBuilder { output_edge: Option, } -pub const TEST_SOURCE_NAME: &str = "test_source"; -pub const TEST_SINK_NAME: &str = "test_sink"; -pub const TEST_TRANSFORM_NAME: &str = "test_transform"; -pub const TEST_INPUT_SOURCE_NAME: &str = "input_source"; -pub const TEST_OUTPUT_SINK_NAME: &str = "output_sink"; - impl TopologyBuilder { /// Creates a component topology for the given component configuration. pub fn from_configuration(configuration: &ValidationConfiguration) -> Self { diff --git a/src/components/validation/validators/component_spec/mod.rs b/src/components/validation/validators/component_spec/mod.rs index 36aa2d4452b05..b1400a6ab847f 100644 --- a/src/components/validation/validators/component_spec/mod.rs +++ b/src/components/validation/validators/component_spec/mod.rs @@ -1,12 +1,14 @@ +mod sinks; mod sources; -use vector_core::event::{Event, Metric}; +use vector_core::event::{Event, Metric, MetricKind}; use crate::components::validation::{ComponentType, RunnerMetrics, TestCaseExpectation, TestEvent}; -use super::Validator; +use super::{ComponentMetricType, Validator}; -use self::sources::{validate_sources, SourceMetricType}; +use self::sinks::validate_sinks; +use self::sources::validate_sources; /// Validates that the component meets the requirements of the [Component Specification][component_spec]. /// @@ -105,7 +107,13 @@ fn validate_telemetry( Err(e) => errs.extend(e), } } - ComponentType::Sink => {} + ComponentType::Sink => { + let result = validate_sinks(telemetry_events, runner_metrics); + match result { + Ok(o) => out.extend(o), + Err(e) => errs.extend(e), + } + } ComponentType::Transform => {} } @@ -118,7 +126,7 @@ fn validate_telemetry( fn filter_events_by_metric_and_component<'a>( telemetry_events: &'a [Event], - metric: &SourceMetricType, + metric: &ComponentMetricType, component_name: &'a str, ) -> Vec<&'a Metric> { let metrics: Vec<&Metric> = telemetry_events @@ -147,3 +155,94 @@ fn filter_events_by_metric_and_component<'a>( metrics } + +fn sum_counters( + metric_name: &ComponentMetricType, + metrics: &[&vector_core::event::Metric], +) -> Result> { + let mut sum: f64 = 0.0; + let mut errs = Vec::new(); + + for m in metrics { + match m.value() { + vector_core::event::MetricValue::Counter { value } => { + if let MetricKind::Absolute = m.data().kind { + sum = *value; + } else { + sum += *value; + } + } + _ => errs.push(format!("{}: metric value is not a counter", metric_name,)), + } + } + + if errs.is_empty() { + Ok(sum as u64) + } else { + Err(errs) + } +} + +fn validate_events_total( + telemetry_events: &[Event], + metric_type: &ComponentMetricType, + component_name: &str, + expected_events: u64, +) -> Result, Vec> { + let mut errs: Vec = Vec::new(); + + let metrics = + filter_events_by_metric_and_component(telemetry_events, metric_type, component_name); + + let actual_events = sum_counters(metric_type, &metrics)?; + + debug!( + "{}: {} events, {} expected events.", + metric_type, actual_events, expected_events, + ); + + if actual_events != expected_events { + errs.push(format!( + "{}: expected {} events, but received {}", + metric_type, expected_events, actual_events + )); + } + + if !errs.is_empty() { + return Err(errs); + } + + Ok(vec![format!("{}: {}", metric_type, actual_events)]) +} + +fn validate_bytes_total( + telemetry_events: &[Event], + metric_type: &ComponentMetricType, + component_name: &str, + expected_bytes: u64, +) -> Result, Vec> { + let mut errs: Vec = Vec::new(); + + let metrics = + filter_events_by_metric_and_component(telemetry_events, metric_type, component_name); + + let actual_bytes = sum_counters(metric_type, &metrics)?; + + debug!( + "{}: {} bytes, {} expected bytes.", + metric_type, actual_bytes, expected_bytes, + ); + + if actual_bytes != expected_bytes { + errs.push(format!( + "{}: expected {} bytes, but received {}", + metric_type, expected_bytes, actual_bytes + )); + } + + if !errs.is_empty() { + return Err(errs); + } + + Ok(vec![format!("{}: {}", metric_type, actual_bytes)]) +} diff --git a/src/components/validation/validators/component_spec/sinks.rs b/src/components/validation/validators/component_spec/sinks.rs new file mode 100644 index 0000000000000..95545b37f5322 --- /dev/null +++ b/src/components/validation/validators/component_spec/sinks.rs @@ -0,0 +1,127 @@ +use vector_core::event::Event; + +// use crate::components::validation::{component_names::TEST_SINK_NAME, RunnerMetrics}; +use crate::components::validation::RunnerMetrics; + +// use super::{filter_events_by_metric_and_component, ComponentMetricType}; + +pub fn validate_sinks( + _telemetry_events: &[Event], + _runner_metrics: &RunnerMetrics, +) -> Result, Vec> { + let out: Vec = Vec::new(); + let errs: Vec = Vec::new(); + + // let validations = [ + // // validate_component_received_events_total, + // // validate_component_received_event_bytes_total, + // // validate_component_received_bytes_total, + // // validate_component_sent_events_total, + // // validate_component_sent_event_bytes_total, + // // validate_component_discarded_events_total, + // ]; + + // for v in validations.iter() { + // match v(telemetry_events, runner_metrics) { + // Err(e) => errs.extend(e), + // Ok(m) => out.extend(m), + // } + // } + + if errs.is_empty() { + Ok(out) + } else { + Err(errs) + } +} + +// fn validate_component_received_events_total( +// telemetry_events: &[Event], +// runner_metrics: &RunnerMetrics, +// ) -> Result, Vec> { +// // The reciprocal metric for events received is events sent, +// // so the expected value is what the input runner sent. +// let expected_events = runner_metrics.sent_events_total; + +// validate_events_total( +// telemetry_events, +// &SourceMetricType::EventsReceived, +// expected_events, +// TEST_SINK_NAME, +// ) +// } + +// fn validate_component_received_event_bytes_total( +// telemetry_events: &[Event], +// runner_metrics: &RunnerMetrics, +// ) -> Result, Vec> { +// // The reciprocal metric for received_event_bytes is sent_event_bytes, +// // so the expected value is what the input runner sent. +// let expected_bytes = runner_metrics.sent_event_bytes_total; + +// validate_bytes_total( +// telemetry_events, +// &SourceMetricType::EventsReceivedBytes, +// expected_bytes, +// ) +// } + +// fn validate_component_received_bytes_total( +// telemetry_events: &[Event], +// runner_metrics: &RunnerMetrics, +// ) -> Result, Vec> { +// // The reciprocal metric for received_bytes is sent_bytes, +// // so the expected value is what the input runner sent. +// let expected_bytes = runner_metrics.sent_bytes_total; + +// validate_bytes_total( +// telemetry_events, +// &SourceMetricType::ReceivedBytesTotal, +// expected_bytes, +// ) +// } + +// fn validate_component_sent_events_total( +// telemetry_events: &[Event], +// runner_metrics: &RunnerMetrics, +// ) -> Result, Vec> { +// // The reciprocal metric for events sent is events received, +// // so the expected value is what the output runner received. +// let expected_events = runner_metrics.received_events_total; + +// validate_events_total( +// telemetry_events, +// &SourceMetricType::SentEventsTotal, +// expected_events, +// ) +// } + +// fn validate_component_sent_event_bytes_total( +// telemetry_events: &[Event], +// runner_metrics: &RunnerMetrics, +// ) -> Result, Vec> { +// // The reciprocal metric for sent_event_bytes is received_event_bytes, +// // so the expected value is what the output runner received. +// let expected_bytes = runner_metrics.received_event_bytes_total; + +// validate_bytes_total( +// telemetry_events, +// &SourceMetricType::SentEventBytesTotal, +// expected_bytes, +// ) +// } + +// fn validate_component_discarded_events_total( +// telemetry_events: &[Event], +// runner_metrics: &RunnerMetrics, +// ) -> Result, Vec> { +// // The reciprocal metric for sent_event_bytes is received_event_bytes, +// // so the expected value is what the output runner received. +// let expected_dropped = runner_metrics.discarded_events_total; + +// validate_bytes_total( +// telemetry_events, +// &SourceMetricType::EventsDropped, +// expected_dropped, +// ) +// } diff --git a/src/components/validation/validators/component_spec/sources.rs b/src/components/validation/validators/component_spec/sources.rs index 6c1a3dca88399..31f648d615113 100644 --- a/src/components/validation/validators/component_spec/sources.rs +++ b/src/components/validation/validators/component_spec/sources.rs @@ -1,38 +1,8 @@ -use std::fmt::{Display, Formatter}; +use vector_core::event::Event; -use vector_core::event::{Event, MetricKind}; +use crate::components::validation::{component_names::TEST_SOURCE_NAME, RunnerMetrics}; -use crate::components::validation::RunnerMetrics; - -use super::filter_events_by_metric_and_component; - -const TEST_SOURCE_NAME: &str = "test_source"; - -pub enum SourceMetricType { - EventsReceived, - EventsReceivedBytes, - ReceivedBytesTotal, - SentEventsTotal, - SentEventBytesTotal, -} - -impl SourceMetricType { - const fn name(&self) -> &'static str { - match self { - SourceMetricType::EventsReceived => "component_received_events_total", - SourceMetricType::EventsReceivedBytes => "component_received_event_bytes_total", - SourceMetricType::ReceivedBytesTotal => "component_received_bytes_total", - SourceMetricType::SentEventsTotal => "component_sent_events_total", - SourceMetricType::SentEventBytesTotal => "component_sent_event_bytes_total", - } - } -} - -impl Display for SourceMetricType { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", self.name()) - } -} +use super::{validate_bytes_total, validate_events_total, ComponentMetricType}; pub fn validate_sources( telemetry_events: &[Event], @@ -63,95 +33,6 @@ pub fn validate_sources( } } -fn sum_counters( - metric_name: &SourceMetricType, - metrics: &[&vector_core::event::Metric], -) -> Result> { - let mut sum: f64 = 0.0; - let mut errs = Vec::new(); - - for m in metrics { - match m.value() { - vector_core::event::MetricValue::Counter { value } => { - if let MetricKind::Absolute = m.data().kind { - sum = *value; - } else { - sum += *value; - } - } - _ => errs.push(format!("{}: metric value is not a counter", metric_name,)), - } - } - - if errs.is_empty() { - Ok(sum as u64) - } else { - Err(errs) - } -} - -fn validate_events_total( - telemetry_events: &[Event], - metric_type: &SourceMetricType, - expected_events: u64, -) -> Result, Vec> { - let mut errs: Vec = Vec::new(); - - let metrics = - filter_events_by_metric_and_component(telemetry_events, metric_type, TEST_SOURCE_NAME); - - let actual_events = sum_counters(metric_type, &metrics)?; - - debug!( - "{}: {} events, {} expected events.", - metric_type, actual_events, expected_events, - ); - - if actual_events != expected_events { - errs.push(format!( - "{}: expected {} events, but received {}", - metric_type, expected_events, actual_events - )); - } - - if !errs.is_empty() { - return Err(errs); - } - - Ok(vec![format!("{}: {}", metric_type, actual_events)]) -} - -fn validate_bytes_total( - telemetry_events: &[Event], - metric_type: &SourceMetricType, - expected_bytes: u64, -) -> Result, Vec> { - let mut errs: Vec = Vec::new(); - - let metrics = - filter_events_by_metric_and_component(telemetry_events, metric_type, TEST_SOURCE_NAME); - - let actual_bytes = sum_counters(metric_type, &metrics)?; - - debug!( - "{}: {} bytes, {} expected bytes.", - metric_type, actual_bytes, expected_bytes, - ); - - if actual_bytes != expected_bytes { - errs.push(format!( - "{}: expected {} bytes, but received {}", - metric_type, expected_bytes, actual_bytes - )); - } - - if !errs.is_empty() { - return Err(errs); - } - - Ok(vec![format!("{}: {}", metric_type, actual_bytes)]) -} - fn validate_component_received_events_total( telemetry_events: &[Event], runner_metrics: &RunnerMetrics, @@ -162,7 +43,8 @@ fn validate_component_received_events_total( validate_events_total( telemetry_events, - &SourceMetricType::EventsReceived, + &ComponentMetricType::EventsReceived, + TEST_SOURCE_NAME, expected_events, ) } @@ -177,7 +59,8 @@ fn validate_component_received_event_bytes_total( validate_bytes_total( telemetry_events, - &SourceMetricType::EventsReceivedBytes, + &ComponentMetricType::EventsReceivedBytes, + TEST_SOURCE_NAME, expected_bytes, ) } @@ -192,7 +75,8 @@ fn validate_component_received_bytes_total( validate_bytes_total( telemetry_events, - &SourceMetricType::ReceivedBytesTotal, + &ComponentMetricType::ReceivedBytesTotal, + TEST_SOURCE_NAME, expected_bytes, ) } @@ -207,7 +91,8 @@ fn validate_component_sent_events_total( validate_events_total( telemetry_events, - &SourceMetricType::SentEventsTotal, + &ComponentMetricType::SentEventsTotal, + TEST_SOURCE_NAME, expected_events, ) } @@ -222,7 +107,8 @@ fn validate_component_sent_event_bytes_total( validate_bytes_total( telemetry_events, - &SourceMetricType::SentEventBytesTotal, + &ComponentMetricType::SentEventBytesTotal, + TEST_SOURCE_NAME, expected_bytes, ) } diff --git a/src/components/validation/validators/mod.rs b/src/components/validation/validators/mod.rs index e563488d7f416..714a835580502 100644 --- a/src/components/validation/validators/mod.rs +++ b/src/components/validation/validators/mod.rs @@ -2,6 +2,8 @@ mod component_spec; pub use self::component_spec::ComponentSpecValidator; +use std::fmt::{Display, Formatter}; + use vector_core::event::Event; use super::{ComponentType, RunnerMetrics, TestCaseExpectation, TestEvent}; @@ -49,3 +51,31 @@ impl From for Box { } } } + +pub enum ComponentMetricType { + EventsReceived, + EventsReceivedBytes, + ReceivedBytesTotal, + SentEventsTotal, + SentEventBytesTotal, + EventsDropped, +} + +impl ComponentMetricType { + const fn name(&self) -> &'static str { + match self { + ComponentMetricType::EventsReceived => "component_received_events_total", + ComponentMetricType::EventsReceivedBytes => "component_received_event_bytes_total", + ComponentMetricType::ReceivedBytesTotal => "component_received_bytes_total", + ComponentMetricType::SentEventsTotal => "component_sent_events_total", + ComponentMetricType::SentEventBytesTotal => "component_sent_event_bytes_total", + ComponentMetricType::EventsDropped => "component_discarded_events_total", + } + } +} + +impl Display for ComponentMetricType { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.name()) + } +} From c460a49fbefb40be2b5609f672c9875e961a24d8 Mon Sep 17 00:00:00 2001 From: neuronull Date: Fri, 14 Jul 2023 15:07:33 -0600 Subject: [PATCH 12/32] mini refactor of the component spec validators --- src/components/validation/mod.rs | 4 +- src/components/validation/resources/http.rs | 21 +- src/components/validation/runner/mod.rs | 5 + src/components/validation/runner/telemetry.rs | 2 +- .../validators/component_spec/mod.rs | 249 +++++++++++------- .../validators/component_spec/sinks.rs | 210 ++++++--------- .../validators/component_spec/sources.rs | 190 ++++++------- src/components/validation/validators/mod.rs | 2 + 8 files changed, 347 insertions(+), 336 deletions(-) diff --git a/src/components/validation/mod.rs b/src/components/validation/mod.rs index a824de0c057d6..8bf5e4130eaa5 100644 --- a/src/components/validation/mod.rs +++ b/src/components/validation/mod.rs @@ -181,12 +181,12 @@ macro_rules! register_validatable_component { /// Input and Output runners populate this structure as they send and receive events. /// The structure is passed into the validator to use as the expected values for the /// metrics that the components under test actually output. -#[derive(Default)] +#[derive(Default, Debug)] pub struct RunnerMetrics { pub received_events_total: u64, pub received_event_bytes_total: u64, pub received_bytes_total: u64, - pub sent_bytes_total: u64, // a reciprocal for received_bytes_total + pub sent_bytes_total: u64, pub sent_event_bytes_total: u64, pub sent_events_total: u64, } diff --git a/src/components/validation/resources/http.rs b/src/components/validation/resources/http.rs index 44e2b7ba7579b..4aa953efa9600 100644 --- a/src/components/validation/resources/http.rs +++ b/src/components/validation/resources/http.rs @@ -232,8 +232,19 @@ fn spawn_output_http_server( // First, we'll build and spawn our HTTP server. let decoder = codec.into_decoder(); - let (_, http_server_shutdown_tx) = - spawn_http_server(task_coordinator, &config, move |request| { + let (_, http_server_shutdown_tx) = spawn_http_server( + task_coordinator, + &config, + move |request| { + if request.headers().contains_key("Content-Length") { + let bytes = request + .headers() + .get("Content-Length") + .unwrap() + .to_str() + .unwrap(); + debug!("HTTP server external output resource received {bytes} bytes."); + } let output_tx = output_tx.clone(); let mut decoder = decoder.clone(); @@ -244,7 +255,8 @@ fn spawn_output_http_server( let mut body = BytesMut::from(&body[..]); loop { match decoder.decode_eof(&mut body) { - Ok(Some((events, _byte_size))) => { + Ok(Some((events, byte_size))) => { + debug!("HTTP server external output resource decoded {byte_size} bytes."); output_tx .send(events.to_vec()) .await @@ -257,7 +269,8 @@ fn spawn_output_http_server( } } } - }); + }, + ); // Now we'll create and spawn the resource's core logic loop which simply waits for the runner // to instruct us to shutdown, and when that happens, cascades to shutting down the HTTP server. diff --git a/src/components/validation/runner/mod.rs b/src/components/validation/runner/mod.rs index b4bcd72de75bf..d2ac50023f8f2 100644 --- a/src/components/validation/runner/mod.rs +++ b/src/components/validation/runner/mod.rs @@ -323,6 +323,9 @@ impl Runner { input_task_coordinator.shutdown().await; debug!("Input task(s) have been shutdown."); + // Without this, not all internal metric events are received for sink components under test. + tokio::time::sleep(Duration::from_secs(1)).await; + telemetry_task_coordinator.shutdown().await; debug!("Telemetry task(s) have been shutdown."); @@ -336,6 +339,8 @@ impl Runner { .await .expect("input driver task should not have panicked"); + debug!("Collected runner metrics: {:?}", runner_metrics); + // Run the relevant data -- inputs, outputs, telemetry, etc -- through each validator to // get the validation results for this test. let TestCase { diff --git a/src/components/validation/runner/telemetry.rs b/src/components/validation/runner/telemetry.rs index 76e8c29f62a9f..10c36001312b2 100644 --- a/src/components/validation/runner/telemetry.rs +++ b/src/components/validation/runner/telemetry.rs @@ -23,7 +23,7 @@ const INTERNAL_LOGS_KEY: &str = "_telemetry_logs"; const INTERNAL_METRICS_KEY: &str = "_telemetry_metrics"; const VECTOR_SINK_KEY: &str = "_telemetry_out"; -const SHUTDOWN_TICKS: u8 = 3; +const SHUTDOWN_TICKS: u8 = 5; /// Telemetry collector for a component under validation. pub struct Telemetry { diff --git a/src/components/validation/validators/component_spec/mod.rs b/src/components/validation/validators/component_spec/mod.rs index b1400a6ab847f..34670b1b2910f 100644 --- a/src/components/validation/validators/component_spec/mod.rs +++ b/src/components/validation/validators/component_spec/mod.rs @@ -7,8 +7,7 @@ use crate::components::validation::{ComponentType, RunnerMetrics, TestCaseExpect use super::{ComponentMetricType, Validator}; -use self::sinks::validate_sinks; -use self::sources::validate_sources; +use self::{sinks::SinkComponentMetricValidator, sources::SourceComponentMetricValidator}; /// Validates that the component meets the requirements of the [Component Specification][component_spec]. /// @@ -101,14 +100,20 @@ fn validate_telemetry( match component_type { ComponentType::Source => { - let result = validate_sources(telemetry_events, runner_metrics); + let result = validate_component::( + telemetry_events, + runner_metrics, + ); match result { Ok(o) => out.extend(o), Err(e) => errs.extend(e), } } ComponentType::Sink => { - let result = validate_sinks(telemetry_events, runner_metrics); + let result = validate_component::( + telemetry_events, + runner_metrics, + ); match result { Ok(o) => out.extend(o), Err(e) => errs.extend(e), @@ -124,125 +129,171 @@ fn validate_telemetry( } } -fn filter_events_by_metric_and_component<'a>( - telemetry_events: &'a [Event], - metric: &ComponentMetricType, - component_name: &'a str, -) -> Vec<&'a Metric> { - let metrics: Vec<&Metric> = telemetry_events - .iter() - .flat_map(|e| { - if let vector_core::event::Event::Metric(m) = e { - Some(m) - } else { - None - } - }) - .filter(|&m| { - if m.name() == metric.to_string() { - if let Some(tags) = m.tags() { - if tags.get("component_name").unwrap_or("") == component_name { - return true; +pub trait ComponentMetricValidator { + fn validate_metric( + telemetry_events: &[Event], + runner_metrics: &RunnerMetrics, + metric_type: &ComponentMetricType, + ) -> Result, Vec> + where + Self: Sized; + fn filter_events_by_metric_and_component<'a>( + telemetry_events: &'a [Event], + metric: &ComponentMetricType, + component_name: &'a str, + ) -> Vec<&'a Metric> { + let metrics: Vec<&Metric> = telemetry_events + .iter() + .flat_map(|e| { + if let vector_core::event::Event::Metric(m) = e { + Some(m) + } else { + None + } + }) + .filter(|&m| { + if m.name() == metric.to_string() { + if let Some(tags) = m.tags() { + if tags.get("component_name").unwrap_or("") == component_name { + return true; + } } } - } - false - }) - .collect(); + false + }) + .collect(); - debug!("{}: {} metrics found.", metric.to_string(), metrics.len(),); + debug!("{}: {} metrics found.", metric.to_string(), metrics.len(),); - metrics -} + metrics + } -fn sum_counters( - metric_name: &ComponentMetricType, - metrics: &[&vector_core::event::Metric], -) -> Result> { - let mut sum: f64 = 0.0; - let mut errs = Vec::new(); - - for m in metrics { - match m.value() { - vector_core::event::MetricValue::Counter { value } => { - if let MetricKind::Absolute = m.data().kind { - sum = *value; - } else { - sum += *value; + fn sum_counters( + metric_name: &ComponentMetricType, + metrics: &[&vector_core::event::Metric], + ) -> Result> { + let mut sum: f64 = 0.0; + let mut errs = Vec::new(); + + for m in metrics { + match m.value() { + vector_core::event::MetricValue::Counter { value } => { + if let MetricKind::Absolute = m.data().kind { + sum = *value; + } else { + sum += *value; + } } + _ => errs.push(format!("{}: metric value is not a counter", metric_name,)), } - _ => errs.push(format!("{}: metric value is not a counter", metric_name,)), } - } - if errs.is_empty() { - Ok(sum as u64) - } else { - Err(errs) + if errs.is_empty() { + Ok(sum as u64) + } else { + Err(errs) + } } -} -fn validate_events_total( - telemetry_events: &[Event], - metric_type: &ComponentMetricType, - component_name: &str, - expected_events: u64, -) -> Result, Vec> { - let mut errs: Vec = Vec::new(); + fn validate_events_total( + telemetry_events: &[Event], + metric_type: &ComponentMetricType, + component_name: &str, + expected_events: u64, + ) -> Result, Vec> { + let mut errs: Vec = Vec::new(); + + let metrics = Self::filter_events_by_metric_and_component( + telemetry_events, + metric_type, + component_name, + ); + + let actual_events = Self::sum_counters(metric_type, &metrics)?; + + debug!( + "{}: {} events, {} expected events.", + metric_type, actual_events, expected_events, + ); + + if actual_events != expected_events { + errs.push(format!( + "{}: expected {} events, but received {}", + metric_type, expected_events, actual_events + )); + } - let metrics = - filter_events_by_metric_and_component(telemetry_events, metric_type, component_name); + if !errs.is_empty() { + return Err(errs); + } - let actual_events = sum_counters(metric_type, &metrics)?; + Ok(vec![format!("{}: {}", metric_type, actual_events)]) + } - debug!( - "{}: {} events, {} expected events.", - metric_type, actual_events, expected_events, - ); + fn validate_bytes_total( + telemetry_events: &[Event], + metric_type: &ComponentMetricType, + component_name: &str, + expected_bytes: u64, + ) -> Result, Vec> { + let mut errs: Vec = Vec::new(); + + let metrics = Self::filter_events_by_metric_and_component( + telemetry_events, + metric_type, + component_name, + ); + + let actual_bytes = Self::sum_counters(metric_type, &metrics)?; + + debug!( + "{}: {} bytes, {} expected bytes.", + metric_type, actual_bytes, expected_bytes, + ); + + if actual_bytes != expected_bytes { + errs.push(format!( + "{}: expected {} bytes, but received {}", + metric_type, expected_bytes, actual_bytes + )); + } - if actual_events != expected_events { - errs.push(format!( - "{}: expected {} events, but received {}", - metric_type, expected_events, actual_events - )); - } + if !errs.is_empty() { + return Err(errs); + } - if !errs.is_empty() { - return Err(errs); + Ok(vec![format!("{}: {}", metric_type, actual_bytes)]) } - - Ok(vec![format!("{}: {}", metric_type, actual_events)]) } -fn validate_bytes_total( +pub fn validate_component( telemetry_events: &[Event], - metric_type: &ComponentMetricType, - component_name: &str, - expected_bytes: u64, + runner_metrics: &RunnerMetrics, ) -> Result, Vec> { + let mut out: Vec = Vec::new(); let mut errs: Vec = Vec::new(); - let metrics = - filter_events_by_metric_and_component(telemetry_events, metric_type, component_name); - - let actual_bytes = sum_counters(metric_type, &metrics)?; - - debug!( - "{}: {} bytes, {} expected bytes.", - metric_type, actual_bytes, expected_bytes, - ); - - if actual_bytes != expected_bytes { - errs.push(format!( - "{}: expected {} bytes, but received {}", - metric_type, expected_bytes, actual_bytes - )); - } + let metric_types = [ + ComponentMetricType::EventsReceived, + ComponentMetricType::EventsReceivedBytes, + ComponentMetricType::ReceivedBytesTotal, + ComponentMetricType::SentEventsTotal, + ComponentMetricType::SentEventBytesTotal, + ComponentMetricType::SentBytesTotal, + ComponentMetricType::EventsDropped, + ]; + + metric_types.iter().for_each(|metric_type| { + match V::validate_metric(telemetry_events, runner_metrics, metric_type) { + Err(e) => errs.extend(e), + Ok(m) => out.extend(m), + } + }); - if !errs.is_empty() { - return Err(errs); + if errs.is_empty() { + Ok(out) + } else { + Err(errs) } - - Ok(vec![format!("{}: {}", metric_type, actual_bytes)]) } diff --git a/src/components/validation/validators/component_spec/sinks.rs b/src/components/validation/validators/component_spec/sinks.rs index 95545b37f5322..771624e0619ad 100644 --- a/src/components/validation/validators/component_spec/sinks.rs +++ b/src/components/validation/validators/component_spec/sinks.rs @@ -1,127 +1,91 @@ use vector_core::event::Event; -// use crate::components::validation::{component_names::TEST_SINK_NAME, RunnerMetrics}; -use crate::components::validation::RunnerMetrics; - -// use super::{filter_events_by_metric_and_component, ComponentMetricType}; - -pub fn validate_sinks( - _telemetry_events: &[Event], - _runner_metrics: &RunnerMetrics, -) -> Result, Vec> { - let out: Vec = Vec::new(); - let errs: Vec = Vec::new(); - - // let validations = [ - // // validate_component_received_events_total, - // // validate_component_received_event_bytes_total, - // // validate_component_received_bytes_total, - // // validate_component_sent_events_total, - // // validate_component_sent_event_bytes_total, - // // validate_component_discarded_events_total, - // ]; - - // for v in validations.iter() { - // match v(telemetry_events, runner_metrics) { - // Err(e) => errs.extend(e), - // Ok(m) => out.extend(m), - // } - // } - - if errs.is_empty() { - Ok(out) - } else { - Err(errs) +use crate::components::validation::{component_names::TEST_SINK_NAME, RunnerMetrics}; + +use super::{ComponentMetricType, ComponentMetricValidator}; + +pub struct SinkComponentMetricValidator; + +impl ComponentMetricValidator for SinkComponentMetricValidator { + fn validate_metric( + telemetry_events: &[Event], + runner_metrics: &RunnerMetrics, + metric_type: &ComponentMetricType, + ) -> Result, Vec> { + match metric_type { + ComponentMetricType::EventsReceived => { + // Since the runner is on the same "side" of the topology as a sink is, + // the expected value is what the input runner received. + // let expected_events = runner_metrics.received_events_total; + let expected_events = runner_metrics.sent_events_total; + + Self::validate_events_total( + telemetry_events, + &ComponentMetricType::EventsReceived, + TEST_SINK_NAME, + expected_events, + ) + } + ComponentMetricType::EventsReceivedBytes => { + // Since the runner is on the same "side" of the topology as a sink is, + // the expected value is what the input runner received. + let expected_bytes = runner_metrics.received_event_bytes_total; + + Self::validate_bytes_total( + telemetry_events, + &ComponentMetricType::EventsReceivedBytes, + TEST_SINK_NAME, + expected_bytes, + ) + } + ComponentMetricType::ReceivedBytesTotal => { + Self::validate_bytes_total( + telemetry_events, + &ComponentMetricType::ReceivedBytesTotal, + TEST_SINK_NAME, + 0, // sinks should not emit this metric + ) + } + ComponentMetricType::SentEventsTotal => { + // Since the runner is on the same "side" of the topology as a sink is, + // the expected value is what the input runner sent. + let expected_events = runner_metrics.received_events_total; + + Self::validate_events_total( + telemetry_events, + &ComponentMetricType::SentEventsTotal, + TEST_SINK_NAME, + expected_events, + ) + } + ComponentMetricType::SentBytesTotal => { + // Since the runner is on the same "side" of the topology as a sink is, + // the expected value is what the input runner sent. + let expected_bytes = runner_metrics.sent_bytes_total; + + Self::validate_bytes_total( + telemetry_events, + &ComponentMetricType::SentBytesTotal, + TEST_SINK_NAME, + expected_bytes, + ) + } + ComponentMetricType::SentEventBytesTotal => { + // Since the runner is on the same "side" of the topology as a sink is, + // the expected value is what the input runner sent. + let expected_bytes = runner_metrics.received_bytes_total; + + Self::validate_bytes_total( + telemetry_events, + &ComponentMetricType::SentEventBytesTotal, + TEST_SINK_NAME, + expected_bytes, + ) + } + ComponentMetricType::EventsDropped => { + // TODO + Ok(vec![]) + } + } } } - -// fn validate_component_received_events_total( -// telemetry_events: &[Event], -// runner_metrics: &RunnerMetrics, -// ) -> Result, Vec> { -// // The reciprocal metric for events received is events sent, -// // so the expected value is what the input runner sent. -// let expected_events = runner_metrics.sent_events_total; - -// validate_events_total( -// telemetry_events, -// &SourceMetricType::EventsReceived, -// expected_events, -// TEST_SINK_NAME, -// ) -// } - -// fn validate_component_received_event_bytes_total( -// telemetry_events: &[Event], -// runner_metrics: &RunnerMetrics, -// ) -> Result, Vec> { -// // The reciprocal metric for received_event_bytes is sent_event_bytes, -// // so the expected value is what the input runner sent. -// let expected_bytes = runner_metrics.sent_event_bytes_total; - -// validate_bytes_total( -// telemetry_events, -// &SourceMetricType::EventsReceivedBytes, -// expected_bytes, -// ) -// } - -// fn validate_component_received_bytes_total( -// telemetry_events: &[Event], -// runner_metrics: &RunnerMetrics, -// ) -> Result, Vec> { -// // The reciprocal metric for received_bytes is sent_bytes, -// // so the expected value is what the input runner sent. -// let expected_bytes = runner_metrics.sent_bytes_total; - -// validate_bytes_total( -// telemetry_events, -// &SourceMetricType::ReceivedBytesTotal, -// expected_bytes, -// ) -// } - -// fn validate_component_sent_events_total( -// telemetry_events: &[Event], -// runner_metrics: &RunnerMetrics, -// ) -> Result, Vec> { -// // The reciprocal metric for events sent is events received, -// // so the expected value is what the output runner received. -// let expected_events = runner_metrics.received_events_total; - -// validate_events_total( -// telemetry_events, -// &SourceMetricType::SentEventsTotal, -// expected_events, -// ) -// } - -// fn validate_component_sent_event_bytes_total( -// telemetry_events: &[Event], -// runner_metrics: &RunnerMetrics, -// ) -> Result, Vec> { -// // The reciprocal metric for sent_event_bytes is received_event_bytes, -// // so the expected value is what the output runner received. -// let expected_bytes = runner_metrics.received_event_bytes_total; - -// validate_bytes_total( -// telemetry_events, -// &SourceMetricType::SentEventBytesTotal, -// expected_bytes, -// ) -// } - -// fn validate_component_discarded_events_total( -// telemetry_events: &[Event], -// runner_metrics: &RunnerMetrics, -// ) -> Result, Vec> { -// // The reciprocal metric for sent_event_bytes is received_event_bytes, -// // so the expected value is what the output runner received. -// let expected_dropped = runner_metrics.discarded_events_total; - -// validate_bytes_total( -// telemetry_events, -// &SourceMetricType::EventsDropped, -// expected_dropped, -// ) -// } diff --git a/src/components/validation/validators/component_spec/sources.rs b/src/components/validation/validators/component_spec/sources.rs index 31f648d615113..851462f82fbf6 100644 --- a/src/components/validation/validators/component_spec/sources.rs +++ b/src/components/validation/validators/component_spec/sources.rs @@ -2,113 +2,89 @@ use vector_core::event::Event; use crate::components::validation::{component_names::TEST_SOURCE_NAME, RunnerMetrics}; -use super::{validate_bytes_total, validate_events_total, ComponentMetricType}; - -pub fn validate_sources( - telemetry_events: &[Event], - runner_metrics: &RunnerMetrics, -) -> Result, Vec> { - let mut out: Vec = Vec::new(); - let mut errs: Vec = Vec::new(); - - let validations = [ - validate_component_received_events_total, - validate_component_received_event_bytes_total, - validate_component_received_bytes_total, - validate_component_sent_events_total, - validate_component_sent_event_bytes_total, - ]; - - for v in validations.iter() { - match v(telemetry_events, runner_metrics) { - Err(e) => errs.extend(e), - Ok(m) => out.extend(m), +use super::{ComponentMetricType, ComponentMetricValidator}; + +pub struct SourceComponentMetricValidator; + +impl ComponentMetricValidator for SourceComponentMetricValidator { + fn validate_metric( + telemetry_events: &[Event], + runner_metrics: &RunnerMetrics, + metric_type: &ComponentMetricType, + ) -> Result, Vec> { + match metric_type { + ComponentMetricType::EventsReceived => { + // The reciprocal metric for events received is events sent, + // so the expected value is what the input runner sent. + let expected_events = runner_metrics.sent_events_total; + + Self::validate_events_total( + telemetry_events, + &ComponentMetricType::EventsReceived, + TEST_SOURCE_NAME, + expected_events, + ) + } + ComponentMetricType::EventsReceivedBytes => { + // The reciprocal metric for received_event_bytes is sent_event_bytes, + // so the expected value is what the input runner sent. + let expected_bytes = runner_metrics.sent_event_bytes_total; + + Self::validate_bytes_total( + telemetry_events, + &ComponentMetricType::EventsReceivedBytes, + TEST_SOURCE_NAME, + expected_bytes, + ) + } + ComponentMetricType::ReceivedBytesTotal => { + // The reciprocal metric for received_bytes is sent_bytes, + // so the expected value is what the input runner sent. + let expected_bytes = runner_metrics.sent_bytes_total; + + Self::validate_bytes_total( + telemetry_events, + &ComponentMetricType::ReceivedBytesTotal, + TEST_SOURCE_NAME, + expected_bytes, + ) + } + ComponentMetricType::SentEventsTotal => { + // The reciprocal metric for events sent is events received, + // so the expected value is what the output runner received. + let expected_events = runner_metrics.received_events_total; + + Self::validate_events_total( + telemetry_events, + &ComponentMetricType::SentEventsTotal, + TEST_SOURCE_NAME, + expected_events, + ) + } + ComponentMetricType::SentBytesTotal => { + Self::validate_bytes_total( + telemetry_events, + &ComponentMetricType::SentBytesTotal, + TEST_SOURCE_NAME, + 0, // sources should not emit this metric + ) + } + ComponentMetricType::SentEventBytesTotal => { + // The reciprocal metric for sent_event_bytes is received_event_bytes, + // so the expected value is what the output runner received. + let expected_bytes = runner_metrics.received_event_bytes_total; + + Self::validate_bytes_total( + telemetry_events, + &ComponentMetricType::SentEventBytesTotal, + TEST_SOURCE_NAME, + expected_bytes, + ) + } + ComponentMetricType::EventsDropped => { + // TODO + Ok(vec![]) + } } } - - if errs.is_empty() { - Ok(out) - } else { - Err(errs) - } -} - -fn validate_component_received_events_total( - telemetry_events: &[Event], - runner_metrics: &RunnerMetrics, -) -> Result, Vec> { - // The reciprocal metric for events received is events sent, - // so the expected value is what the input runner sent. - let expected_events = runner_metrics.sent_events_total; - - validate_events_total( - telemetry_events, - &ComponentMetricType::EventsReceived, - TEST_SOURCE_NAME, - expected_events, - ) -} - -fn validate_component_received_event_bytes_total( - telemetry_events: &[Event], - runner_metrics: &RunnerMetrics, -) -> Result, Vec> { - // The reciprocal metric for received_event_bytes is sent_event_bytes, - // so the expected value is what the input runner sent. - let expected_bytes = runner_metrics.sent_event_bytes_total; - - validate_bytes_total( - telemetry_events, - &ComponentMetricType::EventsReceivedBytes, - TEST_SOURCE_NAME, - expected_bytes, - ) -} - -fn validate_component_received_bytes_total( - telemetry_events: &[Event], - runner_metrics: &RunnerMetrics, -) -> Result, Vec> { - // The reciprocal metric for received_bytes is sent_bytes, - // so the expected value is what the input runner sent. - let expected_bytes = runner_metrics.sent_bytes_total; - - validate_bytes_total( - telemetry_events, - &ComponentMetricType::ReceivedBytesTotal, - TEST_SOURCE_NAME, - expected_bytes, - ) -} - -fn validate_component_sent_events_total( - telemetry_events: &[Event], - runner_metrics: &RunnerMetrics, -) -> Result, Vec> { - // The reciprocal metric for events sent is events received, - // so the expected value is what the output runner received. - let expected_events = runner_metrics.received_events_total; - - validate_events_total( - telemetry_events, - &ComponentMetricType::SentEventsTotal, - TEST_SOURCE_NAME, - expected_events, - ) -} - -fn validate_component_sent_event_bytes_total( - telemetry_events: &[Event], - runner_metrics: &RunnerMetrics, -) -> Result, Vec> { - // The reciprocal metric for sent_event_bytes is received_event_bytes, - // so the expected value is what the output runner received. - let expected_bytes = runner_metrics.received_event_bytes_total; - - validate_bytes_total( - telemetry_events, - &ComponentMetricType::SentEventBytesTotal, - TEST_SOURCE_NAME, - expected_bytes, - ) } diff --git a/src/components/validation/validators/mod.rs b/src/components/validation/validators/mod.rs index 714a835580502..4e82a29aaa691 100644 --- a/src/components/validation/validators/mod.rs +++ b/src/components/validation/validators/mod.rs @@ -57,6 +57,7 @@ pub enum ComponentMetricType { EventsReceivedBytes, ReceivedBytesTotal, SentEventsTotal, + SentBytesTotal, SentEventBytesTotal, EventsDropped, } @@ -68,6 +69,7 @@ impl ComponentMetricType { ComponentMetricType::EventsReceivedBytes => "component_received_event_bytes_total", ComponentMetricType::ReceivedBytesTotal => "component_received_bytes_total", ComponentMetricType::SentEventsTotal => "component_sent_events_total", + ComponentMetricType::SentBytesTotal => "component_sent_bytes_total", ComponentMetricType::SentEventBytesTotal => "component_sent_event_bytes_total", ComponentMetricType::EventsDropped => "component_discarded_events_total", } From 3daced5b4e1f9815e117f8a091861fc97ea42439 Mon Sep 17 00:00:00 2001 From: neuronull Date: Fri, 14 Jul 2023 16:58:44 -0600 Subject: [PATCH 13/32] attempt to set expected values from the resource --- src/components/validation/resources/http.rs | 49 +++++++++++++--- src/components/validation/resources/mod.rs | 33 ++++++++--- src/components/validation/runner/mod.rs | 56 ++++++++++++------- .../validators/component_spec/sinks.rs | 6 +- 4 files changed, 103 insertions(+), 41 deletions(-) diff --git a/src/components/validation/resources/http.rs b/src/components/validation/resources/http.rs index 4aa953efa9600..c28159f628154 100644 --- a/src/components/validation/resources/http.rs +++ b/src/components/validation/resources/http.rs @@ -20,7 +20,10 @@ use tokio::{ }; use tokio_util::codec::Decoder; -use crate::components::validation::sync::{Configuring, TaskCoordinator}; +use crate::components::validation::{ + sync::{Configuring, TaskCoordinator}, + RunnerMetrics, +}; use vector_core::event::Event; use super::{encode_test_event, ResourceCodec, ResourceDirection, TestEvent}; @@ -43,11 +46,12 @@ impl HttpResourceConfig { codec: ResourceCodec, input_rx: mpsc::Receiver, task_coordinator: &TaskCoordinator, + runner_metrics: &Arc>, ) { match direction { // The source will pull data from us. ResourceDirection::Pull => { - spawn_input_http_server(self, codec, input_rx, task_coordinator) + spawn_input_http_server(self, codec, input_rx, task_coordinator, runner_metrics) } // We'll push data to the source. ResourceDirection::Push => { @@ -62,6 +66,7 @@ impl HttpResourceConfig { codec: ResourceCodec, output_tx: mpsc::Sender>, task_coordinator: &TaskCoordinator, + runner_metrics: &Arc>, ) { match direction { // We'll pull data from the sink. @@ -70,7 +75,7 @@ impl HttpResourceConfig { } // The sink will push data to us. ResourceDirection::Push => { - spawn_output_http_server(self, codec, output_tx, task_coordinator) + spawn_output_http_server(self, codec, output_tx, task_coordinator, runner_metrics) } } } @@ -83,6 +88,7 @@ fn spawn_input_http_server( codec: ResourceCodec, mut input_rx: mpsc::Receiver, task_coordinator: &TaskCoordinator, + runner_metrics: &Arc>, ) { // This HTTP server will poll the input receiver for input events and buffer them. When a // request comes in on the right path/method, one buffered input event will be sent back. If no @@ -94,8 +100,11 @@ fn spawn_input_http_server( let encoder = codec.into_encoder(); let sendable_events = Arc::clone(&outstanding_events); - let (resource_notifier, http_server_shutdown_tx) = - spawn_http_server(task_coordinator, &config, move |_| { + let (resource_notifier, http_server_shutdown_tx) = spawn_http_server( + task_coordinator, + &config, + runner_metrics, + move |_request, _runner_metrics| { let sendable_events = Arc::clone(&sendable_events); let mut encoder = encoder.clone(); @@ -111,7 +120,8 @@ fn spawn_input_http_server( StatusCode::NO_CONTENT.into_response() } } - }); + }, + ); // Now we'll create and spawn the resource's core logic loop which drives the buffering of input // events and working with the HTTP server as they're consumed. @@ -225,6 +235,7 @@ fn spawn_output_http_server( codec: ResourceCodec, output_tx: mpsc::Sender>, task_coordinator: &TaskCoordinator, + runner_metrics: &Arc>, ) { // This HTTP server will wait for events to be sent by a sink, and collect them and send them on // via an output sender. We accept/collect events until we're told to shutdown. @@ -235,7 +246,8 @@ fn spawn_output_http_server( let (_, http_server_shutdown_tx) = spawn_http_server( task_coordinator, &config, - move |request| { + runner_metrics, + move |request, output_runner_metrics| { if request.headers().contains_key("Content-Length") { let bytes = request .headers() @@ -249,6 +261,7 @@ fn spawn_output_http_server( let mut decoder = decoder.clone(); async move { + let mut output_runner_metrics = output_runner_metrics.lock().await; match hyper::body::to_bytes(request.into_body()).await { Err(_) => StatusCode::INTERNAL_SERVER_ERROR.into_response(), Ok(body) => { @@ -257,6 +270,21 @@ fn spawn_output_http_server( match decoder.decode_eof(&mut body) { Ok(Some((events, byte_size))) => { debug!("HTTP server external output resource decoded {byte_size} bytes."); + + // Update the runner metrics for the received events. This will later + // be used in the Validators, as the "expected" case. + output_runner_metrics.received_bytes_total += byte_size as u64; + output_runner_metrics.received_events_total += + events.len() as u64; + + // TODO: I think we will need to encode the events received and get the value + // of the buffer length, to get the expected value for `component_sent_event_bytes_total` + // Unfortunately that means we'll have to pass the encoder down to here as well. + + // output_runner_metrics.received_event_bytes_total += + // events.to_vec().estimated_json_encoded_size_of().get() + // as u64; + output_tx .send(events.to_vec()) .await @@ -306,10 +334,11 @@ fn spawn_output_http_client( fn spawn_http_server( task_coordinator: &TaskCoordinator, config: &HttpResourceConfig, + runner_metrics: &Arc>, handler: H, ) -> (Arc, oneshot::Sender<()>) where - H: Fn(Request) -> F + Clone + Send + 'static, + H: Fn(Request, Arc>) -> F + Clone + Send + 'static, F: Future + Send, R: IntoResponse, { @@ -334,6 +363,8 @@ where let resource_notifier = Arc::new(Notify::new()); let server_notifier = Arc::clone(&resource_notifier); + let output_runner_metrics = Arc::clone(runner_metrics); + tokio::spawn(async move { // Create our HTTP server by binding as early as possible to return an error if we can't // actually bind. @@ -360,7 +391,7 @@ where StatusCode::METHOD_NOT_ALLOWED }) .on(method_filter, move |request: Request| { - let request_handler = handler(request); + let request_handler = handler(request, output_runner_metrics); let notifier = Arc::clone(&server_notifier); async move { diff --git a/src/components/validation/resources/mod.rs b/src/components/validation/resources/mod.rs index a22d6fc324dbd..7603d3666eedf 100644 --- a/src/components/validation/resources/mod.rs +++ b/src/components/validation/resources/mod.rs @@ -1,6 +1,8 @@ mod event; mod http; +use std::sync::Arc; + use codecs::{ decoding::{self, DeserializerConfig}, encoding::{ @@ -8,7 +10,7 @@ use codecs::{ }, BytesEncoder, }; -use tokio::sync::mpsc; +use tokio::sync::{mpsc, Mutex}; use vector_core::{config::DataType, event::Event}; use crate::codecs::{Decoder, DecodingConfig, Encoder, EncodingConfig, EncodingConfigWithFraming}; @@ -16,7 +18,10 @@ use crate::codecs::{Decoder, DecodingConfig, Encoder, EncodingConfig, EncodingCo pub use self::event::{encode_test_event, TestEvent}; pub use self::http::HttpResourceConfig; -use super::sync::{Configuring, TaskCoordinator}; +use super::{ + sync::{Configuring, TaskCoordinator}, + RunnerMetrics, +}; /// The codec used by the external resource. /// @@ -273,7 +278,7 @@ impl From for ResourceDefinition { /// the external resource must pull the data from the sink. #[derive(Clone)] pub struct ExternalResource { - direction: ResourceDirection, + pub direction: ResourceDirection, definition: ResourceDefinition, pub codec: ResourceCodec, } @@ -297,11 +302,16 @@ impl ExternalResource { self, input_rx: mpsc::Receiver, task_coordinator: &TaskCoordinator, + runner_metrics: &Arc>, ) { match self.definition { - ResourceDefinition::Http(http_config) => { - http_config.spawn_as_input(self.direction, self.codec, input_rx, task_coordinator) - } + ResourceDefinition::Http(http_config) => http_config.spawn_as_input( + self.direction, + self.codec, + input_rx, + task_coordinator, + runner_metrics, + ), } } @@ -310,11 +320,16 @@ impl ExternalResource { self, output_tx: mpsc::Sender>, task_coordinator: &TaskCoordinator, + runner_metrics: &Arc>, ) { match self.definition { - ResourceDefinition::Http(http_config) => { - http_config.spawn_as_output(self.direction, self.codec, output_tx, task_coordinator) - } + ResourceDefinition::Http(http_config) => http_config.spawn_as_output( + self.direction, + self.codec, + output_tx, + task_coordinator, + runner_metrics, + ), } } } diff --git a/src/components/validation/runner/mod.rs b/src/components/validation/runner/mod.rs index d2ac50023f8f2..9b2d08a18714d 100644 --- a/src/components/validation/runner/mod.rs +++ b/src/components/validation/runner/mod.rs @@ -2,18 +2,16 @@ pub mod config; mod io; mod telemetry; -use std::{ - collections::HashMap, - path::PathBuf, - sync::{Arc, Mutex}, - time::Duration, -}; +use std::{collections::HashMap, path::PathBuf, sync::Arc, time::Duration}; use bytes::BytesMut; use tokio::{ runtime::Builder, select, - sync::mpsc::{self, Receiver, Sender}, + sync::{ + mpsc::{self, Receiver, Sender}, + Mutex, + }, task::JoinHandle, }; use tokio_util::codec::Encoder as _; @@ -251,6 +249,7 @@ impl Runner { &self.configuration, &input_task_coordinator, &output_task_coordinator, + &runner_metrics, ); let input_tx = runner_input.into_sender(controlled_edges.input); let output_rx = runner_output.into_receiver(controlled_edges.output); @@ -294,17 +293,25 @@ impl Runner { // around if we can avoid it. tokio::time::sleep(Duration::from_secs(2)).await; + let skip_input_driver_metrics = + self.configuration.component_type == ComponentType::Source; + let input_driver = spawn_input_driver( test_case.events.clone(), input_tx, &runner_metrics, maybe_runner_encoder.as_ref().cloned(), + skip_input_driver_metrics, ); + let skip_output_driver_metrics = + self.configuration.component_type == ComponentType::Sink; + let output_driver = spawn_output_driver( output_rx, &runner_metrics, maybe_runner_encoder.as_ref().cloned(), + skip_output_driver_metrics, ); // At this point, the component topology is running, and all input/output/telemetry @@ -350,6 +357,8 @@ impl Runner { } = test_case; let telemetry_events = telemetry_collector.collect().await; + let final_runner_metrics = runner_metrics.lock().await; + let validator_results = self .validators .values() @@ -360,7 +369,7 @@ impl Runner { &input_events, &output_events, &telemetry_events, - &runner_metrics.lock().unwrap(), + &final_runner_metrics, ) }) .collect(); @@ -418,6 +427,7 @@ fn build_external_resource( configuration: &ValidationConfiguration, input_task_coordinator: &TaskCoordinator, output_task_coordinator: &TaskCoordinator, + runner_metrics: &Arc>, ) -> (RunnerInput, RunnerOutput, Option>) { let component_type = configuration.component_type(); let maybe_external_resource = configuration.external_resource(); @@ -433,7 +443,7 @@ fn build_external_resource( let (tx, rx) = mpsc::channel(1024); let resource = maybe_external_resource.expect("a source must always have an external resource"); - resource.spawn_as_input(rx, input_task_coordinator); + resource.spawn_as_input(rx, input_task_coordinator, runner_metrics); ( RunnerInput::External(tx), @@ -453,7 +463,7 @@ fn build_external_resource( let (tx, rx) = mpsc::channel(1024); let resource = maybe_external_resource.expect("a sink must always have an external resource"); - resource.spawn_as_output(tx, output_task_coordinator); + resource.spawn_as_output(tx, output_task_coordinator, runner_metrics); ( RunnerInput::Controlled, @@ -520,6 +530,7 @@ fn spawn_input_driver( input_tx: Sender, runner_metrics: &Arc>, mut maybe_encoder: Option>, + _skip_input_driver_metrics: bool, ) -> JoinHandle<()> { let input_runner_metrics = Arc::clone(runner_metrics); @@ -532,7 +543,7 @@ fn spawn_input_driver( // Update the runner metrics for the sent event. This will later // be used in the Validators, as the "expected" case. - let mut input_runner_metrics = input_runner_metrics.lock().unwrap(); + let mut input_runner_metrics = input_runner_metrics.lock().await; if let Some(encoder) = maybe_encoder.as_mut() { let mut buffer = BytesMut::new(); @@ -561,6 +572,7 @@ fn spawn_output_driver( mut output_rx: Receiver>, runner_metrics: &Arc>, maybe_encoder: Option>, + skip_output_driver_metrics: bool, ) -> JoinHandle> { let output_runner_metrics = Arc::clone(runner_metrics); @@ -571,23 +583,25 @@ fn spawn_output_driver( // Update the runner metrics for the received event. This will later // be used in the Validators, as the "expected" case. - let mut output_runner_metrics = output_runner_metrics.lock().unwrap(); + let mut output_runner_metrics = output_runner_metrics.lock().await; for output_event in events { - output_runner_metrics.received_events_total += 1; output_runner_metrics.received_event_bytes_total += vec![output_event.clone()] .estimated_json_encoded_size_of() .get() as u64; - if let Some(encoder) = maybe_encoder.as_ref() { - let mut buffer = BytesMut::new(); - encoder - .clone() - .encode(output_event, &mut buffer) - .expect("should not fail to encode output event"); - - output_runner_metrics.received_bytes_total += buffer.len() as u64; + if !skip_output_driver_metrics { + if let Some(encoder) = maybe_encoder.as_ref() { + let mut buffer = BytesMut::new(); + encoder + .clone() + .encode(output_event, &mut buffer) + .expect("should not fail to encode output event"); + + output_runner_metrics.received_events_total += 1; + output_runner_metrics.received_bytes_total += buffer.len() as u64; + } } } } diff --git a/src/components/validation/validators/component_spec/sinks.rs b/src/components/validation/validators/component_spec/sinks.rs index 771624e0619ad..d65840c014ae2 100644 --- a/src/components/validation/validators/component_spec/sinks.rs +++ b/src/components/validation/validators/component_spec/sinks.rs @@ -61,7 +61,7 @@ impl ComponentMetricValidator for SinkComponentMetricValidator { ComponentMetricType::SentBytesTotal => { // Since the runner is on the same "side" of the topology as a sink is, // the expected value is what the input runner sent. - let expected_bytes = runner_metrics.sent_bytes_total; + let expected_bytes = runner_metrics.received_bytes_total; Self::validate_bytes_total( telemetry_events, @@ -73,7 +73,9 @@ impl ComponentMetricValidator for SinkComponentMetricValidator { ComponentMetricType::SentEventBytesTotal => { // Since the runner is on the same "side" of the topology as a sink is, // the expected value is what the input runner sent. - let expected_bytes = runner_metrics.received_bytes_total; + let expected_bytes = 0; + + // TODO: see TODO in resources/http.rs for idea on how to get this value Self::validate_bytes_total( telemetry_events, From b7a7bd303ccf1e1976bb1d5f4c899cf3278a5397 Mon Sep 17 00:00:00 2001 From: neuronull Date: Wed, 19 Jul 2023 09:39:41 -0600 Subject: [PATCH 14/32] feedback tz- from not try_from --- src/components/validation/resources/event.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/components/validation/resources/event.rs b/src/components/validation/resources/event.rs index beb9c244c2fbd..1f91acfbe660d 100644 --- a/src/components/validation/resources/event.rs +++ b/src/components/validation/resources/event.rs @@ -53,7 +53,7 @@ impl EventData { /// from the event data twice (once for the expected and once for actual), it can result in a timestamp in /// the event which may or may not have the same millisecond precision as it's counterpart. #[derive(Clone, Debug, Deserialize)] -#[serde(try_from = "RawTestEvent")] +#[serde(from = "RawTestEvent")] #[serde(untagged)] pub enum TestEvent { /// The event is used, as-is, without modification. @@ -84,11 +84,9 @@ impl TestEvent { #[derive(Clone, Debug, Eq, PartialEq, Snafu)] pub enum RawTestEventParseError {} -impl TryFrom for TestEvent { - type Error = RawTestEventParseError; - - fn try_from(other: RawTestEvent) -> Result { - Ok(match other { +impl From for TestEvent { + fn from(other: RawTestEvent) -> Self { + match other { RawTestEvent::Passthrough(event_data) => { TestEvent::Passthrough(event_data.into_event()) } @@ -96,7 +94,7 @@ impl TryFrom for TestEvent { modified, event: event.into_event(), }, - }) + } } } From 0ce0e25f98ee265a621dd3905bc21fe1665cabf4 Mon Sep 17 00:00:00 2001 From: neuronull Date: Wed, 19 Jul 2023 10:10:50 -0600 Subject: [PATCH 15/32] back to 3 ticks --- src/components/validation/runner/telemetry.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/validation/runner/telemetry.rs b/src/components/validation/runner/telemetry.rs index 10c36001312b2..76e8c29f62a9f 100644 --- a/src/components/validation/runner/telemetry.rs +++ b/src/components/validation/runner/telemetry.rs @@ -23,7 +23,7 @@ const INTERNAL_LOGS_KEY: &str = "_telemetry_logs"; const INTERNAL_METRICS_KEY: &str = "_telemetry_metrics"; const VECTOR_SINK_KEY: &str = "_telemetry_out"; -const SHUTDOWN_TICKS: u8 = 5; +const SHUTDOWN_TICKS: u8 = 3; /// Telemetry collector for a component under validation. pub struct Telemetry { From 35efd5a21e3f78f3c17a698d0467fab0f7f732ad Mon Sep 17 00:00:00 2001 From: neuronull Date: Wed, 19 Jul 2023 13:50:55 -0600 Subject: [PATCH 16/32] fix incorrect expected values --- src/components/validation/resources/http.rs | 24 +++------ src/components/validation/runner/mod.rs | 54 +++++++++++++++---- .../validators/component_spec/sinks.rs | 27 +++++----- 3 files changed, 62 insertions(+), 43 deletions(-) diff --git a/src/components/validation/resources/http.rs b/src/components/validation/resources/http.rs index c28159f628154..802056209c19f 100644 --- a/src/components/validation/resources/http.rs +++ b/src/components/validation/resources/http.rs @@ -24,7 +24,7 @@ use crate::components::validation::{ sync::{Configuring, TaskCoordinator}, RunnerMetrics, }; -use vector_core::event::Event; +use vector_core::{event::Event, EstimatedJsonEncodedSizeOf}; use super::{encode_test_event, ResourceCodec, ResourceDirection, TestEvent}; @@ -248,15 +248,6 @@ fn spawn_output_http_server( &config, runner_metrics, move |request, output_runner_metrics| { - if request.headers().contains_key("Content-Length") { - let bytes = request - .headers() - .get("Content-Length") - .unwrap() - .to_str() - .unwrap(); - debug!("HTTP server external output resource received {bytes} bytes."); - } let output_tx = output_tx.clone(); let mut decoder = decoder.clone(); @@ -274,16 +265,15 @@ fn spawn_output_http_server( // Update the runner metrics for the received events. This will later // be used in the Validators, as the "expected" case. output_runner_metrics.received_bytes_total += byte_size as u64; + output_runner_metrics.received_events_total += events.len() as u64; - // TODO: I think we will need to encode the events received and get the value - // of the buffer length, to get the expected value for `component_sent_event_bytes_total` - // Unfortunately that means we'll have to pass the encoder down to here as well. - - // output_runner_metrics.received_event_bytes_total += - // events.to_vec().estimated_json_encoded_size_of().get() - // as u64; + events.iter().for_each(|event| { + output_runner_metrics.received_event_bytes_total += + event.clone().estimated_json_encoded_size_of().get() + as u64; + }); output_tx .send(events.to_vec()) diff --git a/src/components/validation/runner/mod.rs b/src/components/validation/runner/mod.rs index 9b2d08a18714d..5d6e86f8ac8c5 100644 --- a/src/components/validation/runner/mod.rs +++ b/src/components/validation/runner/mod.rs @@ -5,6 +5,7 @@ mod telemetry; use std::{collections::HashMap, path::PathBuf, sync::Arc, time::Duration}; use bytes::BytesMut; +use chrono::Utc; use tokio::{ runtime::Builder, select, @@ -17,7 +18,7 @@ use tokio::{ use tokio_util::codec::Encoder as _; use codecs::encoding; -use vector_core::{event::Event, EstimatedJsonEncodedSizeOf}; +use vector_core::{config::LogNamespace, event::Event, EstimatedJsonEncodedSizeOf}; use crate::{ codecs::Encoder, @@ -296,12 +297,16 @@ impl Runner { let skip_input_driver_metrics = self.configuration.component_type == ComponentType::Source; + let source_is_controlled_edge = + self.configuration.component_type != ComponentType::Source; + let input_driver = spawn_input_driver( test_case.events.clone(), input_tx, &runner_metrics, maybe_runner_encoder.as_ref().cloned(), skip_input_driver_metrics, + source_is_controlled_edge, ); let skip_output_driver_metrics = @@ -531,9 +536,13 @@ fn spawn_input_driver( runner_metrics: &Arc>, mut maybe_encoder: Option>, _skip_input_driver_metrics: bool, + source_is_controlled_edge: bool, ) -> JoinHandle<()> { let input_runner_metrics = Arc::clone(runner_metrics); + let log_namespace = LogNamespace::Legacy; + let now = Utc::now(); + tokio::spawn(async move { for input_event in input_events { input_tx @@ -545,6 +554,34 @@ fn spawn_input_driver( // be used in the Validators, as the "expected" case. let mut input_runner_metrics = input_runner_metrics.lock().await; + let (modified, mut event) = match input_event.clone() { + TestEvent::Passthrough(event) => (false, event), + TestEvent::Modified { modified, event } => (modified, event), + }; + + event + .as_log() + .get_timestamp() + .unwrap() + .as_timestamp_unwrap(); + + // the controlled edge (vector source) adds metadata to the event when it is received. + // thus we need to add it here so the expected values for the comparisons on transforms + // and sinks are accurate. + if source_is_controlled_edge { + if let Event::Log(ref mut log) = event { + log_namespace.insert_standard_vector_source_metadata(log, "vector", now); + } + } + + let input_event = match &input_event { + TestEvent::Passthrough(_) => TestEvent::Passthrough(event.clone()), + TestEvent::Modified { modified, event: _ } => TestEvent::Modified { + modified: *modified, + event: event.clone(), + }, + }; + if let Some(encoder) = maybe_encoder.as_mut() { let mut buffer = BytesMut::new(); encode_test_event(encoder, &mut buffer, input_event.clone()); @@ -552,11 +589,6 @@ fn spawn_input_driver( input_runner_metrics.sent_bytes_total += buffer.len() as u64; } - let (modified, event) = match input_event { - TestEvent::Passthrough(event) => (false, event), - TestEvent::Modified { modified, event } => (modified, event), - }; - // account for failure case if !modified { input_runner_metrics.sent_events_total += 1; @@ -586,12 +618,12 @@ fn spawn_output_driver( let mut output_runner_metrics = output_runner_metrics.lock().await; for output_event in events { - output_runner_metrics.received_event_bytes_total += vec![output_event.clone()] - .estimated_json_encoded_size_of() - .get() - as u64; - if !skip_output_driver_metrics { + output_runner_metrics.received_event_bytes_total += vec![output_event.clone()] + .estimated_json_encoded_size_of() + .get() + as u64; + if let Some(encoder) = maybe_encoder.as_ref() { let mut buffer = BytesMut::new(); encoder diff --git a/src/components/validation/validators/component_spec/sinks.rs b/src/components/validation/validators/component_spec/sinks.rs index d65840c014ae2..50eea39c4ccef 100644 --- a/src/components/validation/validators/component_spec/sinks.rs +++ b/src/components/validation/validators/component_spec/sinks.rs @@ -14,9 +14,8 @@ impl ComponentMetricValidator for SinkComponentMetricValidator { ) -> Result, Vec> { match metric_type { ComponentMetricType::EventsReceived => { - // Since the runner is on the same "side" of the topology as a sink is, - // the expected value is what the input runner received. - // let expected_events = runner_metrics.received_events_total; + // The reciprocal metric for events received is events sent, + // so the expected value is what the input runner sent. let expected_events = runner_metrics.sent_events_total; Self::validate_events_total( @@ -27,9 +26,9 @@ impl ComponentMetricValidator for SinkComponentMetricValidator { ) } ComponentMetricType::EventsReceivedBytes => { - // Since the runner is on the same "side" of the topology as a sink is, - // the expected value is what the input runner received. - let expected_bytes = runner_metrics.received_event_bytes_total; + // The reciprocal metric for received_event_bytes is sent_event_bytes, + // so the expected value is what the input runner sent. + let expected_bytes = runner_metrics.sent_event_bytes_total; Self::validate_bytes_total( telemetry_events, @@ -47,8 +46,8 @@ impl ComponentMetricValidator for SinkComponentMetricValidator { ) } ComponentMetricType::SentEventsTotal => { - // Since the runner is on the same "side" of the topology as a sink is, - // the expected value is what the input runner sent. + // The reciprocal metric for events sent is events received, + // so the expected value is what the output runner received. let expected_events = runner_metrics.received_events_total; Self::validate_events_total( @@ -59,8 +58,8 @@ impl ComponentMetricValidator for SinkComponentMetricValidator { ) } ComponentMetricType::SentBytesTotal => { - // Since the runner is on the same "side" of the topology as a sink is, - // the expected value is what the input runner sent. + // The reciprocal metric for sent_bytes is received_bytes, + // so the expected value is what the output runner received. let expected_bytes = runner_metrics.received_bytes_total; Self::validate_bytes_total( @@ -71,11 +70,9 @@ impl ComponentMetricValidator for SinkComponentMetricValidator { ) } ComponentMetricType::SentEventBytesTotal => { - // Since the runner is on the same "side" of the topology as a sink is, - // the expected value is what the input runner sent. - let expected_bytes = 0; - - // TODO: see TODO in resources/http.rs for idea on how to get this value + // The reciprocal metric for sent_event_bytes is received_event_bytes, + // so the expected value is what the output runner received. + let expected_bytes = runner_metrics.received_event_bytes_total; Self::validate_bytes_total( telemetry_events, From 1f4ea0210ec9c6a5f791aba50b873cdd613b9ed2 Mon Sep 17 00:00:00 2001 From: neuronull Date: Wed, 19 Jul 2023 14:24:33 -0600 Subject: [PATCH 17/32] Even more reduction --- .../validators/component_spec/mod.rs | 414 +++++++++++------- .../validators/component_spec/sinks.rs | 90 ---- .../validators/component_spec/sources.rs | 90 ---- 3 files changed, 247 insertions(+), 347 deletions(-) delete mode 100644 src/components/validation/validators/component_spec/sinks.rs delete mode 100644 src/components/validation/validators/component_spec/sources.rs diff --git a/src/components/validation/validators/component_spec/mod.rs b/src/components/validation/validators/component_spec/mod.rs index 34670b1b2910f..ad4e0f400cff3 100644 --- a/src/components/validation/validators/component_spec/mod.rs +++ b/src/components/validation/validators/component_spec/mod.rs @@ -3,12 +3,12 @@ mod sources; use vector_core::event::{Event, Metric, MetricKind}; -use crate::components::validation::{ComponentType, RunnerMetrics, TestCaseExpectation, TestEvent}; +use crate::components::validation::{ + component_names::*, ComponentType, RunnerMetrics, TestCaseExpectation, TestEvent, +}; use super::{ComponentMetricType, Validator}; -use self::{sinks::SinkComponentMetricValidator, sources::SourceComponentMetricValidator}; - /// Validates that the component meets the requirements of the [Component Specification][component_spec]. /// /// Generally speaking, the Component Specification dictates the expected events and metrics @@ -98,29 +98,27 @@ fn validate_telemetry( let mut out: Vec = Vec::new(); let mut errs: Vec = Vec::new(); - match component_type { - ComponentType::Source => { - let result = validate_component::( - telemetry_events, - runner_metrics, - ); - match result { - Ok(o) => out.extend(o), - Err(e) => errs.extend(e), - } - } - ComponentType::Sink => { - let result = validate_component::( - telemetry_events, - runner_metrics, - ); - match result { - Ok(o) => out.extend(o), - Err(e) => errs.extend(e), - } + let metric_types = [ + ComponentMetricType::EventsReceived, + ComponentMetricType::EventsReceivedBytes, + ComponentMetricType::ReceivedBytesTotal, + ComponentMetricType::SentEventsTotal, + ComponentMetricType::SentEventBytesTotal, + ComponentMetricType::SentBytesTotal, + ComponentMetricType::EventsDropped, + ]; + + metric_types.iter().for_each(|metric_type| { + match validate_metric( + telemetry_events, + runner_metrics, + metric_type, + component_type, + ) { + Err(e) => errs.extend(e), + Ok(m) => out.extend(m), } - ComponentType::Transform => {} - } + }); if errs.is_empty() { Ok(out) @@ -129,171 +127,253 @@ fn validate_telemetry( } } -pub trait ComponentMetricValidator { - fn validate_metric( - telemetry_events: &[Event], - runner_metrics: &RunnerMetrics, - metric_type: &ComponentMetricType, - ) -> Result, Vec> - where - Self: Sized; - fn filter_events_by_metric_and_component<'a>( - telemetry_events: &'a [Event], - metric: &ComponentMetricType, - component_name: &'a str, - ) -> Vec<&'a Metric> { - let metrics: Vec<&Metric> = telemetry_events - .iter() - .flat_map(|e| { - if let vector_core::event::Event::Metric(m) = e { - Some(m) - } else { - None - } - }) - .filter(|&m| { - if m.name() == metric.to_string() { - if let Some(tags) = m.tags() { - if tags.get("component_name").unwrap_or("") == component_name { - return true; - } - } - } - - false - }) - .collect(); - - debug!("{}: {} metrics found.", metric.to_string(), metrics.len(),); - - metrics - } - - fn sum_counters( - metric_name: &ComponentMetricType, - metrics: &[&vector_core::event::Metric], - ) -> Result> { - let mut sum: f64 = 0.0; - let mut errs = Vec::new(); - - for m in metrics { - match m.value() { - vector_core::event::MetricValue::Counter { value } => { - if let MetricKind::Absolute = m.data().kind { - sum = *value; - } else { - sum += *value; - } - } - _ => errs.push(format!("{}: metric value is not a counter", metric_name,)), - } +fn validate_metric( + telemetry_events: &[Event], + runner_metrics: &RunnerMetrics, + metric_type: &ComponentMetricType, + component_type: ComponentType, +) -> Result, Vec> { + let component_name = match component_type { + ComponentType::Source => TEST_SOURCE_NAME, + ComponentType::Transform => TEST_TRANSFORM_NAME, + ComponentType::Sink => TEST_SINK_NAME, + }; + + match metric_type { + ComponentMetricType::EventsReceived => { + // The reciprocal metric for events received is events sent, + // so the expected value is what the input runner sent. + let expected_events = runner_metrics.sent_events_total; + + compare_actual_to_expected( + telemetry_events, + &ComponentMetricType::EventsReceived, + component_name, + expected_events, + ) } + ComponentMetricType::EventsReceivedBytes => { + // The reciprocal metric for received_event_bytes is sent_event_bytes, + // so the expected value is what the input runner sent. + let expected_bytes = runner_metrics.sent_event_bytes_total; - if errs.is_empty() { - Ok(sum as u64) - } else { - Err(errs) + compare_actual_to_expected( + telemetry_events, + &ComponentMetricType::EventsReceivedBytes, + component_name, + expected_bytes, + ) } - } - - fn validate_events_total( - telemetry_events: &[Event], - metric_type: &ComponentMetricType, - component_name: &str, - expected_events: u64, - ) -> Result, Vec> { - let mut errs: Vec = Vec::new(); - - let metrics = Self::filter_events_by_metric_and_component( - telemetry_events, - metric_type, - component_name, - ); - - let actual_events = Self::sum_counters(metric_type, &metrics)?; - - debug!( - "{}: {} events, {} expected events.", - metric_type, actual_events, expected_events, - ); - - if actual_events != expected_events { - errs.push(format!( - "{}: expected {} events, but received {}", - metric_type, expected_events, actual_events - )); + ComponentMetricType::ReceivedBytesTotal => { + // The reciprocal metric for received_bytes is sent_bytes, + // so the expected value is what the input runner sent. + let expected_bytes = if component_type == ComponentType::Sink { + 0 // sinks should not emit this metric + } else { + runner_metrics.sent_bytes_total + }; + compare_actual_to_expected( + telemetry_events, + &ComponentMetricType::ReceivedBytesTotal, + component_name, + expected_bytes, + ) } + ComponentMetricType::SentEventsTotal => { + // The reciprocal metric for events sent is events received, + // so the expected value is what the output runner received. + let expected_events = runner_metrics.received_events_total; - if !errs.is_empty() { - return Err(errs); + compare_actual_to_expected( + telemetry_events, + &ComponentMetricType::SentEventsTotal, + component_name, + expected_events, + ) } + ComponentMetricType::SentBytesTotal => { + // The reciprocal metric for sent_bytes is received_bytes, + // so the expected value is what the output runner received. + let expected_bytes = if component_type == ComponentType::Source { + 0 // sources should not emit this metric + } else { + runner_metrics.received_bytes_total + }; + + compare_actual_to_expected( + telemetry_events, + &ComponentMetricType::SentBytesTotal, + component_name, + expected_bytes, + ) + } + ComponentMetricType::SentEventBytesTotal => { + // The reciprocal metric for sent_event_bytes is received_event_bytes, + // so the expected value is what the output runner received. + let expected_bytes = runner_metrics.received_event_bytes_total; - Ok(vec![format!("{}: {}", metric_type, actual_events)]) + compare_actual_to_expected( + telemetry_events, + &ComponentMetricType::SentEventBytesTotal, + component_name, + expected_bytes, + ) + } + ComponentMetricType::EventsDropped => { + // TODO + Ok(vec![]) + } } +} - fn validate_bytes_total( - telemetry_events: &[Event], - metric_type: &ComponentMetricType, - component_name: &str, - expected_bytes: u64, - ) -> Result, Vec> { - let mut errs: Vec = Vec::new(); - - let metrics = Self::filter_events_by_metric_and_component( - telemetry_events, - metric_type, - component_name, - ); +fn filter_events_by_metric_and_component<'a>( + telemetry_events: &'a [Event], + metric: &ComponentMetricType, + component_name: &'a str, +) -> Vec<&'a Metric> { + let metrics: Vec<&Metric> = telemetry_events + .iter() + .flat_map(|e| { + if let vector_core::event::Event::Metric(m) = e { + Some(m) + } else { + None + } + }) + .filter(|&m| { + if m.name() == metric.to_string() { + if let Some(tags) = m.tags() { + if tags.get("component_name").unwrap_or("") == component_name { + return true; + } + } + } - let actual_bytes = Self::sum_counters(metric_type, &metrics)?; + false + }) + .collect(); - debug!( - "{}: {} bytes, {} expected bytes.", - metric_type, actual_bytes, expected_bytes, - ); + debug!("{}: {} metrics found.", metric.to_string(), metrics.len(),); - if actual_bytes != expected_bytes { - errs.push(format!( - "{}: expected {} bytes, but received {}", - metric_type, expected_bytes, actual_bytes - )); - } + metrics +} - if !errs.is_empty() { - return Err(errs); +fn sum_counters( + metric_name: &ComponentMetricType, + metrics: &[&vector_core::event::Metric], +) -> Result> { + let mut sum: f64 = 0.0; + let mut errs = Vec::new(); + + for m in metrics { + match m.value() { + vector_core::event::MetricValue::Counter { value } => { + if let MetricKind::Absolute = m.data().kind { + sum = *value; + } else { + sum += *value; + } + } + _ => errs.push(format!("{}: metric value is not a counter", metric_name,)), } + } - Ok(vec![format!("{}: {}", metric_type, actual_bytes)]) + if errs.is_empty() { + Ok(sum as u64) + } else { + Err(errs) } } -pub fn validate_component( +fn compare_actual_to_expected( telemetry_events: &[Event], - runner_metrics: &RunnerMetrics, + metric_type: &ComponentMetricType, + component_name: &str, + expected: u64, ) -> Result, Vec> { - let mut out: Vec = Vec::new(); let mut errs: Vec = Vec::new(); - let metric_types = [ - ComponentMetricType::EventsReceived, - ComponentMetricType::EventsReceivedBytes, - ComponentMetricType::ReceivedBytesTotal, - ComponentMetricType::SentEventsTotal, - ComponentMetricType::SentEventBytesTotal, - ComponentMetricType::SentBytesTotal, - ComponentMetricType::EventsDropped, - ]; + let metrics = + filter_events_by_metric_and_component(telemetry_events, metric_type, component_name); - metric_types.iter().for_each(|metric_type| { - match V::validate_metric(telemetry_events, runner_metrics, metric_type) { - Err(e) => errs.extend(e), - Ok(m) => out.extend(m), - } - }); + let actual = sum_counters(metric_type, &metrics)?; - if errs.is_empty() { - Ok(out) - } else { - Err(errs) + debug!("{}: expected {}, actual {}.", metric_type, expected, actual,); + + if actual != expected { + errs.push(format!( + "{}: expected {}, but received {}", + metric_type, expected, actual + )); + } + + if !errs.is_empty() { + return Err(errs); } + + Ok(vec![format!("{}: {}", metric_type, actual)]) } + +// fn validate_events_total( +// telemetry_events: &[Event], +// metric_type: &ComponentMetricType, +// component_name: &str, +// expected_events: u64, +// ) -> Result, Vec> { +// let mut errs: Vec = Vec::new(); + +// let metrics = +// filter_events_by_metric_and_component(telemetry_events, metric_type, component_name); + +// let actual_events = sum_counters(metric_type, &metrics)?; + +// debug!( +// "{}: {} events, {} expected events.", +// metric_type, actual_events, expected_events, +// ); + +// if actual_events != expected_events { +// errs.push(format!( +// "{}: expected {} events, but received {}", +// metric_type, expected_events, actual_events +// )); +// } + +// if !errs.is_empty() { +// return Err(errs); +// } + +// Ok(vec![format!("{}: {}", metric_type, actual_events)]) +// } + +// fn validate_bytes_total( +// telemetry_events: &[Event], +// metric_type: &ComponentMetricType, +// component_name: &str, +// expected_bytes: u64, +// ) -> Result, Vec> { +// let mut errs: Vec = Vec::new(); + +// let metrics = +// filter_events_by_metric_and_component(telemetry_events, metric_type, component_name); + +// let actual_bytes = sum_counters(metric_type, &metrics)?; + +// debug!( +// "{}: {} bytes, {} expected bytes.", +// metric_type, actual_bytes, expected_bytes, +// ); + +// if actual_bytes != expected_bytes { +// errs.push(format!( +// "{}: expected {} bytes, but received {}", +// metric_type, expected_bytes, actual_bytes +// )); +// } + +// if !errs.is_empty() { +// return Err(errs); +// } + +// Ok(vec![format!("{}: {}", metric_type, actual_bytes)]) +// } diff --git a/src/components/validation/validators/component_spec/sinks.rs b/src/components/validation/validators/component_spec/sinks.rs deleted file mode 100644 index 50eea39c4ccef..0000000000000 --- a/src/components/validation/validators/component_spec/sinks.rs +++ /dev/null @@ -1,90 +0,0 @@ -use vector_core::event::Event; - -use crate::components::validation::{component_names::TEST_SINK_NAME, RunnerMetrics}; - -use super::{ComponentMetricType, ComponentMetricValidator}; - -pub struct SinkComponentMetricValidator; - -impl ComponentMetricValidator for SinkComponentMetricValidator { - fn validate_metric( - telemetry_events: &[Event], - runner_metrics: &RunnerMetrics, - metric_type: &ComponentMetricType, - ) -> Result, Vec> { - match metric_type { - ComponentMetricType::EventsReceived => { - // The reciprocal metric for events received is events sent, - // so the expected value is what the input runner sent. - let expected_events = runner_metrics.sent_events_total; - - Self::validate_events_total( - telemetry_events, - &ComponentMetricType::EventsReceived, - TEST_SINK_NAME, - expected_events, - ) - } - ComponentMetricType::EventsReceivedBytes => { - // The reciprocal metric for received_event_bytes is sent_event_bytes, - // so the expected value is what the input runner sent. - let expected_bytes = runner_metrics.sent_event_bytes_total; - - Self::validate_bytes_total( - telemetry_events, - &ComponentMetricType::EventsReceivedBytes, - TEST_SINK_NAME, - expected_bytes, - ) - } - ComponentMetricType::ReceivedBytesTotal => { - Self::validate_bytes_total( - telemetry_events, - &ComponentMetricType::ReceivedBytesTotal, - TEST_SINK_NAME, - 0, // sinks should not emit this metric - ) - } - ComponentMetricType::SentEventsTotal => { - // The reciprocal metric for events sent is events received, - // so the expected value is what the output runner received. - let expected_events = runner_metrics.received_events_total; - - Self::validate_events_total( - telemetry_events, - &ComponentMetricType::SentEventsTotal, - TEST_SINK_NAME, - expected_events, - ) - } - ComponentMetricType::SentBytesTotal => { - // The reciprocal metric for sent_bytes is received_bytes, - // so the expected value is what the output runner received. - let expected_bytes = runner_metrics.received_bytes_total; - - Self::validate_bytes_total( - telemetry_events, - &ComponentMetricType::SentBytesTotal, - TEST_SINK_NAME, - expected_bytes, - ) - } - ComponentMetricType::SentEventBytesTotal => { - // The reciprocal metric for sent_event_bytes is received_event_bytes, - // so the expected value is what the output runner received. - let expected_bytes = runner_metrics.received_event_bytes_total; - - Self::validate_bytes_total( - telemetry_events, - &ComponentMetricType::SentEventBytesTotal, - TEST_SINK_NAME, - expected_bytes, - ) - } - ComponentMetricType::EventsDropped => { - // TODO - Ok(vec![]) - } - } - } -} diff --git a/src/components/validation/validators/component_spec/sources.rs b/src/components/validation/validators/component_spec/sources.rs deleted file mode 100644 index 851462f82fbf6..0000000000000 --- a/src/components/validation/validators/component_spec/sources.rs +++ /dev/null @@ -1,90 +0,0 @@ -use vector_core::event::Event; - -use crate::components::validation::{component_names::TEST_SOURCE_NAME, RunnerMetrics}; - -use super::{ComponentMetricType, ComponentMetricValidator}; - -pub struct SourceComponentMetricValidator; - -impl ComponentMetricValidator for SourceComponentMetricValidator { - fn validate_metric( - telemetry_events: &[Event], - runner_metrics: &RunnerMetrics, - metric_type: &ComponentMetricType, - ) -> Result, Vec> { - match metric_type { - ComponentMetricType::EventsReceived => { - // The reciprocal metric for events received is events sent, - // so the expected value is what the input runner sent. - let expected_events = runner_metrics.sent_events_total; - - Self::validate_events_total( - telemetry_events, - &ComponentMetricType::EventsReceived, - TEST_SOURCE_NAME, - expected_events, - ) - } - ComponentMetricType::EventsReceivedBytes => { - // The reciprocal metric for received_event_bytes is sent_event_bytes, - // so the expected value is what the input runner sent. - let expected_bytes = runner_metrics.sent_event_bytes_total; - - Self::validate_bytes_total( - telemetry_events, - &ComponentMetricType::EventsReceivedBytes, - TEST_SOURCE_NAME, - expected_bytes, - ) - } - ComponentMetricType::ReceivedBytesTotal => { - // The reciprocal metric for received_bytes is sent_bytes, - // so the expected value is what the input runner sent. - let expected_bytes = runner_metrics.sent_bytes_total; - - Self::validate_bytes_total( - telemetry_events, - &ComponentMetricType::ReceivedBytesTotal, - TEST_SOURCE_NAME, - expected_bytes, - ) - } - ComponentMetricType::SentEventsTotal => { - // The reciprocal metric for events sent is events received, - // so the expected value is what the output runner received. - let expected_events = runner_metrics.received_events_total; - - Self::validate_events_total( - telemetry_events, - &ComponentMetricType::SentEventsTotal, - TEST_SOURCE_NAME, - expected_events, - ) - } - ComponentMetricType::SentBytesTotal => { - Self::validate_bytes_total( - telemetry_events, - &ComponentMetricType::SentBytesTotal, - TEST_SOURCE_NAME, - 0, // sources should not emit this metric - ) - } - ComponentMetricType::SentEventBytesTotal => { - // The reciprocal metric for sent_event_bytes is received_event_bytes, - // so the expected value is what the output runner received. - let expected_bytes = runner_metrics.received_event_bytes_total; - - Self::validate_bytes_total( - telemetry_events, - &ComponentMetricType::SentEventBytesTotal, - TEST_SOURCE_NAME, - expected_bytes, - ) - } - ComponentMetricType::EventsDropped => { - // TODO - Ok(vec![]) - } - } - } -} From e8b17af8c801c8c6df48aaa691f9936251be04e7 Mon Sep 17 00:00:00 2001 From: neuronull Date: Wed, 19 Jul 2023 14:38:55 -0600 Subject: [PATCH 18/32] clippy --- .../validators/component_spec/mod.rs | 67 ------------------- 1 file changed, 67 deletions(-) diff --git a/src/components/validation/validators/component_spec/mod.rs b/src/components/validation/validators/component_spec/mod.rs index ad4e0f400cff3..7b40c1a45ab85 100644 --- a/src/components/validation/validators/component_spec/mod.rs +++ b/src/components/validation/validators/component_spec/mod.rs @@ -1,6 +1,3 @@ -mod sinks; -mod sources; - use vector_core::event::{Event, Metric, MetricKind}; use crate::components::validation::{ @@ -313,67 +310,3 @@ fn compare_actual_to_expected( Ok(vec![format!("{}: {}", metric_type, actual)]) } - -// fn validate_events_total( -// telemetry_events: &[Event], -// metric_type: &ComponentMetricType, -// component_name: &str, -// expected_events: u64, -// ) -> Result, Vec> { -// let mut errs: Vec = Vec::new(); - -// let metrics = -// filter_events_by_metric_and_component(telemetry_events, metric_type, component_name); - -// let actual_events = sum_counters(metric_type, &metrics)?; - -// debug!( -// "{}: {} events, {} expected events.", -// metric_type, actual_events, expected_events, -// ); - -// if actual_events != expected_events { -// errs.push(format!( -// "{}: expected {} events, but received {}", -// metric_type, expected_events, actual_events -// )); -// } - -// if !errs.is_empty() { -// return Err(errs); -// } - -// Ok(vec![format!("{}: {}", metric_type, actual_events)]) -// } - -// fn validate_bytes_total( -// telemetry_events: &[Event], -// metric_type: &ComponentMetricType, -// component_name: &str, -// expected_bytes: u64, -// ) -> Result, Vec> { -// let mut errs: Vec = Vec::new(); - -// let metrics = -// filter_events_by_metric_and_component(telemetry_events, metric_type, component_name); - -// let actual_bytes = sum_counters(metric_type, &metrics)?; - -// debug!( -// "{}: {} bytes, {} expected bytes.", -// metric_type, actual_bytes, expected_bytes, -// ); - -// if actual_bytes != expected_bytes { -// errs.push(format!( -// "{}: expected {} bytes, but received {}", -// metric_type, expected_bytes, actual_bytes -// )); -// } - -// if !errs.is_empty() { -// return Err(errs); -// } - -// Ok(vec![format!("{}: {}", metric_type, actual_bytes)]) -// } From cdeab8f3448b819ceac78864b6c1414970e3ec82 Mon Sep 17 00:00:00 2001 From: neuronull Date: Wed, 19 Jul 2023 15:16:51 -0600 Subject: [PATCH 19/32] add the discarded events total check --- src/components/validation/mod.rs | 1 + .../validators/component_spec/mod.rs | 26 ++++++++++++------- src/components/validation/validators/mod.rs | 4 +-- 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/components/validation/mod.rs b/src/components/validation/mod.rs index 8bf5e4130eaa5..50c5e493d5f53 100644 --- a/src/components/validation/mod.rs +++ b/src/components/validation/mod.rs @@ -189,6 +189,7 @@ pub struct RunnerMetrics { pub sent_bytes_total: u64, pub sent_event_bytes_total: u64, pub sent_events_total: u64, + pub discarded_events_total: u64, } #[cfg(all(test, feature = "component-validation-tests"))] diff --git a/src/components/validation/validators/component_spec/mod.rs b/src/components/validation/validators/component_spec/mod.rs index 7b40c1a45ab85..8d3eaba1bbdd1 100644 --- a/src/components/validation/validators/component_spec/mod.rs +++ b/src/components/validation/validators/component_spec/mod.rs @@ -102,7 +102,7 @@ fn validate_telemetry( ComponentMetricType::SentEventsTotal, ComponentMetricType::SentEventBytesTotal, ComponentMetricType::SentBytesTotal, - ComponentMetricType::EventsDropped, + ComponentMetricType::DiscardedEventsTotal, ]; metric_types.iter().for_each(|metric_type| { @@ -144,7 +144,7 @@ fn validate_metric( compare_actual_to_expected( telemetry_events, - &ComponentMetricType::EventsReceived, + metric_type, component_name, expected_events, ) @@ -156,7 +156,7 @@ fn validate_metric( compare_actual_to_expected( telemetry_events, - &ComponentMetricType::EventsReceivedBytes, + metric_type, component_name, expected_bytes, ) @@ -171,7 +171,7 @@ fn validate_metric( }; compare_actual_to_expected( telemetry_events, - &ComponentMetricType::ReceivedBytesTotal, + metric_type, component_name, expected_bytes, ) @@ -183,7 +183,7 @@ fn validate_metric( compare_actual_to_expected( telemetry_events, - &ComponentMetricType::SentEventsTotal, + metric_type, component_name, expected_events, ) @@ -199,7 +199,7 @@ fn validate_metric( compare_actual_to_expected( telemetry_events, - &ComponentMetricType::SentBytesTotal, + metric_type, component_name, expected_bytes, ) @@ -211,14 +211,20 @@ fn validate_metric( compare_actual_to_expected( telemetry_events, - &ComponentMetricType::SentEventBytesTotal, + metric_type, component_name, expected_bytes, ) } - ComponentMetricType::EventsDropped => { - // TODO - Ok(vec![]) + ComponentMetricType::DiscardedEventsTotal => { + let expected_events = runner_metrics.discarded_events_total; + + compare_actual_to_expected( + telemetry_events, + metric_type, + component_name, + expected_events, + ) } } } diff --git a/src/components/validation/validators/mod.rs b/src/components/validation/validators/mod.rs index 4e82a29aaa691..7bc79284c6c64 100644 --- a/src/components/validation/validators/mod.rs +++ b/src/components/validation/validators/mod.rs @@ -59,7 +59,7 @@ pub enum ComponentMetricType { SentEventsTotal, SentBytesTotal, SentEventBytesTotal, - EventsDropped, + DiscardedEventsTotal, } impl ComponentMetricType { @@ -71,7 +71,7 @@ impl ComponentMetricType { ComponentMetricType::SentEventsTotal => "component_sent_events_total", ComponentMetricType::SentBytesTotal => "component_sent_bytes_total", ComponentMetricType::SentEventBytesTotal => "component_sent_event_bytes_total", - ComponentMetricType::EventsDropped => "component_discarded_events_total", + ComponentMetricType::DiscardedEventsTotal => "component_discarded_events_total", } } } From 006db51a16be8248f943a23b38ca71aa0348d076 Mon Sep 17 00:00:00 2001 From: neuronull Date: Thu, 20 Jul 2023 16:36:31 -0600 Subject: [PATCH 20/32] workaround the new sync issues --- src/components/validation/resources/http.rs | 14 ++++++++++++-- src/components/validation/runner/mod.rs | 10 ++++++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/components/validation/resources/http.rs b/src/components/validation/resources/http.rs index 5f0f9ca43de1c..553603577cc4c 100644 --- a/src/components/validation/resources/http.rs +++ b/src/components/validation/resources/http.rs @@ -129,6 +129,7 @@ fn spawn_input_http_server( // events and working with the HTTP server as they're consumed. let resource_started = task_coordinator.track_started(); let resource_completed = task_coordinator.track_completed(); + tokio::spawn(async move { resource_started.mark_as_done(); debug!("HTTP server external input resource started."); @@ -147,7 +148,10 @@ fn spawn_input_http_server( let mut outstanding_events = outstanding_events.lock().await; outstanding_events.push_back(event); }, - None => input_finished = true, + None => { + trace!("HTTP server external input resource input is finished."); + input_finished = true; + }, }, _ = resource_notifier.notified() => { @@ -166,10 +170,15 @@ fn spawn_input_http_server( }, } } - // Mark ourselves as completed now that we've sent all inputs to the source, and // additionally signal the HTTP server to also gracefully shutdown. _ = http_server_shutdown_tx.send(()); + + // TODO - currently we are getting lucky in the testing of `http_client` source... if the source tries to query + // this server but we have already shut down the thread, then it will generate an error which can throw off our error + // validation. + // I think the solution involves adding synchronization to wait here for the runner to tell us to shutdown. + resource_completed.mark_as_done(); debug!("HTTP server external input resource completed."); @@ -297,6 +306,7 @@ fn spawn_output_http_server( let resource_started = task_coordinator.track_started(); let resource_completed = task_coordinator.track_completed(); let mut resource_shutdown_rx = task_coordinator.register_for_shutdown(); + tokio::spawn(async move { resource_started.mark_as_done(); debug!("HTTP server external output resource started."); diff --git a/src/components/validation/runner/mod.rs b/src/components/validation/runner/mod.rs index 83d95835a4114..957bb166bbcce 100644 --- a/src/components/validation/runner/mod.rs +++ b/src/components/validation/runner/mod.rs @@ -336,7 +336,12 @@ impl Runner { debug!("Input task(s) have been shutdown."); // Without this, not all internal metric events are received for sink components under test. - tokio::time::sleep(Duration::from_secs(1)).await; + // TODO: This is awful and needs a proper solution. + // I think we are going to need to setup distinct task sync logic potentially for each + // combination of Source/Sink + Resource direction Push/Pull + if self.configuration.component_type == ComponentType::Sink { + tokio::time::sleep(Duration::from_secs(1)).await; + } telemetry_task_coordinator.shutdown().await; debug!("Telemetry task(s) have been shutdown."); @@ -349,7 +354,7 @@ impl Runner { let output_events = output_driver .await - .expect("input driver task should not have panicked"); + .expect("output driver task should not have panicked"); debug!("Collected runner metrics: {:?}", runner_metrics); @@ -599,6 +604,7 @@ fn spawn_input_driver( vec![event].estimated_json_encoded_size_of().get() as u64; } } + trace!("Input driver sent all events."); }) } From 604abea1f8a7905f697d00a6393c5446a3c9e3c8 Mon Sep 17 00:00:00 2001 From: neuronull Date: Fri, 21 Jul 2023 15:28:43 -0600 Subject: [PATCH 21/32] multi config support --- src/components/validation/mod.rs | 104 ++++++++++++++++--- src/components/validation/resources/event.rs | 31 +++++- src/components/validation/runner/config.rs | 16 ++- src/components/validation/runner/mod.rs | 62 ++++++----- src/components/validation/test_case.rs | 1 + src/sinks/http.rs | 67 ++++++++++-- src/sources/http_client/client.rs | 5 +- src/sources/http_server.rs | 5 +- tests/validation/components/sinks/http.yaml | 8 ++ 9 files changed, 244 insertions(+), 55 deletions(-) diff --git a/src/components/validation/mod.rs b/src/components/validation/mod.rs index cdbe0b28a8ff6..d84a609503dfc 100644 --- a/src/components/validation/mod.rs +++ b/src/components/validation/mod.rs @@ -57,6 +57,14 @@ pub enum ComponentConfiguration { Sink(BoxedSink), } +#[derive(Clone)] +pub struct ComponentTestCaseConfig { + config: ComponentConfiguration, + test_case: Option, + external_resource: Option, + input_codec: Option, +} + /// Configuration for validating a component. /// /// This type encompasses all of the required information for configuring and validating a @@ -66,46 +74,82 @@ pub enum ComponentConfiguration { pub struct ValidationConfiguration { component_name: &'static str, component_type: ComponentType, - component_configuration: ComponentConfiguration, - external_resource: Option, + component_configurations: Vec, } impl ValidationConfiguration { /// Creates a new `ValidationConfiguration` for a source. pub fn from_source>( component_name: &'static str, - config: C, - external_resource: Option, + component_configurations: Vec<( + C, + Option, + Option, + Option, + )>, ) -> Self { Self { component_name, component_type: ComponentType::Source, - component_configuration: ComponentConfiguration::Source(config.into()), - external_resource, + component_configurations: component_configurations + .into_iter() + .map(|c| ComponentTestCaseConfig { + config: ComponentConfiguration::Source(c.0.into()), + test_case: c.1, + external_resource: c.2, + input_codec: c.3, + }) + .collect(), } } /// Creates a new `ValidationConfiguration` for a transform. - pub fn from_transform(component_name: &'static str, config: impl Into) -> Self { + pub fn from_transform( + component_name: &'static str, + component_configurations: Vec<( + impl Into, + Option, + Option, + Option, + )>, + ) -> Self { Self { component_name, component_type: ComponentType::Transform, - component_configuration: ComponentConfiguration::Transform(config.into()), - external_resource: None, + component_configurations: component_configurations + .into_iter() + .map(|c| ComponentTestCaseConfig { + config: ComponentConfiguration::Transform(c.0.into()), + test_case: c.1, + external_resource: c.2, + input_codec: c.3, + }) + .collect(), } } /// Creates a new `ValidationConfiguration` for a sink. pub fn from_sink>( component_name: &'static str, - config: C, - external_resource: Option, + component_configurations: Vec<( + C, + Option, + Option, + Option, + )>, ) -> Self { Self { component_name, component_type: ComponentType::Sink, - component_configuration: ComponentConfiguration::Sink(config.into()), - external_resource, + component_configurations: component_configurations + .into_iter() + .map(|c| ComponentTestCaseConfig { + config: ComponentConfiguration::Sink(c.0.into()), + test_case: c.1, + external_resource: c.2, + input_codec: c.3, + }) + .collect(), } } @@ -120,13 +164,39 @@ impl ValidationConfiguration { } /// Gets the configuration of the component. - pub fn component_configuration(&self) -> ComponentConfiguration { - self.component_configuration.clone() + pub fn component_configurations(&self) -> Vec { + self.component_configurations.clone() + } + + fn get_comp_test_case(&self, test_case: Option<&String>) -> Option { + let empty = String::from(""); + let test_case = test_case.unwrap_or(&empty); + self.component_configurations + .clone() + .into_iter() + .find(|c| c.test_case.as_ref().unwrap_or(&String::from("")) == test_case) + } + + /// Gets the configuration of the component. + pub fn component_configuration_for_test_case( + &self, + test_case: Option<&String>, + ) -> Option { + self.get_comp_test_case(test_case).map(|c| c.config) } /// Gets the external resource definition for validating the component, if any. - pub fn external_resource(&self) -> Option { - self.external_resource.clone() + pub fn external_resource(&self, test_case: Option<&String>) -> Option { + self.get_comp_test_case(test_case) + .map(|c| c.external_resource) + .flatten() + } + + /// Gets the input codec for validating the component, if any. + pub fn input_codec(&self, test_case: Option<&String>) -> Option { + self.get_comp_test_case(test_case) + .map(|c| c.input_codec) + .flatten() } } diff --git a/src/components/validation/resources/event.rs b/src/components/validation/resources/event.rs index 1f91acfbe660d..8bdc8d5b37f45 100644 --- a/src/components/validation/resources/event.rs +++ b/src/components/validation/resources/event.rs @@ -1,5 +1,6 @@ use bytes::BytesMut; use serde::Deserialize; +use serde_json::Value; use snafu::Snafu; use tokio_util::codec::Encoder as _; @@ -28,6 +29,14 @@ pub enum RawTestEvent { /// For transforms and sinks, generally, the only way to cause an error is if the event itself /// is malformed in some way, which can be achieved without this test event variant. Modified { modified: bool, event: EventData }, + + /// TODO + WithField { + event: EventData, + name: String, + value: Value, + fail: Option, + }, } #[derive(Clone, Debug, Deserialize)] @@ -59,6 +68,9 @@ pub enum TestEvent { /// The event is used, as-is, without modification. Passthrough(Event), + /// The event is used, as-is, without modification by the framework, but we expect it to fail. + PassthroughFail(Event), + /// The event is potentially modified by the external resource. /// /// The modification made is dependent on the external resource, but this mode is made available @@ -76,6 +88,7 @@ impl TestEvent { pub fn into_event(self) -> Event { match self { Self::Passthrough(event) => event, + Self::PassthroughFail(event) => event, Self::Modified { event, .. } => event, } } @@ -94,6 +107,22 @@ impl From for TestEvent { modified, event: event.into_event(), }, + RawTestEvent::WithField { + event, + name, + value, + fail, + } => { + let mut event = event.into_event(); + let log_event = event.as_mut_log(); + log_event.insert(name.as_str(), value); + + if fail.unwrap_or_default() { + TestEvent::PassthroughFail(event) + } else { + TestEvent::Passthrough(event) + } + } } } } @@ -104,7 +133,7 @@ pub fn encode_test_event( event: TestEvent, ) { match event { - TestEvent::Passthrough(event) => { + TestEvent::Passthrough(event) | TestEvent::PassthroughFail(event) => { // Encode the event normally. encoder .encode(event, buf) diff --git a/src/components/validation/runner/config.rs b/src/components/validation/runner/config.rs index cab721450db3d..1f8fd9b747796 100644 --- a/src/components/validation/runner/config.rs +++ b/src/components/validation/runner/config.rs @@ -24,9 +24,17 @@ pub struct TopologyBuilder { impl TopologyBuilder { /// Creates a component topology for the given component configuration. - pub fn from_configuration(configuration: &ValidationConfiguration) -> Self { - let component_configuration = configuration.component_configuration(); - match component_configuration { + pub fn from_configuration( + configuration: &ValidationConfiguration, + config_name: Option<&String>, + ) -> Result { + let component_configuration = configuration + .component_configuration_for_test_case(config_name) + .ok_or(format!( + "No test case name defined for configuration {:?}.", + config_name + ))?; + Ok(match component_configuration { ComponentConfiguration::Source(source) => { debug_assert_eq!(configuration.component_type(), ComponentType::Source); Self::from_source(source) @@ -39,7 +47,7 @@ impl TopologyBuilder { debug_assert_eq!(configuration.component_type(), ComponentType::Sink); Self::from_sink(sink) } - } + }) } /// Creates a component topology for validating a source. diff --git a/src/components/validation/runner/mod.rs b/src/components/validation/runner/mod.rs index 957bb166bbcce..3b39d9cd1c346 100644 --- a/src/components/validation/runner/mod.rs +++ b/src/components/validation/runner/mod.rs @@ -220,7 +220,10 @@ impl Runner { // We then finalize the topology builder to get our actual `ConfigBuilder`, as well as // any controlled edges (channel sender/receiver to the aforementioned filler // components) and a telemetry client for collecting internal telemetry. - let topology_builder = TopologyBuilder::from_configuration(&self.configuration); + let topology_builder = TopologyBuilder::from_configuration( + &self.configuration, + test_case.config_name.as_ref(), + )?; let (config_builder, controlled_edges, telemetry_collector) = topology_builder .finalize( &input_task_coordinator, @@ -247,6 +250,7 @@ impl Runner { // controlled output edge, which means we then need a server task listening for the // events sent by that sink. let (runner_input, runner_output, maybe_runner_encoder) = build_external_resource( + test_case.config_name.as_ref(), &self.configuration, &input_task_coordinator, &output_task_coordinator, @@ -294,29 +298,19 @@ impl Runner { // around if we can avoid it. tokio::time::sleep(Duration::from_secs(2)).await; - let skip_input_driver_metrics = - self.configuration.component_type == ComponentType::Source; - - let source_is_controlled_edge = - self.configuration.component_type != ComponentType::Source; - let input_driver = spawn_input_driver( test_case.events.clone(), input_tx, &runner_metrics, maybe_runner_encoder.as_ref().cloned(), - skip_input_driver_metrics, - source_is_controlled_edge, + self.configuration.component_type, ); - let skip_output_driver_metrics = - self.configuration.component_type == ComponentType::Sink; - let output_driver = spawn_output_driver( output_rx, &runner_metrics, maybe_runner_encoder.as_ref().cloned(), - skip_output_driver_metrics, + self.configuration.component_type, ); // At this point, the component topology is running, and all input/output/telemetry @@ -364,6 +358,7 @@ impl Runner { name: test_name, expectation, events: input_events, + .. } = test_case; let telemetry_events = telemetry_collector.collect().await; @@ -434,16 +429,26 @@ fn load_component_test_cases(test_case_data_path: PathBuf) -> Result, configuration: &ValidationConfiguration, input_task_coordinator: &TaskCoordinator, output_task_coordinator: &TaskCoordinator, runner_metrics: &Arc>, ) -> (RunnerInput, RunnerOutput, Option>) { let component_type = configuration.component_type(); - let maybe_external_resource = configuration.external_resource(); - let maybe_encoder = maybe_external_resource - .as_ref() - .map(|resource| resource.codec.into_encoder()); + let maybe_external_resource = configuration.external_resource(test_case); + + // use the provided input codec if specified, this generally occurs if we are testing error paths that + // necessitate being able to encode the input event for the input driver without error, but error on + // the encoding in the component under test. + let resource_codec = configuration.input_codec(test_case).or_else(|| { + maybe_external_resource + .as_ref() + .map(|resource| resource.codec.clone()) + }); + + let maybe_encoder = resource_codec.as_ref().map(|codec| codec.into_encoder()); + match component_type { ComponentType::Source => { // As an external resource for a source, we create a channel that the validation runner @@ -540,8 +545,7 @@ fn spawn_input_driver( input_tx: Sender, runner_metrics: &Arc>, mut maybe_encoder: Option>, - _skip_input_driver_metrics: bool, - source_is_controlled_edge: bool, + component_type: ComponentType, ) -> JoinHandle<()> { let input_runner_metrics = Arc::clone(runner_metrics); @@ -559,8 +563,9 @@ fn spawn_input_driver( // be used in the Validators, as the "expected" case. let mut input_runner_metrics = input_runner_metrics.lock().await; - let (modified, mut event) = match input_event.clone() { + let (failure_case, mut event) = match input_event.clone() { TestEvent::Passthrough(event) => (false, event), + TestEvent::PassthroughFail(event) => (true, event), TestEvent::Modified { modified, event } => (modified, event), }; @@ -573,7 +578,7 @@ fn spawn_input_driver( // the controlled edge (vector source) adds metadata to the event when it is received. // thus we need to add it here so the expected values for the comparisons on transforms // and sinks are accurate. - if source_is_controlled_edge { + if component_type != ComponentType::Source { if let Event::Log(ref mut log) = event { log_namespace.insert_standard_vector_source_metadata(log, "vector", now); } @@ -581,6 +586,7 @@ fn spawn_input_driver( let input_event = match &input_event { TestEvent::Passthrough(_) => TestEvent::Passthrough(event.clone()), + TestEvent::PassthroughFail(_) => TestEvent::PassthroughFail(event.clone()), TestEvent::Modified { modified, event: _ } => TestEvent::Modified { modified: *modified, event: event.clone(), @@ -595,9 +601,15 @@ fn spawn_input_driver( } // account for failure case - if modified { + if failure_case { input_runner_metrics.errors_total += 1; - } else { + // TODO: this assumption may need to be made configurable at some point + if component_type == ComponentType::Sink { + input_runner_metrics.discarded_events_total += 1; + } + } + + if !failure_case || component_type == ComponentType::Sink { input_runner_metrics.sent_events_total += 1; input_runner_metrics.sent_event_bytes_total += @@ -612,7 +624,7 @@ fn spawn_output_driver( mut output_rx: Receiver>, runner_metrics: &Arc>, maybe_encoder: Option>, - skip_output_driver_metrics: bool, + component_type: ComponentType, ) -> JoinHandle> { let output_runner_metrics = Arc::clone(runner_metrics); @@ -626,7 +638,7 @@ fn spawn_output_driver( let mut output_runner_metrics = output_runner_metrics.lock().await; for output_event in events { - if !skip_output_driver_metrics { + if component_type != ComponentType::Sink { output_runner_metrics.received_event_bytes_total += vec![output_event.clone()] .estimated_json_encoded_size_of() .get() diff --git a/src/components/validation/test_case.rs b/src/components/validation/test_case.rs index 360c6513161b0..a281f6e5883f0 100644 --- a/src/components/validation/test_case.rs +++ b/src/components/validation/test_case.rs @@ -26,6 +26,7 @@ pub enum TestCaseExpectation { #[derive(Deserialize)] pub struct TestCase { pub name: String, + pub config_name: Option, pub expectation: TestCaseExpectation, pub events: Vec, } diff --git a/src/sinks/http.rs b/src/sinks/http.rs index 2b0b5e7b36f26..b8f82530d7453 100644 --- a/src/sinks/http.rs +++ b/src/sinks/http.rs @@ -1,7 +1,10 @@ use std::io::Write; use bytes::{BufMut, Bytes, BytesMut}; -use codecs::encoding::{CharacterDelimitedEncoder, Framer, Serializer}; +use codecs::{ + encoding::{CharacterDelimitedEncoder, Framer, Serializer}, + GelfSerializerConfig, +}; use futures::{future, FutureExt, SinkExt}; use http::{ header::{HeaderName, HeaderValue, AUTHORIZATION}, @@ -272,7 +275,7 @@ impl ValidatableComponent for HttpSinkConfig { use codecs::{JsonSerializerConfig, MetricTagValues}; use std::str::FromStr; - let config = Self { + let happy_config = Self { uri: UriSerde::from_str("http://127.0.0.1:9000/endpoint") .expect("should never fail to parse"), method: HttpMethod::Post, @@ -292,13 +295,65 @@ impl ValidatableComponent for HttpSinkConfig { payload_suffix: String::new(), }; - let external_resource = ExternalResource::new( + let happy_external_resource = ExternalResource::new( ResourceDirection::Push, - HttpResourceConfig::from_parts(config.uri.uri.clone(), Some(config.method.into())), - config.encoding.clone(), + HttpResourceConfig::from_parts( + happy_config.uri.uri.clone(), + Some(happy_config.method.into()), + ), + happy_config.encoding.clone(), ); - ValidationConfiguration::from_sink(Self::NAME, config, Some(external_resource)) + // this config uses the Gelf serializer, which requires the "level" field to + // be an integer + let sad_config = Self { + uri: UriSerde::from_str("http://127.0.0.1:9000/endpoint") + .expect("should never fail to parse"), + method: HttpMethod::Post, + encoding: EncodingConfigWithFraming::new( + None, + GelfSerializerConfig::new().into(), + Transformer::default(), + ), + auth: None, + headers: None, + compression: Compression::default(), + batch: BatchConfig::default(), + request: RequestConfig::default(), + tls: None, + acknowledgements: AcknowledgementsConfig::default(), + payload_prefix: String::new(), + payload_suffix: String::new(), + }; + + let sad_external_resource = ExternalResource::new( + ResourceDirection::Push, + HttpResourceConfig::from_parts( + sad_config.uri.uri.clone(), + Some(sad_config.method.into()), + ), + sad_config.encoding.clone(), + ); + + // this encoder is used to ingest the input event to the input driver (which we need to succeed) + let sad_encoder = EncodingConfigWithFraming::new( + None, + JsonSerializerConfig::new(MetricTagValues::Full).into(), + Transformer::default(), + ); + + ValidationConfiguration::from_sink( + Self::NAME, + vec![ + (happy_config, None, Some(happy_external_resource), None), + ( + sad_config, + Some("encoding_error".to_owned()), + Some(sad_external_resource), + Some(sad_encoder.into()), + ), + ], + ) } } diff --git a/src/sources/http_client/client.rs b/src/sources/http_client/client.rs index fc2a39e0e5743..bc03aed461de4 100644 --- a/src/sources/http_client/client.rs +++ b/src/sources/http_client/client.rs @@ -245,7 +245,10 @@ impl ValidatableComponent for HttpClientConfig { config.get_decoding_config(None), ); - ValidationConfiguration::from_source(Self::NAME, config, Some(external_resource)) + ValidationConfiguration::from_source( + Self::NAME, + vec![(config, None, Some(external_resource), None)], + ) } } diff --git a/src/sources/http_server.rs b/src/sources/http_server.rs index cadd154fc8795..7fd9037410a18 100644 --- a/src/sources/http_server.rs +++ b/src/sources/http_server.rs @@ -271,7 +271,10 @@ impl ValidatableComponent for SimpleHttpConfig { .expect("should not fail to get decoding config"), ); - ValidationConfiguration::from_source(Self::NAME, config, Some(external_resource)) + ValidationConfiguration::from_source( + Self::NAME, + vec![(config, None, Some(external_resource), None)], + ) } } diff --git a/tests/validation/components/sinks/http.yaml b/tests/validation/components/sinks/http.yaml index 7f7c18db35ad4..3d3525766d74f 100644 --- a/tests/validation/components/sinks/http.yaml +++ b/tests/validation/components/sinks/http.yaml @@ -4,3 +4,11 @@ - simple message 1 - simple message 2 - simple message 3 +- name: sad path + config_name: encoding_error + expectation: failure + events: + - event: simple message with the invalid data type for encoder + name: level + value: "1" + fail: true From 82faf6c720ea37bf5745b9a12705f230962dda95 Mon Sep 17 00:00:00 2001 From: neuronull Date: Fri, 21 Jul 2023 16:09:53 -0600 Subject: [PATCH 22/32] cleanup --- src/components/validation/mod.rs | 110 ++++++++++--------- src/components/validation/resources/event.rs | 6 +- src/sinks/http.rs | 10 +- src/sources/http_client/client.rs | 7 +- src/sources/http_server.rs | 7 +- 5 files changed, 84 insertions(+), 56 deletions(-) diff --git a/src/components/validation/mod.rs b/src/components/validation/mod.rs index d84a609503dfc..28c7e942e9f8b 100644 --- a/src/components/validation/mod.rs +++ b/src/components/validation/mod.rs @@ -57,14 +57,61 @@ pub enum ComponentConfiguration { Sink(BoxedSink), } +/// Component configuration for a test case. #[derive(Clone)] pub struct ComponentTestCaseConfig { config: ComponentConfiguration, + /// If specified, this name must match the `config_name` field of at least one of the test case events. test_case: Option, external_resource: Option, + /// If specified, this codec is used instead of the one that the external resource uses. This is necessary + /// in order to allow testing failure paths where the ingest into the topology needs to succeed, but the + /// encoding of the event by the component under test needs to fail. input_codec: Option, } +impl ComponentTestCaseConfig { + pub fn from_source>( + config: C, + test_case: Option, + external_resource: Option, + input_codec: Option, + ) -> Self { + Self { + config: ComponentConfiguration::Source(config.into()), + test_case, + external_resource, + input_codec, + } + } + pub fn from_transform>( + config: C, + test_case: Option, + external_resource: Option, + input_codec: Option, + ) -> Self { + Self { + config: ComponentConfiguration::Transform(config.into()), + test_case, + external_resource, + input_codec, + } + } + pub fn from_sink>( + config: C, + test_case: Option, + external_resource: Option, + input_codec: Option, + ) -> Self { + Self { + config: ComponentConfiguration::Sink(config.into()), + test_case, + external_resource, + input_codec, + } + } +} + /// Configuration for validating a component. /// /// This type encompasses all of the required information for configuring and validating a @@ -74,82 +121,45 @@ pub struct ComponentTestCaseConfig { pub struct ValidationConfiguration { component_name: &'static str, component_type: ComponentType, + /// There may be only one `ComponentTestCaseConfig` necessary to execute all test cases, but some cases + /// require more advanced configuration in order to hit the code path desired. component_configurations: Vec, } impl ValidationConfiguration { /// Creates a new `ValidationConfiguration` for a source. - pub fn from_source>( + pub fn from_source( component_name: &'static str, - component_configurations: Vec<( - C, - Option, - Option, - Option, - )>, + component_configurations: Vec, ) -> Self { Self { component_name, component_type: ComponentType::Source, - component_configurations: component_configurations - .into_iter() - .map(|c| ComponentTestCaseConfig { - config: ComponentConfiguration::Source(c.0.into()), - test_case: c.1, - external_resource: c.2, - input_codec: c.3, - }) - .collect(), + component_configurations, } } /// Creates a new `ValidationConfiguration` for a transform. pub fn from_transform( component_name: &'static str, - component_configurations: Vec<( - impl Into, - Option, - Option, - Option, - )>, + component_configurations: Vec, ) -> Self { Self { component_name, component_type: ComponentType::Transform, - component_configurations: component_configurations - .into_iter() - .map(|c| ComponentTestCaseConfig { - config: ComponentConfiguration::Transform(c.0.into()), - test_case: c.1, - external_resource: c.2, - input_codec: c.3, - }) - .collect(), + component_configurations, } } /// Creates a new `ValidationConfiguration` for a sink. - pub fn from_sink>( + pub fn from_sink( component_name: &'static str, - component_configurations: Vec<( - C, - Option, - Option, - Option, - )>, + component_configurations: Vec, ) -> Self { Self { component_name, component_type: ComponentType::Sink, - component_configurations: component_configurations - .into_iter() - .map(|c| ComponentTestCaseConfig { - config: ComponentConfiguration::Sink(c.0.into()), - test_case: c.1, - external_resource: c.2, - input_codec: c.3, - }) - .collect(), + component_configurations, } } @@ -188,15 +198,13 @@ impl ValidationConfiguration { /// Gets the external resource definition for validating the component, if any. pub fn external_resource(&self, test_case: Option<&String>) -> Option { self.get_comp_test_case(test_case) - .map(|c| c.external_resource) - .flatten() + .and_then(|c| c.external_resource) } /// Gets the input codec for validating the component, if any. pub fn input_codec(&self, test_case: Option<&String>) -> Option { self.get_comp_test_case(test_case) - .map(|c| c.input_codec) - .flatten() + .and_then(|c| c.input_codec) } } diff --git a/src/components/validation/resources/event.rs b/src/components/validation/resources/event.rs index 8bdc8d5b37f45..e87e135ee41d6 100644 --- a/src/components/validation/resources/event.rs +++ b/src/components/validation/resources/event.rs @@ -30,7 +30,11 @@ pub enum RawTestEvent { /// is malformed in some way, which can be achieved without this test event variant. Modified { modified: bool, event: EventData }, - /// TODO + /// The event is created, and the specified field is added to it. + /// + /// This allows the ability to hit code paths where some codecs require specific fields to be of specific + /// types, thus allowing us to encode into the input runner without error, but encoding in the component + /// under test can be set up to fail. WithField { event: EventData, name: String, diff --git a/src/sinks/http.rs b/src/sinks/http.rs index b8f82530d7453..010fd042be58f 100644 --- a/src/sinks/http.rs +++ b/src/sinks/http.rs @@ -345,8 +345,14 @@ impl ValidatableComponent for HttpSinkConfig { ValidationConfiguration::from_sink( Self::NAME, vec![ - (happy_config, None, Some(happy_external_resource), None), - ( + ComponentTestCaseConfig::from_sink( + happy_config, + None, + Some(happy_external_resource), + None, + ), + // this config only runs with the test case "encoding_error" in the yaml file. + ComponentTestCaseConfig::from_sink( sad_config, Some("encoding_error".to_owned()), Some(sad_external_resource), diff --git a/src/sources/http_client/client.rs b/src/sources/http_client/client.rs index bc03aed461de4..debd4a47abde4 100644 --- a/src/sources/http_client/client.rs +++ b/src/sources/http_client/client.rs @@ -247,7 +247,12 @@ impl ValidatableComponent for HttpClientConfig { ValidationConfiguration::from_source( Self::NAME, - vec![(config, None, Some(external_resource), None)], + vec![ComponentTestCaseConfig::from_source( + config, + None, + Some(external_resource), + None, + )], ) } } diff --git a/src/sources/http_server.rs b/src/sources/http_server.rs index 7fd9037410a18..89d56bab446e4 100644 --- a/src/sources/http_server.rs +++ b/src/sources/http_server.rs @@ -273,7 +273,12 @@ impl ValidatableComponent for SimpleHttpConfig { ValidationConfiguration::from_source( Self::NAME, - vec![(config, None, Some(external_resource), None)], + vec![ComponentTestCaseConfig::from_source( + config, + None, + Some(external_resource), + None, + )], ) } } From 1a43e8b3583e7f2574f76cd226352a9c34bb63e0 Mon Sep 17 00:00:00 2001 From: neuronull Date: Tue, 23 Jan 2024 13:43:55 -0700 Subject: [PATCH 23/32] check events --- src/components/validation/validators/component_spec/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/validation/validators/component_spec/mod.rs b/src/components/validation/validators/component_spec/mod.rs index 3789d3ac73fb1..799f27a394de3 100644 --- a/src/components/validation/validators/component_spec/mod.rs +++ b/src/components/validation/validators/component_spec/mod.rs @@ -188,7 +188,7 @@ fn filter_events_by_metric_and_component<'a>( component_id: &'a str, ) -> Vec<&'a Metric> { info!( - "filter looking for metric {} {}", + "Filter looking for metric {} {}", metric.to_string(), component_id ); From f6aa0199067e24c59614f89bed446d123c9e4ee3 Mon Sep 17 00:00:00 2001 From: neuronull Date: Fri, 2 Feb 2024 16:07:38 -0700 Subject: [PATCH 24/32] partial feedback --- src/components/validation/resources/event.rs | 21 ++++++++ src/components/validation/resources/http.rs | 6 +-- src/components/validation/runner/mod.rs | 52 ++++++-------------- 3 files changed, 38 insertions(+), 41 deletions(-) diff --git a/src/components/validation/resources/event.rs b/src/components/validation/resources/event.rs index 4bce09acd4ce5..1f89edff001c5 100644 --- a/src/components/validation/resources/event.rs +++ b/src/components/validation/resources/event.rs @@ -79,6 +79,27 @@ impl TestEvent { Self::Modified { event, .. } => event, } } + + pub fn get_event(&mut self) -> &mut Event { + match self { + Self::Passthrough(event) => event, + Self::Modified { event, .. } => event, + } + } + + pub fn is_modified(&self) -> bool { + match self { + Self::Passthrough(_) => false, + Self::Modified { modified, .. } => *modified, + } + } + + pub fn get(self) -> (bool, Event) { + match self { + Self::Passthrough(event) => (false, event), + Self::Modified { modified, event } => (modified, event), + } + } } #[derive(Clone, Debug, Eq, PartialEq, Snafu)] diff --git a/src/components/validation/resources/http.rs b/src/components/validation/resources/http.rs index 8fc98de15018e..b2fb003ff688f 100644 --- a/src/components/validation/resources/http.rs +++ b/src/components/validation/resources/http.rs @@ -266,7 +266,6 @@ fn spawn_output_http_server( let mut decoder = decoder.clone(); async move { - let mut output_runner_metrics = output_runner_metrics.lock().await; match hyper::body::to_bytes(request.into_body()).await { Err(_) => StatusCode::INTERNAL_SERVER_ERROR.into_response(), Ok(body) => { @@ -274,6 +273,8 @@ fn spawn_output_http_server( loop { match decoder.decode_eof(&mut body) { Ok(Some((events, byte_size))) => { + let mut output_runner_metrics = + output_runner_metrics.lock().await; debug!("HTTP server external output resource decoded {byte_size} bytes."); // Update the runner metrics for the received events. This will later @@ -285,8 +286,7 @@ fn spawn_output_http_server( events.iter().for_each(|event| { output_runner_metrics.received_event_bytes_total += - event.clone().estimated_json_encoded_size_of().get() - as u64; + event.estimated_json_encoded_size_of().get() as u64; }); output_tx diff --git a/src/components/validation/runner/mod.rs b/src/components/validation/runner/mod.rs index 4fd9b743abfda..5e085cd19728f 100644 --- a/src/components/validation/runner/mod.rs +++ b/src/components/validation/runner/mod.rs @@ -303,29 +303,19 @@ impl Runner { // around if we can avoid it. tokio::time::sleep(Duration::from_secs(2)).await; - let skip_input_driver_metrics = - self.configuration.component_type == ComponentType::Source; - - let source_is_controlled_edge = - self.configuration.component_type != ComponentType::Source; - let input_driver = spawn_input_driver( test_case.events.clone(), input_tx, &runner_metrics, maybe_runner_encoder.as_ref().cloned(), - skip_input_driver_metrics, - source_is_controlled_edge, + self.configuration.component_type == ComponentType::Source, ); - let skip_output_driver_metrics = - self.configuration.component_type == ComponentType::Sink; - let output_driver = spawn_output_driver( output_rx, &runner_metrics, maybe_runner_encoder.as_ref().cloned(), - skip_output_driver_metrics, + self.configuration.component_type == ComponentType::Sink, ); // At this point, the component topology is running, and all input/output/telemetry @@ -545,8 +535,7 @@ fn spawn_input_driver( input_tx: Sender, runner_metrics: &Arc>, mut maybe_encoder: Option>, - _skip_input_driver_metrics: bool, - source_is_controlled_edge: bool, + is_source: bool, ) -> JoinHandle<()> { let input_runner_metrics = Arc::clone(runner_metrics); @@ -554,7 +543,7 @@ fn spawn_input_driver( let now = Utc::now(); tokio::spawn(async move { - for input_event in input_events { + for mut input_event in input_events { input_tx .send(input_event.clone()) .await @@ -564,37 +553,20 @@ fn spawn_input_driver( // be used in the Validators, as the "expected" case. let mut input_runner_metrics = input_runner_metrics.lock().await; - let (modified, mut event) = match input_event.clone() { - TestEvent::Passthrough(event) => (false, event), - TestEvent::Modified { modified, event } => (modified, event), - }; - - event - .as_log() - .get_timestamp() - .unwrap() - .as_timestamp_unwrap(); - // the controlled edge (vector source) adds metadata to the event when it is received. // thus we need to add it here so the expected values for the comparisons on transforms // and sinks are accurate. - if source_is_controlled_edge { - if let Event::Log(ref mut log) = event { + if !is_source { + if let Event::Log(ref mut log) = input_event.get_event() { log_namespace.insert_standard_vector_source_metadata(log, "vector", now); } } - let input_event = match &input_event { - TestEvent::Passthrough(_) => TestEvent::Passthrough(event.clone()), - TestEvent::Modified { modified, event: _ } => TestEvent::Modified { - modified: *modified, - event: event.clone(), - }, - }; + let (modified, event) = input_event.clone().get(); if let Some(encoder) = maybe_encoder.as_mut() { let mut buffer = BytesMut::new(); - encode_test_event(encoder, &mut buffer, input_event.clone()); + encode_test_event(encoder, &mut buffer, input_event); input_runner_metrics.sent_bytes_total += buffer.len() as u64; } @@ -605,6 +577,8 @@ fn spawn_input_driver( } else { input_runner_metrics.sent_events_total += 1; + // The event is wrapped in a Vec to match the actual event storage in + // the real topology input_runner_metrics.sent_event_bytes_total += vec![event].estimated_json_encoded_size_of().get() as u64; } @@ -617,7 +591,7 @@ fn spawn_output_driver( mut output_rx: Receiver>, runner_metrics: &Arc>, maybe_encoder: Option>, - skip_output_driver_metrics: bool, + is_sink: bool, ) -> JoinHandle> { let output_runner_metrics = Arc::clone(runner_metrics); @@ -631,7 +605,9 @@ fn spawn_output_driver( let mut output_runner_metrics = output_runner_metrics.lock().await; for output_event in events { - if !skip_output_driver_metrics { + if !is_sink { + // The event is wrapped in a Vec to match the actual event storage in + // the real topology output_runner_metrics.received_event_bytes_total += vec![output_event.clone()] .estimated_json_encoded_size_of() .get() From a2689fefdb9da3a604ac346bb8fc419f97c85ed7 Mon Sep 17 00:00:00 2001 From: neuronull Date: Fri, 2 Feb 2024 16:23:59 -0700 Subject: [PATCH 25/32] thought i removed that --- src/components/validation/resources/event.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/components/validation/resources/event.rs b/src/components/validation/resources/event.rs index 1f89edff001c5..96870f6b773af 100644 --- a/src/components/validation/resources/event.rs +++ b/src/components/validation/resources/event.rs @@ -87,13 +87,6 @@ impl TestEvent { } } - pub fn is_modified(&self) -> bool { - match self { - Self::Passthrough(_) => false, - Self::Modified { modified, .. } => *modified, - } - } - pub fn get(self) -> (bool, Event) { match self { Self::Passthrough(event) => (false, event), From 3ff66e0c02ae2d106e61a199d23a7f8e34da8c6f Mon Sep 17 00:00:00 2001 From: neuronull Date: Mon, 5 Feb 2024 09:44:46 -0700 Subject: [PATCH 26/32] use ref --- src/components/validation/runner/mod.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/components/validation/runner/mod.rs b/src/components/validation/runner/mod.rs index 5e085cd19728f..5014edc5aaa40 100644 --- a/src/components/validation/runner/mod.rs +++ b/src/components/validation/runner/mod.rs @@ -608,10 +608,8 @@ fn spawn_output_driver( if !is_sink { // The event is wrapped in a Vec to match the actual event storage in // the real topology - output_runner_metrics.received_event_bytes_total += vec![output_event.clone()] - .estimated_json_encoded_size_of() - .get() - as u64; + output_runner_metrics.received_event_bytes_total += + vec![&output_event].estimated_json_encoded_size_of().get() as u64; if let Some(encoder) = maybe_encoder.as_ref() { let mut buffer = BytesMut::new(); From 7d17599312aaedaefa6aa4692baced1aedac8399 Mon Sep 17 00:00:00 2001 From: neuronull Date: Tue, 20 Feb 2024 11:21:25 -0700 Subject: [PATCH 27/32] feedback: dont introduce PassThroughFail variant --- src/components/validation/resources/event.rs | 60 +++++++++++--------- 1 file changed, 33 insertions(+), 27 deletions(-) diff --git a/src/components/validation/resources/event.rs b/src/components/validation/resources/event.rs index 58605102dae20..2777bc0136a95 100644 --- a/src/components/validation/resources/event.rs +++ b/src/components/validation/resources/event.rs @@ -72,9 +72,8 @@ pub enum TestEvent { /// The event is used, as-is, without modification. Passthrough(Event), - /// The event is used, as-is, without modification by the framework, but we expect it to fail. - PassthroughFail(Event), - + // /// The event is used, as-is, without modification by the framework, but we expect it to fail. + // PassthroughFail(Event), /// The event is potentially modified by the external resource. /// /// The modification made is dependent on the external resource, but this mode is made available @@ -92,7 +91,6 @@ impl TestEvent { pub fn into_event(self) -> Event { match self { Self::Passthrough(event) => event, - Self::PassthroughFail(event) => event, Self::Modified { event, .. } => event, } } @@ -100,7 +98,6 @@ impl TestEvent { pub fn get_event(&mut self) -> &mut Event { match self { Self::Passthrough(event) => event, - Self::PassthroughFail(event) => event, Self::Modified { event, .. } => event, } } @@ -108,7 +105,6 @@ impl TestEvent { pub fn get(self) -> (bool, Event) { match self { Self::Passthrough(event) => (false, event), - Self::PassthroughFail(event) => (true, event), Self::Modified { modified, event } => (modified, event), } } @@ -138,7 +134,10 @@ impl From for TestEvent { log_event.insert(name.as_str(), value); if fail.unwrap_or_default() { - TestEvent::PassthroughFail(event) + TestEvent::Modified { + modified: true, + event, + } } else { TestEvent::Passthrough(event) } @@ -153,32 +152,39 @@ pub fn encode_test_event( event: TestEvent, ) { match event { - TestEvent::Passthrough(event) | TestEvent::PassthroughFail(event) => { + TestEvent::Passthrough(event) => { // Encode the event normally. encoder .encode(event, buf) .expect("should not fail to encode input event"); } - TestEvent::Modified { event, .. } => { - // This is a little fragile, but we check what serializer this encoder uses, and based - // on `Serializer::supports_json`, we choose an opposing codec. For example, if the - // encoder supports JSON, we'll use a serializer that doesn't support JSON, and vise - // versa. - let mut alt_encoder = if encoder.serializer().supports_json() { - Encoder::::new( - LengthDelimitedEncoder::new().into(), - LogfmtSerializer::new().into(), - ) + TestEvent::Modified { event, modified } => { + if modified { + // This is a little fragile, but we check what serializer this encoder uses, and based + // on `Serializer::supports_json`, we choose an opposing codec. For example, if the + // encoder supports JSON, we'll use a serializer that doesn't support JSON, and vise + // versa. + let mut alt_encoder = if encoder.serializer().supports_json() { + Encoder::::new( + LengthDelimitedEncoder::new().into(), + LogfmtSerializer::new().into(), + ) + } else { + Encoder::::new( + NewlineDelimitedEncoder::new().into(), + JsonSerializer::new(MetricTagValues::default()).into(), + ) + }; + + alt_encoder + .encode(event, buf) + .expect("should not fail to encode input event"); } else { - Encoder::::new( - NewlineDelimitedEncoder::new().into(), - JsonSerializer::new(MetricTagValues::default()).into(), - ) - }; - - alt_encoder - .encode(event, buf) - .expect("should not fail to encode input event"); + // Encode the event normally. + encoder + .encode(event, buf) + .expect("should not fail to encode input event"); + } } } } From da6267cb357968aba912918d6a9327ff2b286507 Mon Sep 17 00:00:00 2001 From: neuronull Date: Tue, 20 Feb 2024 11:54:40 -0700 Subject: [PATCH 28/32] feedback: adjust enum variant names for clarity --- src/components/validation/resources/event.rs | 93 +++++++++---------- .../components/sources/http_client.yaml | 3 +- .../components/sources/http_server.yaml | 3 +- 3 files changed, 44 insertions(+), 55 deletions(-) diff --git a/src/components/validation/resources/event.rs b/src/components/validation/resources/event.rs index 2777bc0136a95..e9617495fd55f 100644 --- a/src/components/validation/resources/event.rs +++ b/src/components/validation/resources/event.rs @@ -28,7 +28,7 @@ pub enum RawTestEvent { /// /// For transforms and sinks, generally, the only way to cause an error is if the event itself /// is malformed in some way, which can be achieved without this test event variant. - Modified { modified: bool, event: EventData }, + AlternateEncoder { fail_encoding_of: EventData }, /// The event is created, and the specified field is added to it. /// @@ -65,6 +65,9 @@ impl EventData { /// metrics collection is based on the same event. Namely, one issue that can arise from creating the event /// from the event data twice (once for the expected and once for actual), it can result in a timestamp in /// the event which may or may not have the same millisecond precision as it's counterpart. +/// +/// For transforms and sinks, generally, the only way to cause an error is if the event itself +/// is malformed in some way, which can be achieved without this test event variant. #[derive(Clone, Debug, Deserialize)] #[serde(from = "RawTestEvent")] #[serde(untagged)] @@ -72,18 +75,13 @@ pub enum TestEvent { /// The event is used, as-is, without modification. Passthrough(Event), - // /// The event is used, as-is, without modification by the framework, but we expect it to fail. - // PassthroughFail(Event), - /// The event is potentially modified by the external resource. - /// - /// The modification made is dependent on the external resource, but this mode is made available - /// for when a test case wants to exercise the failure path, but cannot cause a failure simply - /// by constructing the event in a certain way i.e. adding an invalid field, or removing a - /// required field, or using an invalid field value, and so on. - /// - /// For transforms and sinks, generally, the only way to cause an error is if the event itself - /// is malformed in some way, which can be achieved without this test event variant. - Modified { modified: bool, event: Event }, + /// The event is expected to fail because an alernative encoder than the one specified in the confiuguration + /// is used to encode the event. + FailWithAlternateEncoder(Event), + + /// The event is expected to fail because during parsing of the test case YAML file, the specified field/value is + /// added to the event which should cause the component to error. + FailWithInjectedField(Event), } impl TestEvent { @@ -91,21 +89,25 @@ impl TestEvent { pub fn into_event(self) -> Event { match self { Self::Passthrough(event) => event, - Self::Modified { event, .. } => event, + Self::FailWithAlternateEncoder(event) => event, + Self::FailWithInjectedField(event) => event, } } pub fn get_event(&mut self) -> &mut Event { match self { Self::Passthrough(event) => event, - Self::Modified { event, .. } => event, + Self::FailWithAlternateEncoder(event) => event, + Self::FailWithInjectedField(event) => event, } } + /// (should_fail, event) pub fn get(self) -> (bool, Event) { match self { Self::Passthrough(event) => (false, event), - Self::Modified { modified, event } => (modified, event), + Self::FailWithAlternateEncoder(event) => (true, event), + Self::FailWithInjectedField(event) => (true, event), } } } @@ -119,10 +121,9 @@ impl From for TestEvent { RawTestEvent::Passthrough(event_data) => { TestEvent::Passthrough(event_data.into_event()) } - RawTestEvent::Modified { modified, event } => TestEvent::Modified { - modified, - event: event.into_event(), - }, + RawTestEvent::AlternateEncoder { + fail_encoding_of: event_data, + } => TestEvent::FailWithAlternateEncoder(event_data.into_event()), RawTestEvent::WithField { event, name, @@ -134,10 +135,7 @@ impl From for TestEvent { log_event.insert(name.as_str(), value); if fail.unwrap_or_default() { - TestEvent::Modified { - modified: true, - event, - } + TestEvent::FailWithInjectedField(event) } else { TestEvent::Passthrough(event) } @@ -152,39 +150,32 @@ pub fn encode_test_event( event: TestEvent, ) { match event { - TestEvent::Passthrough(event) => { + TestEvent::Passthrough(event) | TestEvent::FailWithInjectedField(event) => { // Encode the event normally. encoder .encode(event, buf) .expect("should not fail to encode input event"); } - TestEvent::Modified { event, modified } => { - if modified { - // This is a little fragile, but we check what serializer this encoder uses, and based - // on `Serializer::supports_json`, we choose an opposing codec. For example, if the - // encoder supports JSON, we'll use a serializer that doesn't support JSON, and vise - // versa. - let mut alt_encoder = if encoder.serializer().supports_json() { - Encoder::::new( - LengthDelimitedEncoder::new().into(), - LogfmtSerializer::new().into(), - ) - } else { - Encoder::::new( - NewlineDelimitedEncoder::new().into(), - JsonSerializer::new(MetricTagValues::default()).into(), - ) - }; - - alt_encoder - .encode(event, buf) - .expect("should not fail to encode input event"); + TestEvent::FailWithAlternateEncoder(event) => { + // This is a little fragile, but we check what serializer this encoder uses, and based + // on `Serializer::supports_json`, we choose an opposing codec. For example, if the + // encoder supports JSON, we'll use a serializer that doesn't support JSON, and vise + // versa. + let mut alt_encoder = if encoder.serializer().supports_json() { + Encoder::::new( + LengthDelimitedEncoder::new().into(), + LogfmtSerializer::new().into(), + ) } else { - // Encode the event normally. - encoder - .encode(event, buf) - .expect("should not fail to encode input event"); - } + Encoder::::new( + NewlineDelimitedEncoder::new().into(), + JsonSerializer::new(MetricTagValues::default()).into(), + ) + }; + + alt_encoder + .encode(event, buf) + .expect("should not fail to encode input event"); } } } diff --git a/tests/validation/components/sources/http_client.yaml b/tests/validation/components/sources/http_client.yaml index 08424a9cde089..437a7d680b566 100644 --- a/tests/validation/components/sources/http_client.yaml +++ b/tests/validation/components/sources/http_client.yaml @@ -9,5 +9,4 @@ events: - simple message 1 - simple message 2 - - modified: true - event: simple message with the wrong encoding + - fail_encoding_of: simple message with the wrong encoding diff --git a/tests/validation/components/sources/http_server.yaml b/tests/validation/components/sources/http_server.yaml index 08424a9cde089..437a7d680b566 100644 --- a/tests/validation/components/sources/http_server.yaml +++ b/tests/validation/components/sources/http_server.yaml @@ -9,5 +9,4 @@ events: - simple message 1 - simple message 2 - - modified: true - event: simple message with the wrong encoding + - fail_encoding_of: simple message with the wrong encoding From b5a23888162fdb5def349f88b198546c4dcd0445 Mon Sep 17 00:00:00 2001 From: neuronull Date: Tue, 20 Feb 2024 14:03:47 -0700 Subject: [PATCH 29/32] feedback: no idea what I was thinking with `input_codec` --- src/components/validation/mod.rs | 16 ---- src/components/validation/runner/mod.rs | 11 +-- src/sinks/http/config.rs | 111 +++++++++++------------- src/sources/http_client/client.rs | 1 - src/sources/http_server.rs | 1 - 5 files changed, 53 insertions(+), 87 deletions(-) diff --git a/src/components/validation/mod.rs b/src/components/validation/mod.rs index f947bd7adaece..d195b48ea99b0 100644 --- a/src/components/validation/mod.rs +++ b/src/components/validation/mod.rs @@ -64,10 +64,6 @@ pub struct ComponentTestCaseConfig { /// If specified, this name must match the `config_name` field of at least one of the test case events. test_case: Option, external_resource: Option, - /// If specified, this codec is used instead of the one that the external resource uses. This is necessary - /// in order to allow testing failure paths where the ingest into the topology needs to succeed, but the - /// encoding of the event by the component under test needs to fail. - input_codec: Option, } impl ComponentTestCaseConfig { @@ -75,39 +71,33 @@ impl ComponentTestCaseConfig { config: C, test_case: Option, external_resource: Option, - input_codec: Option, ) -> Self { Self { config: ComponentConfiguration::Source(config.into()), test_case, external_resource, - input_codec, } } pub fn from_transform>( config: C, test_case: Option, external_resource: Option, - input_codec: Option, ) -> Self { Self { config: ComponentConfiguration::Transform(config.into()), test_case, external_resource, - input_codec, } } pub fn from_sink>( config: C, test_case: Option, external_resource: Option, - input_codec: Option, ) -> Self { Self { config: ComponentConfiguration::Sink(config.into()), test_case, external_resource, - input_codec, } } } @@ -200,12 +190,6 @@ impl ValidationConfiguration { self.get_comp_test_case(test_case) .and_then(|c| c.external_resource) } - - /// Gets the input codec for validating the component, if any. - pub fn input_codec(&self, test_case: Option<&String>) -> Option { - self.get_comp_test_case(test_case) - .and_then(|c| c.input_codec) - } } pub trait ValidatableComponent: Send + Sync { diff --git a/src/components/validation/runner/mod.rs b/src/components/validation/runner/mod.rs index 56b22887d5e68..91048a1e4438e 100644 --- a/src/components/validation/runner/mod.rs +++ b/src/components/validation/runner/mod.rs @@ -446,14 +446,9 @@ fn build_external_resource( let component_type = configuration.component_type(); let maybe_external_resource = configuration.external_resource(test_case); - // use the provided input codec if specified, this generally occurs if we are testing error paths that - // necessitate being able to encode the input event for the input driver without error, but error on - // the encoding in the component under test. - let resource_codec = configuration.input_codec(test_case).or_else(|| { - maybe_external_resource - .as_ref() - .map(|resource| resource.codec.clone()) - }); + let resource_codec = maybe_external_resource + .as_ref() + .map(|resource| resource.codec.clone()); let maybe_encoder = resource_codec.as_ref().map(|codec| codec.into_encoder()); diff --git a/src/sinks/http/config.rs b/src/sinks/http/config.rs index 9dc95dddf1622..91f516fea1da4 100644 --- a/src/sinks/http/config.rs +++ b/src/sinks/http/config.rs @@ -312,71 +312,62 @@ impl ValidatableComponent for HttpSinkConfig { use std::str::FromStr; use vector_lib::codecs::{JsonSerializerConfig, MetricTagValues}; - let happy_config = Self { - uri: UriSerde::from_str("http://127.0.0.1:9000/endpoint") - .expect("should never fail to parse"), - method: HttpMethod::Post, - encoding: EncodingConfigWithFraming::new( - None, - JsonSerializerConfig::new(MetricTagValues::Full).into(), - Transformer::default(), - ), - auth: None, - headers: None, - compression: Compression::default(), - batch: BatchConfig::default(), - request: RequestConfig::default(), - tls: None, - acknowledgements: AcknowledgementsConfig::default(), - payload_prefix: String::new(), - payload_suffix: String::new(), - }; - - let happy_external_resource = ExternalResource::new( - ResourceDirection::Push, - HttpResourceConfig::from_parts( - happy_config.uri.uri.clone(), - Some(happy_config.method.into()), - ), - happy_config.encoding.clone(), + let happy_encoder = EncodingConfigWithFraming::new( + None, + JsonSerializerConfig::new(MetricTagValues::Full).into(), + Transformer::default(), ); - // this config uses the Gelf serializer, which requires the "level" field to - // be an integer - let sad_config = Self { - uri: UriSerde::from_str("http://127.0.0.1:9000/endpoint") - .expect("should never fail to parse"), - method: HttpMethod::Post, - encoding: EncodingConfigWithFraming::new( - None, - GelfSerializerConfig::new().into(), - Transformer::default(), - ), - auth: None, - headers: None, - compression: Compression::default(), - batch: BatchConfig::default(), - request: RequestConfig::default(), - tls: None, - acknowledgements: AcknowledgementsConfig::default(), - payload_prefix: String::new(), - payload_suffix: String::new(), - }; + fn get_config(encoding: EncodingConfigWithFraming) -> HttpSinkConfig { + HttpSinkConfig { + uri: UriSerde::from_str("http://127.0.0.1:9000/endpoint") + .expect("should never fail to parse"), + method: HttpMethod::Post, + encoding, + auth: None, + headers: None, + compression: Compression::default(), + batch: BatchConfig::default(), + request: RequestConfig::default(), + tls: None, + acknowledgements: AcknowledgementsConfig::default(), + payload_prefix: String::new(), + payload_suffix: String::new(), + } + } - let sad_external_resource = ExternalResource::new( - ResourceDirection::Push, - HttpResourceConfig::from_parts( - sad_config.uri.uri.clone(), - Some(sad_config.method.into()), - ), - sad_config.encoding.clone(), - ); + fn get_external_resource( + config: &HttpSinkConfig, + encoding: Option, + ) -> ExternalResource { + ExternalResource::new( + ResourceDirection::Push, + HttpResourceConfig::from_parts(config.uri.uri.clone(), Some(config.method.into())), + if let Some(encoding) = encoding { + encoding + } else { + config.encoding.clone() + }, + ) + } + + let happy_config = get_config(happy_encoder.clone()); - // this encoder is used to ingest the input event to the input driver (which we need to succeed) - let sad_encoder = EncodingConfigWithFraming::new( + let happy_external_resource = get_external_resource(&happy_config, None); + + // this config uses the Gelf serializer, which requires the "level" field to + // be an integer + let sad_config = get_config(EncodingConfigWithFraming::new( None, - JsonSerializerConfig::new(MetricTagValues::Full).into(), + GelfSerializerConfig::new().into(), Transformer::default(), + )); + + let sad_external_resource = get_external_resource( + &happy_config, + // the external resource needs to use an encoder that actually works, in order to + // get the event into the topology successfuly + Some(happy_encoder), ); ValidationConfiguration::from_sink( @@ -386,14 +377,12 @@ impl ValidatableComponent for HttpSinkConfig { happy_config, None, Some(happy_external_resource), - None, ), // this config only runs with the test case "encoding_error" in the yaml file. ComponentTestCaseConfig::from_sink( sad_config, Some("encoding_error".to_owned()), Some(sad_external_resource), - Some(sad_encoder.into()), ), ], ) diff --git a/src/sources/http_client/client.rs b/src/sources/http_client/client.rs index eecf00a17b584..3093847904101 100644 --- a/src/sources/http_client/client.rs +++ b/src/sources/http_client/client.rs @@ -259,7 +259,6 @@ impl ValidatableComponent for HttpClientConfig { config, None, Some(external_resource), - None, )], ) } diff --git a/src/sources/http_server.rs b/src/sources/http_server.rs index 733031a286486..255c4231d7bfd 100644 --- a/src/sources/http_server.rs +++ b/src/sources/http_server.rs @@ -298,7 +298,6 @@ impl ValidatableComponent for SimpleHttpConfig { config, None, Some(external_resource), - None, )], ) } From 94fcdbd60eb2cfce0ff8d836519049d625171442 Mon Sep 17 00:00:00 2001 From: neuronull Date: Tue, 20 Feb 2024 14:10:42 -0700 Subject: [PATCH 30/32] spell check --- src/components/validation/resources/event.rs | 2 +- src/sinks/http/config.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/validation/resources/event.rs b/src/components/validation/resources/event.rs index e9617495fd55f..fc5ca91237235 100644 --- a/src/components/validation/resources/event.rs +++ b/src/components/validation/resources/event.rs @@ -75,7 +75,7 @@ pub enum TestEvent { /// The event is used, as-is, without modification. Passthrough(Event), - /// The event is expected to fail because an alernative encoder than the one specified in the confiuguration + /// The event is expected to fail because an alernative encoder than the one specified in the configuration /// is used to encode the event. FailWithAlternateEncoder(Event), diff --git a/src/sinks/http/config.rs b/src/sinks/http/config.rs index 91f516fea1da4..10f567f96d74d 100644 --- a/src/sinks/http/config.rs +++ b/src/sinks/http/config.rs @@ -366,7 +366,7 @@ impl ValidatableComponent for HttpSinkConfig { let sad_external_resource = get_external_resource( &happy_config, // the external resource needs to use an encoder that actually works, in order to - // get the event into the topology successfuly + // get the event into the topology sucessfully Some(happy_encoder), ); From f560911b5b1053201df81d2dcfbf027551ccccf0 Mon Sep 17 00:00:00 2001 From: neuronull Date: Tue, 20 Feb 2024 14:14:29 -0700 Subject: [PATCH 31/32] fr --- src/components/validation/resources/event.rs | 2 +- src/sinks/http/config.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/validation/resources/event.rs b/src/components/validation/resources/event.rs index fc5ca91237235..370c0d7769928 100644 --- a/src/components/validation/resources/event.rs +++ b/src/components/validation/resources/event.rs @@ -75,7 +75,7 @@ pub enum TestEvent { /// The event is used, as-is, without modification. Passthrough(Event), - /// The event is expected to fail because an alernative encoder than the one specified in the configuration + /// The event is expected to fail because an alternative encoder than the one specified in the configuration /// is used to encode the event. FailWithAlternateEncoder(Event), diff --git a/src/sinks/http/config.rs b/src/sinks/http/config.rs index 10f567f96d74d..62ee534ef1e6f 100644 --- a/src/sinks/http/config.rs +++ b/src/sinks/http/config.rs @@ -366,7 +366,7 @@ impl ValidatableComponent for HttpSinkConfig { let sad_external_resource = get_external_resource( &happy_config, // the external resource needs to use an encoder that actually works, in order to - // get the event into the topology sucessfully + // get the event into the topology successfully Some(happy_encoder), ); From 0415759782c43150316f1d1ed1650a96774b9666 Mon Sep 17 00:00:00 2001 From: neuronull Date: Thu, 22 Feb 2024 09:15:07 -0700 Subject: [PATCH 32/32] feedback- update docs --- src/components/validation/resources/event.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/components/validation/resources/event.rs b/src/components/validation/resources/event.rs index 370c0d7769928..343466ee9e51c 100644 --- a/src/components/validation/resources/event.rs +++ b/src/components/validation/resources/event.rs @@ -75,12 +75,15 @@ pub enum TestEvent { /// The event is used, as-is, without modification. Passthrough(Event), - /// The event is expected to fail because an alternative encoder than the one specified in the configuration - /// is used to encode the event. + /// The event is encoded using an encoding that differs from the component's + /// configured encoding, which should cause an error when the event is decoded. FailWithAlternateEncoder(Event), - /// The event is expected to fail because during parsing of the test case YAML file, the specified field/value is - /// added to the event which should cause the component to error. + /// The event has an additional field injected prior to encoding, which should cause + /// an error when the event is decoded. + /// + /// This is useful for testing encodings that have strict schemas and cannot + /// handle arbitrary fields or differing data types for certain fields. FailWithInjectedField(Event), }