Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 128 additions & 0 deletions lib/vector-core/src/event/vrl_target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,38 @@ impl VrlTarget {
)
}

/// Turn the target back into exactly one event, never fanning out.
///
/// [`Self::into_events`] chooses its variant from the *root value type*, so an array root
/// becomes a multi-event fan-out whether or not anything was assigned. That is right for
/// `remap`, but wrong for a read-only caller such as a condition, which must get back the
/// event it was given - and an event can legitimately *arrive* array-rooted (the native
/// protobuf codec accepts one). Callers that assumed `One` and treated anything else as
/// unreachable could be panicked by input shape alone, including by an empty array.
///
/// Here an array root stays a single event with an array root, so the round-trip is
/// shape-preserving for every possible input.
pub fn into_event(self, log_namespace: LogNamespace) -> Event {
match self {
VrlTarget::LogEvent(value, metadata) => match value {
value @ (Value::Object(_) | Value::Array(_)) => {
LogEvent::from_parts(value, metadata).into()
}
v => match log_namespace {
LogNamespace::Vector => LogEvent::from_parts(v, metadata).into(),
LogNamespace::Legacy => create_log_event(v, metadata).into(),
},
},
VrlTarget::Trace(value, metadata) => match value {
value @ (Value::Object(_) | Value::Array(_)) => {
TraceEvent::from(LogEvent::from_parts(value, metadata)).into()
}
v => TraceEvent::from(create_log_event(v, metadata)).into(),
},
VrlTarget::Metric { metric, .. } => Event::Metric(metric),
}
}

/// Turn the target back into events.
///
/// This returns an iterator of events as one event can be turned into multiple by assigning an
Expand Down Expand Up @@ -1347,4 +1379,100 @@ mod test {
// get single value (should be the last one)
assert_eq!(metric.tag_value("foo"), Some("b".into()));
}

// OBE-11558: `into_events` picks its variant from the root value type, so an array-rooted
// event became a multi-event fan-out even though nothing was assigned. Read-only callers
// (conditions) treated anything but `One` as unreachable and panicked on input shape alone.
// A log event can legitimately arrive array-rooted over the native protobuf codec.

fn empty_program_info() -> ProgramInfo {
ProgramInfo {
fallible: false,
abortable: false,
target_queries: vec![],
target_assignments: vec![],
}
}

fn array_rooted_log(values: Vec<Value>) -> LogEvent {
LogEvent::from_parts(Value::Array(values), EventMetadata::default())
}

#[test]
fn into_event_keeps_an_array_root_as_one_event() {
for log_namespace in [LogNamespace::Legacy, LogNamespace::Vector] {
let log = array_rooted_log(vec![1.into(), 2.into()]);
let target = VrlTarget::new(Event::from(log), &empty_program_info(), false);

let event = target.into_event(log_namespace);

assert_eq!(
event.as_log().value(),
&Value::Array(vec![1.into(), 2.into()]),
"the array root must survive the round-trip intact ({log_namespace:?})"
);
}
}

#[test]
fn into_event_keeps_an_empty_array_root_as_one_event() {
// The sharpest case: `into_events` turns this into a fan-out of *zero* events, which is
// why blaming the fan-out on the condition was wrong - it comes purely from the input.
for log_namespace in [LogNamespace::Legacy, LogNamespace::Vector] {
let target = VrlTarget::new(
Event::from(array_rooted_log(vec![])),
&empty_program_info(),
false,
);

let event = target.into_event(log_namespace);

assert_eq!(event.as_log().value(), &Value::Array(vec![]));
}
}

#[test]
fn into_event_agrees_with_into_events_for_ordinary_roots() {
// Anything that was already `One` must be unaffected.
let roots = [
Value::from(btreemap! { "a" => 1 }),
Value::from("hi"),
Value::from(42),
];

for root in roots {
for log_namespace in [LogNamespace::Legacy, LogNamespace::Vector] {
let build = || {
VrlTarget::new(
Event::from(LogEvent::from_parts(root.clone(), EventMetadata::default())),
&empty_program_info(),
false,
)
};

let via_into_event = build().into_event(log_namespace);
let via_into_events = match build().into_events(log_namespace) {
TargetEvents::One(event) => event,
_ => panic!("expected One for root {root:?}"),
};

assert_eq!(via_into_event, via_into_events, "root {root:?}");
}
}
}

#[test]
fn into_events_still_fans_out_an_array_root() {
// `remap` relies on this; the fix must not change it.
let target = VrlTarget::new(
Event::from(array_rooted_log(vec![1.into(), 2.into()])),
&empty_program_info(),
false,
);

match target.into_events(LogNamespace::Vector) {
TargetEvents::Logs(iter) => assert_eq!(iter.count(), 2),
_ => panic!("an array root must still fan out through into_events"),
}
}
}
59 changes: 54 additions & 5 deletions src/conditions/equality.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use chrono::{DateTime, Utc};
use ordered_float::NotNan;
use vector_lib::config::LogNamespace;
use vector_lib::configurable::{configurable_component, ConfigurableString};
use vector_lib::event::{TargetEvents, VrlTarget};
use vector_lib::event::VrlTarget;
use vector_lib::{event::Event, lookup::lookup_v2::ConfigTargetPath};
use vrl::compiler::{ProgramInfo, Target};
use vrl::core::Value;
Expand Down Expand Up @@ -219,10 +219,10 @@ impl Equality {
}

pub(crate) fn into_event(target: VrlTarget, log_ns: LogNamespace) -> Event {
match target.into_events(log_ns) {
TargetEvents::One(event) => event,
_ => panic!("Event was modified in a condition. This is an internal compiler error."),
}
// Deliberately not `into_events`: that infers fan-out from the root value type, so an
// array-rooted event - which can arrive over the native protobuf codec, and may even be
// empty - would yield `Logs` and panic here on input shape alone.
target.into_event(log_ns)
}

pub(crate) fn check(&self, e: Event) -> (bool, Event) {
Expand Down Expand Up @@ -619,6 +619,55 @@ mod tests {
assert!(!eq_wrong.check(metric).0);
}

// OBE-11558: an array-rooted log event made `into_events` report a fan-out, which
// `into_event` treated as an internal compiler error and panicked on. A transform panic
// becomes a fatal ShutdownError that exits the whole daemon, so input shape alone could kill
// Vector. Such an event can arrive over the native protobuf codec, and an *empty* array is
// enough. This is the fork-only sibling of the same bug in `conditions::vrl`.
fn array_rooted_event(values: Vec<Value>) -> Event {
Event::Log(LogEvent::from_parts(
Value::Array(values),
crate::event::EventMetadata::default(),
))
}

#[test]
fn check_on_array_rooted_event_does_not_panic() {
let eq = build(vec![(".foo", Constant::Integer(42))]);
let original = array_rooted_event(vec![Value::from(1), Value::from(2)]);

let (matched, returned) = eq.check(original.clone());

// `.foo` cannot resolve against an array root, so the clause simply does not match.
assert!(!matched);
assert_eq!(
original, returned,
"a read-only condition must return the event unchanged"
);
}

#[test]
fn check_on_empty_array_rooted_event_does_not_panic() {
let eq = build(vec![(".foo", Constant::Integer(42))]);
let original = array_rooted_event(vec![]);

let (matched, returned) = eq.check(original.clone());

assert!(!matched);
assert_eq!(original, returned);
}

#[test]
fn check_with_context_on_array_rooted_event_does_not_panic() {
let eq = build(vec![(".foo", Constant::Integer(42))]);
let original = array_rooted_event(vec![]);

let (result, returned) = eq.check_with_context(original.clone());

assert!(result.is_err());
assert_eq!(original, returned);
}

// ---------- Equality::check_with_context ----------

#[test]
Expand Down
67 changes: 62 additions & 5 deletions src/conditions/vrl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use vrl::diagnostic::Formatter;
use vrl::value::Value;

use crate::config::LogNamespace;
use crate::event::TargetEvents;
use crate::{
conditions::{Condition, Conditional, ConditionalConfig},
event::{Event, VrlTarget},
Expand Down Expand Up @@ -103,10 +102,10 @@ impl Vrl {
let timezone = TimeZone::default();

let result = Runtime::default().resolve(&mut target, &self.program, &timezone);
let original_event = match target.into_events(log_namespace) {
TargetEvents::One(event) => event,
_ => panic!("Event was modified in a condition. This is an internal compiler error."),
};
// Deliberately not `into_events`: that infers fan-out from the root value type, so an
// array-rooted event - which can arrive over the native protobuf codec, and may even be
// empty - would yield `Logs` and panic here on input shape alone.
let original_event = target.into_event(log_namespace);
(original_event, result)
}
}
Expand Down Expand Up @@ -273,4 +272,62 @@ mod test {
}
}
}

// OBE-11558: an array-rooted log event made `into_events` report a fan-out, which this
// condition treated as an internal compiler error and panicked on. A transform panic becomes
// a fatal ShutdownError that exits the whole daemon, so input shape alone could kill Vector.
// An event can arrive array-rooted over the native protobuf codec, and an *empty* array is
// enough to trigger it.
mod array_rooted_events {
use super::*;
use vector_lib::event::{EventMetadata, LogEvent};
use vrl::value::Value;

fn array_rooted_event(values: Vec<Value>) -> Event {
Event::from(LogEvent::from_parts(
Value::Array(values),
EventMetadata::default(),
))
}

fn condition() -> Condition {
VrlConfig {
source: "true".to_owned(),
runtime: Default::default(),
}
.build(&Default::default())
.expect("condition should build")
}

#[test]
fn array_rooted_event_does_not_panic() {
let (result, event) = condition().check(array_rooted_event(vec![1.into(), 2.into()]));

assert!(result);
assert_eq!(
event.as_log().value(),
&Value::Array(vec![1.into(), 2.into()]),
"a read-only condition must return the event unchanged"
);
}

#[test]
fn empty_array_rooted_event_does_not_panic() {
let (result, event) = condition().check(array_rooted_event(vec![]));

assert!(result);
assert_eq!(event.as_log().value(), &Value::Array(vec![]));
}

#[test]
fn object_rooted_event_is_unaffected() {
let (result, event) = condition().check(log_event!["foo" => "bar"]);

assert!(result);
assert_eq!(
event.as_log().get("foo").map(|v| v.to_string_lossy()),
Some("bar".into())
);
}
}
}
Loading