Skip to content

Commit 9807185

Browse files
authored
chore(components): replace vacuous, weak, and tautological test assertions (#2132)
1 parent c4a0271 commit 9807185

13 files changed

Lines changed: 452 additions & 502 deletions

File tree

bin/agent-data-plane/src/components/ottl_filter_processor/mod.rs

Lines changed: 0 additions & 111 deletions
Original file line numberDiff line numberDiff line change
@@ -551,115 +551,4 @@ mod tests {
551551
transform.transform_buffer(&mut buffer);
552552
assert_eq!(span_count_in_buffer(&buffer), 0);
553553
}
554-
555-
// --- Performance tests (run manually: cargo test -- --ignored --nocapture perf_throughput) ---
556-
557-
/// Number of buffer clones used in throughput tests; only `transform_buffer` time is measured.
558-
const PERF_NUM_BUFFERS: usize = 1024;
559-
/// Spans per buffer (one trace per buffer) in throughput tests.
560-
const PERF_SPANS_PER_BUFFER: usize = 100;
561-
562-
/// Builds a template buffer with one trace containing `spans`.
563-
fn perf_make_buffer(spans: Vec<Span>) -> EventsBuffer {
564-
let trace = make_trace(spans, None);
565-
let mut buf = EventsBuffer::default();
566-
assert!(buf.try_push(Event::Trace(trace)).is_none());
567-
buf
568-
}
569-
570-
/// Throughput test: OTTL predicate matches every span (all spans dropped).
571-
/// Run with: `cargo test -p agent-data-plane perf_throughput_filter_all --release -- --ignored --nocapture`
572-
#[tokio::test]
573-
#[ignore = "performance test; run with: cargo test -- --ignored --nocapture perf_throughput"]
574-
async fn perf_throughput_filter_all() {
575-
let cfg_json = serde_json::json!({
576-
"ottl_filter_config": { "traces": { "span": ["attributes[\"env\"] == \"drop\""] } }
577-
});
578-
let (config, _) = ConfigurationLoader::for_tests(Some(cfg_json), None, false).await;
579-
let ottl_config = OttlFilterConfiguration::from_configuration(&config).unwrap();
580-
let ctx = test_component_context();
581-
let mut transform = ottl_config.build(ctx).await.unwrap();
582-
583-
let spans: Vec<Span> = (0..PERF_SPANS_PER_BUFFER)
584-
.map(|i| make_span(1, i as u64, HashMap::from([("env".into(), "drop".into())])))
585-
.collect();
586-
let template = perf_make_buffer(spans);
587-
let mut buffers: Vec<EventsBuffer> = (0..PERF_NUM_BUFFERS).map(|_| template.clone()).collect();
588-
let total_spans = PERF_NUM_BUFFERS * PERF_SPANS_PER_BUFFER;
589-
590-
let start = std::time::Instant::now();
591-
for buf in &mut buffers {
592-
transform.transform_buffer(buf);
593-
}
594-
let elapsed = start.elapsed();
595-
let throughput = total_spans as f64 / elapsed.as_secs_f64();
596-
println!(
597-
"perf_throughput_filter_all: {} spans in {:?} -> {:.0} spans/s",
598-
total_spans, elapsed, throughput
599-
);
600-
}
601-
602-
/// Throughput test: OTTL predicate matches every second span (half dropped).
603-
/// Run with: `cargo test -p agent-data-plane perf_throughput_filter_half --release -- --ignored --nocapture`
604-
#[tokio::test]
605-
#[ignore = "performance test; run with: cargo test -- --ignored --nocapture perf_throughput"]
606-
async fn perf_throughput_filter_half() {
607-
let cfg_json = serde_json::json!({
608-
"ottl_filter_config": { "traces": { "span": ["attributes[\"drop\"] == \"yes\""] } }
609-
});
610-
let (config, _) = ConfigurationLoader::for_tests(Some(cfg_json), None, false).await;
611-
let ottl_config = OttlFilterConfiguration::from_configuration(&config).unwrap();
612-
let ctx = test_component_context();
613-
let mut transform = ottl_config.build(ctx).await.unwrap();
614-
615-
let spans: Vec<Span> = (0..PERF_SPANS_PER_BUFFER)
616-
.map(|i| {
617-
let drop_val = if i % 2 == 0 { "yes" } else { "no" };
618-
make_span(1, i as u64, HashMap::from([("drop".into(), drop_val.to_string())]))
619-
})
620-
.collect();
621-
let template = perf_make_buffer(spans);
622-
let mut buffers: Vec<EventsBuffer> = (0..PERF_NUM_BUFFERS).map(|_| template.clone()).collect();
623-
let total_spans = PERF_NUM_BUFFERS * PERF_SPANS_PER_BUFFER;
624-
625-
let start = std::time::Instant::now();
626-
for buf in &mut buffers {
627-
transform.transform_buffer(buf);
628-
}
629-
let elapsed = start.elapsed();
630-
let throughput = total_spans as f64 / elapsed.as_secs_f64();
631-
println!(
632-
"perf_throughput_filter_half: {} spans in {:?} -> {:.0} spans/s",
633-
total_spans, elapsed, throughput
634-
);
635-
}
636-
637-
/// Throughput test: OTTL predicate matches no span (none dropped).
638-
/// Run with: `cargo test -p agent-data-plane perf_throughput_filter_none --release -- --ignored --nocapture`
639-
#[tokio::test]
640-
#[ignore = "performance test; run with: cargo test -- --ignored --nocapture perf_throughput"]
641-
async fn perf_throughput_filter_none() {
642-
let (config, _) = ConfigurationLoader::for_tests(None, None, false).await;
643-
let ottl_config = OttlFilterConfiguration::from_configuration(&config).unwrap();
644-
let ctx = test_component_context();
645-
let mut transform = ottl_config.build(ctx).await.unwrap();
646-
647-
let spans: Vec<Span> = (0..PERF_SPANS_PER_BUFFER)
648-
.map(|i| make_span(1, i as u64, HashMap::from([("env".into(), "keep".into())])))
649-
.collect();
650-
let template = perf_make_buffer(spans);
651-
let mut buffers: Vec<EventsBuffer> = (0..PERF_NUM_BUFFERS).map(|_| template.clone()).collect();
652-
let total_spans = PERF_NUM_BUFFERS * PERF_SPANS_PER_BUFFER;
653-
654-
let start = std::time::Instant::now();
655-
for buf in &mut buffers {
656-
transform.transform_buffer(buf);
657-
}
658-
let elapsed = start.elapsed();
659-
let throughput = total_spans as f64 / elapsed.as_secs_f64();
660-
println!(
661-
"perf_throughput_filter_none: {} spans in {:?} -> {:.0} spans/s",
662-
total_spans, elapsed, throughput
663-
);
664-
}
665554
}

bin/agent-data-plane/src/components/ottl_transform_processor/mod.rs

Lines changed: 0 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -849,109 +849,4 @@ mod tests {
849849
"trace span should still be transformed"
850850
);
851851
}
852-
853-
// ---- Group 9: Performance tests ----
854-
855-
const PERF_NUM_BUFFERS: usize = 1024;
856-
const PERF_SPANS_PER_BUFFER: usize = 100;
857-
858-
fn perf_make_buffer(spans: Vec<Span>) -> EventsBuffer {
859-
let trace = make_trace(spans, None);
860-
let mut buf = EventsBuffer::default();
861-
assert!(buf.try_push(Event::Trace(trace)).is_none());
862-
buf
863-
}
864-
865-
/// Run with: `cargo test -p agent-data-plane perf_throughput_set_all --release -- --ignored --nocapture`
866-
#[tokio::test]
867-
#[ignore = "performance test; run with: cargo test -- --ignored --nocapture perf_throughput"]
868-
async fn perf_throughput_set_all() {
869-
let cfg_json = serde_json::json!({
870-
"ottl_transform_config": {
871-
"trace_statements": ["set(attributes[\"tag\"], \"value\")"]
872-
}
873-
});
874-
let mut transform = build_transform(Some(cfg_json)).await;
875-
876-
let spans: Vec<Span> = (0..PERF_SPANS_PER_BUFFER)
877-
.map(|i| make_span(1, i as u64, HashMap::new()))
878-
.collect();
879-
let template = perf_make_buffer(spans);
880-
let mut buffers: Vec<EventsBuffer> = (0..PERF_NUM_BUFFERS).map(|_| template.clone()).collect();
881-
let total_spans = PERF_NUM_BUFFERS * PERF_SPANS_PER_BUFFER;
882-
883-
let start = std::time::Instant::now();
884-
for buf in &mut buffers {
885-
transform.transform_buffer(buf);
886-
}
887-
let elapsed = start.elapsed();
888-
let throughput = total_spans as f64 / elapsed.as_secs_f64();
889-
println!(
890-
"perf_throughput_set_all: {} spans in {:?} -> {:.0} spans/s",
891-
total_spans, elapsed, throughput
892-
);
893-
}
894-
895-
/// Run with: `cargo test -p agent-data-plane perf_throughput_set_half --release -- --ignored --nocapture`
896-
#[tokio::test]
897-
#[ignore = "performance test; run with: cargo test -- --ignored --nocapture perf_throughput"]
898-
async fn perf_throughput_set_half() {
899-
let cfg_json = serde_json::json!({
900-
"ottl_transform_config": {
901-
"trace_statements": ["set(attributes[\"tag\"], \"value\") where attributes[\"half\"] == \"yes\""]
902-
}
903-
});
904-
let mut transform = build_transform(Some(cfg_json)).await;
905-
906-
let spans: Vec<Span> = (0..PERF_SPANS_PER_BUFFER)
907-
.map(|i| {
908-
let half_val = if i % 2 == 0 { "yes" } else { "no" };
909-
make_span(1, i as u64, HashMap::from([("half".into(), half_val.to_string())]))
910-
})
911-
.collect();
912-
let template = perf_make_buffer(spans);
913-
let mut buffers: Vec<EventsBuffer> = (0..PERF_NUM_BUFFERS).map(|_| template.clone()).collect();
914-
let total_spans = PERF_NUM_BUFFERS * PERF_SPANS_PER_BUFFER;
915-
916-
let start = std::time::Instant::now();
917-
for buf in &mut buffers {
918-
transform.transform_buffer(buf);
919-
}
920-
let elapsed = start.elapsed();
921-
let throughput = total_spans as f64 / elapsed.as_secs_f64();
922-
println!(
923-
"perf_throughput_set_half: {} spans in {:?} -> {:.0} spans/s",
924-
total_spans, elapsed, throughput
925-
);
926-
}
927-
928-
/// Run with: `cargo test -p agent-data-plane perf_throughput_set_none --release -- --ignored --nocapture`
929-
#[tokio::test]
930-
#[ignore = "performance test; run with: cargo test -- --ignored --nocapture perf_throughput"]
931-
async fn perf_throughput_set_none() {
932-
let cfg_json = serde_json::json!({
933-
"ottl_transform_config": {
934-
"trace_statements": ["set(attributes[\"tag\"], \"value\") where attributes[\"nomatch\"] == \"yes\""]
935-
}
936-
});
937-
let mut transform = build_transform(Some(cfg_json)).await;
938-
939-
let spans: Vec<Span> = (0..PERF_SPANS_PER_BUFFER)
940-
.map(|i| make_span(1, i as u64, HashMap::from([("env".into(), "keep".into())])))
941-
.collect();
942-
let template = perf_make_buffer(spans);
943-
let mut buffers: Vec<EventsBuffer> = (0..PERF_NUM_BUFFERS).map(|_| template.clone()).collect();
944-
let total_spans = PERF_NUM_BUFFERS * PERF_SPANS_PER_BUFFER;
945-
946-
let start = std::time::Instant::now();
947-
for buf in &mut buffers {
948-
transform.transform_buffer(buf);
949-
}
950-
let elapsed = start.elapsed();
951-
let throughput = total_spans as f64 / elapsed.as_secs_f64();
952-
println!(
953-
"perf_throughput_set_none: {} spans in {:?} -> {:.0} spans/s",
954-
total_spans, elapsed, throughput
955-
);
956-
}
957852
}

bin/agent-data-plane/src/components/tag_filterlist/mod.rs

Lines changed: 140 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -343,14 +343,30 @@ pub fn filter_metric_tags(
343343

344344
#[cfg(test)]
345345
mod tests {
346+
use std::sync::Arc;
347+
346348
use saluki_config::{dynamic::ConfigUpdate, ConfigurationLoader};
347349
use saluki_context::{
348350
tags::{Tag, TagSet},
349351
Context, TagSetMutViewState,
350352
};
351-
use saluki_core::data_model::event::metric::Metric;
353+
use saluki_core::accounting::{ComponentRegistry, MemoryLimiter};
354+
use saluki_core::components::{
355+
transforms::{TransformBuilder, TransformContext},
356+
ComponentContext,
357+
};
358+
use saluki_core::data_model::event::{
359+
metric::{Metric, MetricValues},
360+
Event,
361+
};
362+
use saluki_core::health::HealthRegistry;
363+
use saluki_core::runtime::{state::DataspaceRegistry, Supervisor};
364+
use saluki_core::topology::interconnect::{Consumer, Dispatcher};
365+
use saluki_core::topology::{EventsBuffer, OutputName, TopologyContext};
352366
use saluki_metrics::{test::TestRecorder, MetricsBuilder};
353367
use serde_json::json;
368+
use tokio::runtime::Handle;
369+
use tokio::sync::mpsc;
354370

355371
use super::*;
356372

@@ -1038,4 +1054,127 @@ mod tests {
10381054
assert!(!metric.context().tags().is_modified());
10391055
assert!(!metric.context().origin_tags().is_modified());
10401056
}
1057+
1058+
#[tokio::test]
1059+
async fn run_loop_enforces_type_guard_and_exercises_context_cache() {
1060+
// The other tests call `filter_metric_tags` directly; this one drives the real `Transform::run()` loop to
1061+
// cover two behaviors those can't reach:
1062+
// 1. the run-loop type guard filters only distribution (sketch) and count metrics, leaving other metric
1063+
// types (a gauge here) completely untouched even when a rule matches their name; and
1064+
// 2. the per-context dedup cache is exercised end-to-end -- the cache is keyed by `Context`, so metrics
1065+
// that share a (name, tags) context resolve to a single cache entry, and every metric sharing that
1066+
// context is filtered identically (the second and later occurrences take the cache-hit branch).
1067+
1068+
let cfg_json = json!({
1069+
"metric_tag_filterlist": [
1070+
{ "metric_name": "svc.latency", "action": "exclude", "tags": ["host"] }
1071+
]
1072+
});
1073+
let (config, _sender) = ConfigurationLoader::for_tests(Some(cfg_json), None, false).await;
1074+
let builder = TagFilterlistConfiguration::from_configuration(&config).expect("config should parse");
1075+
1076+
let component_context = ComponentContext::test_transform("tag_filterlist");
1077+
let transform = builder
1078+
.build(component_context.clone())
1079+
.await
1080+
.expect("tag filterlist should build");
1081+
1082+
// Wire a dispatcher whose default output we can drain after the run loop completes.
1083+
let mut dispatcher = Dispatcher::new(component_context.clone());
1084+
dispatcher.add_output(OutputName::Default).expect("add default output");
1085+
let (out_tx, mut out_rx) = mpsc::channel(4);
1086+
dispatcher
1087+
.attach_sender_to_output(&OutputName::Default, out_tx)
1088+
.expect("attach default sender");
1089+
1090+
// A distribution, a counter, and a gauge that all share the same (name, tags) context, followed by a repeat
1091+
// of the distribution. The counter and the repeated distribution hit the cache entry created by the first
1092+
// distribution.
1093+
let tags = &["host:h1", "env:prod"];
1094+
let mut input = EventsBuffer::default();
1095+
for event in [
1096+
Event::Metric(Metric::distribution(
1097+
Context::from_static_parts("svc.latency", tags),
1098+
1.0,
1099+
)),
1100+
Event::Metric(Metric::counter(Context::from_static_parts("svc.latency", tags), 1.0)),
1101+
Event::Metric(Metric::gauge(Context::from_static_parts("svc.latency", tags), 1.0)),
1102+
Event::Metric(Metric::distribution(
1103+
Context::from_static_parts("svc.latency", tags),
1104+
1.0,
1105+
)),
1106+
] {
1107+
assert!(input.try_push(event).is_none(), "input buffer should have capacity");
1108+
}
1109+
1110+
let (in_tx, in_rx) = mpsc::channel(4);
1111+
let consumer = Consumer::new(component_context.clone(), in_rx);
1112+
in_tx.send(input).await.expect("send input buffer");
1113+
drop(in_tx); // Closing the input makes the run loop terminate deterministically.
1114+
1115+
let topology_context = TopologyContext::new(
1116+
Arc::from("test"),
1117+
MemoryLimiter::noop(),
1118+
HealthRegistry::new(),
1119+
Handle::current(),
1120+
DataspaceRegistry::new(),
1121+
);
1122+
let health = HealthRegistry::new()
1123+
.register_component(&saluki_core::support::SubsystemIdentifier::from_dotted("test"))
1124+
.expect("component was not previously registered");
1125+
let supervisor_handle = Supervisor::new("test").expect("valid supervisor name").handle();
1126+
1127+
let context = TransformContext::new(
1128+
&topology_context,
1129+
&component_context,
1130+
ComponentRegistry::default(),
1131+
health,
1132+
dispatcher,
1133+
consumer,
1134+
supervisor_handle,
1135+
);
1136+
1137+
transform.run(context).await.expect("tag filterlist run should succeed");
1138+
1139+
let mut dispatched: Vec<Metric> = Vec::new();
1140+
while let Ok(buffer) = out_rx.try_recv() {
1141+
for event in buffer {
1142+
if let Event::Metric(metric) = event {
1143+
dispatched.push(metric);
1144+
}
1145+
}
1146+
}
1147+
1148+
// Nothing is dropped by the transform; order is preserved.
1149+
assert_eq!(dispatched.len(), 4, "all four metrics should be dispatched");
1150+
1151+
let sorted_tags = |metric: &Metric| {
1152+
let mut names: Vec<String> = metric
1153+
.context()
1154+
.tags()
1155+
.into_iter()
1156+
.map(|t| t.as_str().to_owned())
1157+
.collect();
1158+
names.sort();
1159+
names
1160+
};
1161+
1162+
// Distribution (sketch) -> filtered on the cache-miss path.
1163+
assert!(dispatched[0].values().is_sketch());
1164+
assert_eq!(sorted_tags(&dispatched[0]), vec!["env:prod"]);
1165+
// Counter (count metric) -> filtered via the cache-hit branch (shares the distribution's context entry).
1166+
assert!(matches!(dispatched[1].values(), MetricValues::Counter(_)));
1167+
assert_eq!(sorted_tags(&dispatched[1]), vec!["env:prod"]);
1168+
// Gauge -> NOT a sketch and NOT a counter, so the type guard skips it and it passes through untouched.
1169+
assert!(!dispatched[2].values().is_sketch());
1170+
assert!(!matches!(dispatched[2].values(), MetricValues::Counter(_)));
1171+
assert_eq!(
1172+
sorted_tags(&dispatched[2]),
1173+
vec!["env:prod", "host:h1"],
1174+
"gauge metrics must not be filtered by the type guard"
1175+
);
1176+
// Repeated distribution -> filtered via the cache-hit branch, identical to the first distribution.
1177+
assert!(dispatched[3].values().is_sketch());
1178+
assert_eq!(sorted_tags(&dispatched[3]), vec!["env:prod"]);
1179+
}
10411180
}

0 commit comments

Comments
 (0)