From ac34d2e0aba3d3dc7d25b0f8f4987c1cb1e8ace2 Mon Sep 17 00:00:00 2001 From: Will Killian Date: Mon, 13 Jul 2026 23:08:46 -0400 Subject: [PATCH 1/3] feat!: add multi-sink ATOF export Signed-off-by: Will Killian --- crates/cli/src/doctor.rs | 160 +++--- crates/cli/src/launcher.rs | 32 +- crates/cli/tests/coverage/doctor_tests.rs | 17 + crates/cli/tests/coverage/plugins_tests.rs | 22 +- crates/cli/tests/coverage/session_tests.rs | 6 +- crates/core/src/observability/atof.rs | 180 +++++-- .../src/observability/plugin_component.rs | 485 ++++++++++++++---- .../tests/unit/observability/atof_tests.rs | 61 ++- .../observability/plugin_component_tests.rs | 248 +++++++-- crates/ffi/nemo_relay.h | 2 +- crates/ffi/src/api/observability.rs | 8 +- .../integration/api/coverage_sweeps_tests.rs | 2 +- crates/ffi/tests/integration/api_tests.rs | 2 +- crates/ffi/tests/unit/api/core_tests.rs | 15 +- crates/node/observability.d.ts | 15 +- crates/node/observability.js | 7 +- crates/node/src/api/mod.rs | 134 ++--- crates/node/tests/atof_tests.mjs | 33 +- .../node/tests/observability_plugin_tests.mjs | 28 +- .../tests/unit/component_tests.rs | 2 +- crates/python/src/py_types/observability.rs | 89 +++- go/nemo_relay/atof_test.go | 51 +- go/nemo_relay/nemo_relay.go | 128 +++-- go/nemo_relay/observability_plugin.go | 76 ++- go/nemo_relay/observability_plugin_test.go | 26 +- justfile | 6 + python/nemo_relay/__init__.py | 4 +- python/nemo_relay/__init__.pyi | 6 +- python/nemo_relay/_native.pyi | 24 +- python/nemo_relay/observability.py | 40 +- python/nemo_relay/observability.pyi | 11 +- python/tests/test_observability_plugin.py | 34 +- python/tests/test_types.py | 40 +- 33 files changed, 1400 insertions(+), 594 deletions(-) diff --git a/crates/cli/src/doctor.rs b/crates/cli/src/doctor.rs index f86999626..dee7d4d93 100644 --- a/crates/cli/src/doctor.rs +++ b/crates/cli/src/doctor.rs @@ -736,29 +736,67 @@ async fn collect_observability(gateway: &GatewayConfig) -> Vec { } async fn collect_observability_component_checks(checks: &mut Vec, config: &Value) { - for section in ["atof", "atif"] { - if let Some(check) = observability_file_exporter_check(config, section) { - checks.push(check); - } + checks.extend(observability_atof_file_checks(config)); + if let Some(check) = observability_file_exporter_check(config, "atif") { + checks.push(check); } for section in ["opentelemetry", "openinference"] { if let Some(check) = observability_http_exporter_check(config, section).await { checks.push(check); } } - if section_enabled(config, "atof") && atof_endpoint_count(config) > 0 { + if section_enabled(config, "atof") && !atof_stream_sinks(config).is_empty() { if atof_streaming_supported() { - checks.extend(observability_atof_endpoint_checks(config).await); + checks.extend(observability_atof_stream_checks(config).await); } else { checks.push(Check { - name: "ATOF endpoint", + name: "ATOF stream sink", status: Status::Fail, - details: "ATOF streaming endpoints are not available in this binary".into(), + details: "ATOF stream sinks are not available in this binary".into(), }); } } } +fn observability_atof_file_checks(config: &Value) -> Vec { + if !section_enabled(config, "atof") { + return Vec::new(); + } + let sinks = config + .get("atof") + .and_then(|section| section.get("sinks")) + .and_then(Value::as_array) + .into_iter() + .flatten() + .enumerate() + .filter(|(_, sink)| sink.get("type").and_then(Value::as_str) == Some("file")); + let checks = sinks + .map( + |(index, sink)| match sink.get("output_directory").and_then(Value::as_str) { + Some(path) => { + let mut check = check_directory("ATOF file sink", Path::new(path)); + check.details = format!("sinks[{index}]: {}", check.details); + check + } + None => Check { + name: "ATOF file sink", + status: Status::Info, + details: format!("sinks[{index}] uses the runtime default output directory"), + }, + }, + ) + .collect::>(); + if checks.is_empty() { + vec![Check { + name: "ATOF file sink", + status: Status::Info, + details: "no file sinks configured".into(), + }] + } else { + checks + } +} + fn observability_file_exporter_check(config: &Value, section: &str) -> Option { if !section_enabled(config, section) { return None; @@ -919,40 +957,41 @@ fn section_endpoint(config: &Value, section: &str) -> Option { .map(str::to_string) } -fn atof_endpoint_count(config: &Value) -> usize { +fn atof_stream_sinks(config: &Value) -> Vec<(usize, &Value)> { config .get("atof") - .and_then(|section| section.get("endpoints")) + .and_then(|section| section.get("sinks")) .and_then(Value::as_array) - .map_or(0, Vec::len) + .map(|sinks| { + sinks + .iter() + .enumerate() + .filter(|(_, sink)| sink.get("type").and_then(Value::as_str) == Some("stream")) + .collect() + }) + .unwrap_or_default() } fn atof_streaming_supported() -> bool { cfg!(feature = "atof-streaming") } -async fn observability_atof_endpoint_checks(config: &Value) -> Vec { - let Some(endpoints) = config - .get("atof") - .and_then(|section| section.get("endpoints")) - .and_then(Value::as_array) - else { - return Vec::new(); - }; - let mut checks = Vec::with_capacity(endpoints.len()); - for (index, endpoint) in endpoints.iter().enumerate() { - checks.push(probe_atof_endpoint(index, endpoint).await); +async fn observability_atof_stream_checks(config: &Value) -> Vec { + let streams = atof_stream_sinks(config); + let mut checks = Vec::with_capacity(streams.len()); + for (index, sink) in streams { + checks.push(probe_atof_stream_sink(index, sink).await); } checks } -async fn probe_atof_endpoint(index: usize, endpoint: &Value) -> Check { - let name = "ATOF endpoint"; +async fn probe_atof_stream_sink(index: usize, endpoint: &Value) -> Check { + let name = "ATOF stream sink"; let Some(url) = endpoint.get("url").and_then(Value::as_str) else { return Check { name, status: Status::Fail, - details: format!("endpoints[{index}]: missing url"), + details: format!("sinks[{index}]: missing url"), }; }; let transport = endpoint @@ -967,7 +1006,7 @@ async fn probe_atof_endpoint(index: usize, endpoint: &Value) -> Check { return Check { name, status: Status::Fail, - details: format!("endpoints[{index}] {transport} {url}: timeout_millis must be > 0"), + details: format!("sinks[{index}] {transport} {url}: timeout_millis must be > 0"), }; } let headers = match endpoint_headers(endpoint) { @@ -976,7 +1015,7 @@ async fn probe_atof_endpoint(index: usize, endpoint: &Value) -> Check { return Check { name, status: Status::Fail, - details: format!("endpoints[{index}] {transport} {url}: {err}"), + details: format!("sinks[{index}] {transport} {url}: {err}"), }; } }; @@ -986,7 +1025,7 @@ async fn probe_atof_endpoint(index: usize, endpoint: &Value) -> Check { return Check { name, status: Status::Fail, - details: format!("endpoints[{index}] {transport} {url}: {err}"), + details: format!("sinks[{index}] {transport} {url}: {err}"), }; } }; @@ -998,11 +1037,16 @@ async fn probe_atof_endpoint(index: usize, endpoint: &Value) -> Check { _ => Check { name, status: Status::Fail, - details: format!("endpoints[{index}] {transport} {url}: unsupported transport"), + details: format!("sinks[{index}] {transport} {url}: unsupported transport"), }, } } +#[cfg(test)] +async fn probe_atof_endpoint(index: usize, endpoint: &Value) -> Check { + probe_atof_stream_sink(index, endpoint).await +} + fn endpoint_headers(endpoint: &Value) -> Result, String> { let mut out = Vec::new(); let mut names = std::collections::HashSet::new(); @@ -1096,11 +1140,9 @@ async fn probe_atof_http_upload( Ok(client) => client, Err(err) => { return Check { - name: "ATOF endpoint", + name: "ATOF stream sink", status: Status::Fail, - details: format!( - "endpoints[{index}] {transport} {url}: could not build client: {err}" - ), + details: format!("sinks[{index}] {transport} {url}: could not build client: {err}"), }; } }; @@ -1113,25 +1155,25 @@ async fn probe_atof_http_upload( } match request.send().await { Ok(response) if response.status().is_success() => Check { - name: "ATOF endpoint", + name: "ATOF stream sink", status: Status::Pass, details: format!( - "endpoints[{index}] {transport} {url} (HTTP {})", + "sinks[{index}] {transport} {url} (HTTP {})", response.status() ), }, Ok(response) => Check { - name: "ATOF endpoint", + name: "ATOF stream sink", status: Status::Fail, details: format!( - "endpoints[{index}] {transport} {url} (HTTP {})", + "sinks[{index}] {transport} {url} (HTTP {})", response.status() ), }, Err(err) => Check { - name: "ATOF endpoint", + name: "ATOF stream sink", status: Status::Fail, - details: format!("endpoints[{index}] {transport} {url}: {err}"), + details: format!("sinks[{index}] {transport} {url}: {err}"), }, } } @@ -1147,18 +1189,18 @@ async fn probe_atof_websocket( Ok(parsed) if matches!(parsed.scheme(), "ws" | "wss") => {} Ok(_) => { return Check { - name: "ATOF endpoint", + name: "ATOF stream sink", status: Status::Fail, details: format!( - "endpoints[{index}] websocket {url}: invalid scheme (must be ws or wss)" + "sinks[{index}] websocket {url}: invalid scheme (must be ws or wss)" ), }; } Err(err) => { return Check { - name: "ATOF endpoint", + name: "ATOF stream sink", status: Status::Fail, - details: format!("endpoints[{index}] websocket {url}: {err}"), + details: format!("sinks[{index}] websocket {url}: {err}"), }; } } @@ -1166,9 +1208,9 @@ async fn probe_atof_websocket( Ok(request) => request, Err(err) => { return Check { - name: "ATOF endpoint", + name: "ATOF stream sink", status: Status::Fail, - details: format!("endpoints[{index}] websocket {url}: {err}"), + details: format!("sinks[{index}] websocket {url}: {err}"), }; } }; @@ -1179,9 +1221,9 @@ async fn probe_atof_websocket( Ok(name) => name, Err(err) => { return Check { - name: "ATOF endpoint", + name: "ATOF stream sink", status: Status::Fail, - details: format!("endpoints[{index}] websocket {url}: {err}"), + details: format!("sinks[{index}] websocket {url}: {err}"), }; } }; @@ -1190,9 +1232,9 @@ async fn probe_atof_websocket( Ok(value) => value, Err(err) => { return Check { - name: "ATOF endpoint", + name: "ATOF stream sink", status: Status::Fail, - details: format!("endpoints[{index}] websocket {url}: {err}"), + details: format!("sinks[{index}] websocket {url}: {err}"), }; } }; @@ -1210,33 +1252,33 @@ async fn probe_atof_websocket( let _ = timeout(timeout_duration, socket.close(None)).await; match send { Ok(Ok(())) => Check { - name: "ATOF endpoint", + name: "ATOF stream sink", status: Status::Pass, - details: format!("endpoints[{index}] websocket {url}"), + details: format!("sinks[{index}] websocket {url}"), }, Ok(Err(err)) => Check { - name: "ATOF endpoint", + name: "ATOF stream sink", status: Status::Fail, - details: format!("endpoints[{index}] websocket {url}: {err}"), + details: format!("sinks[{index}] websocket {url}: {err}"), }, Err(_) => Check { - name: "ATOF endpoint", + name: "ATOF stream sink", status: Status::Fail, details: format!( - "endpoints[{index}] websocket {url}: timed out sending probe payload" + "sinks[{index}] websocket {url}: timed out sending probe payload" ), }, } } Ok(Err(err)) => Check { - name: "ATOF endpoint", + name: "ATOF stream sink", status: Status::Fail, - details: format!("endpoints[{index}] websocket {url}: {err}"), + details: format!("sinks[{index}] websocket {url}: {err}"), }, Err(_) => Check { - name: "ATOF endpoint", + name: "ATOF stream sink", status: Status::Fail, - details: format!("endpoints[{index}] websocket {url}: timed out"), + details: format!("sinks[{index}] websocket {url}: timed out"), }, } } diff --git a/crates/cli/src/launcher.rs b/crates/cli/src/launcher.rs index 40954bf58..7c3fa6116 100644 --- a/crates/cli/src/launcher.rs +++ b/crates/cli/src/launcher.rs @@ -671,17 +671,27 @@ pub(crate) fn exporter_destinations(config: &GatewayConfig) -> Vec { fn observability_exporter_destinations(config: &ObservabilityConfig) -> Vec { let mut destinations = Vec::new(); if let Some(section) = config.atof.as_ref().filter(|section| section.enabled) { - let directory = section - .output_directory - .clone() - .unwrap_or_else(current_output_directory); - let path = directory.join( - section - .filename - .clone() - .unwrap_or_else(|| "nemo-relay-events-.jsonl".into()), - ); - destinations.push(format!("ATOF {}", path.display())); + for sink in §ion.sinks { + match sink { + nemo_relay::observability::plugin_component::AtofSinkSectionConfig::File(file) => { + let directory = file + .output_directory + .clone() + .unwrap_or_else(current_output_directory); + let path = directory.join( + file.filename + .clone() + .unwrap_or_else(|| "nemo-relay-events-.jsonl".into()), + ); + destinations.push(format!("ATOF {}", path.display())); + } + nemo_relay::observability::plugin_component::AtofSinkSectionConfig::Stream( + stream, + ) => { + destinations.push(format!("ATOF {}", stream.url)); + } + } + } } if let Some(section) = config.atif.as_ref().filter(|section| section.enabled) { if section.storage.is_empty() { diff --git a/crates/cli/tests/coverage/doctor_tests.rs b/crates/cli/tests/coverage/doctor_tests.rs index fb3f751cb..cec9772e8 100644 --- a/crates/cli/tests/coverage/doctor_tests.rs +++ b/crates/cli/tests/coverage/doctor_tests.rs @@ -872,6 +872,23 @@ fn observability_component_helpers_cover_disabled_and_default_paths() { assert!(default_dir.details.contains("runtime default")); } +#[test] +fn atof_file_checks_preserve_configured_sink_indices() { + let config = serde_json::json!({ + "atof": { + "enabled": true, + "sinks": [ + {"type": "stream", "url": "http://127.0.0.1/events"}, + {"type": "file"} + ] + } + }); + + let checks = observability_atof_file_checks(&config); + assert_eq!(checks.len(), 1); + assert!(checks[0].details.starts_with("sinks[1]")); +} + #[test] fn check_directory_reports_pass_warn_and_fail() { let temp = tempfile::tempdir().unwrap(); diff --git a/crates/cli/tests/coverage/plugins_tests.rs b/crates/cli/tests/coverage/plugins_tests.rs index 493c487b6..bc264b416 100644 --- a/crates/cli/tests/coverage/plugins_tests.rs +++ b/crates/cli/tests/coverage/plugins_tests.rs @@ -9,7 +9,9 @@ use crate::config::{ use nemo_relay::config_editor::{ EditorConfig, EditorListItemSpec, EditorSchema, EditorTaggedUnionSpec, EditorVariantSpec, }; -use nemo_relay::observability::plugin_component::{OBSERVABILITY_PLUGIN_KIND, ObservabilityConfig}; +use nemo_relay::observability::plugin_component::{ + AtofSectionConfig, OBSERVABILITY_PLUGIN_KIND, ObservabilityConfig, +}; use nemo_relay::plugin::{ConfigPolicy, PluginComponentSpec, PluginConfig}; use nemo_relay::plugins::nemo_guardrails::component::{ LocalBackendConfig, NEMO_GUARDRAILS_PLUGIN_KIND, NeMoGuardrailsConfig, RemoteBackendConfig, @@ -2533,3 +2535,21 @@ fn target_path_resolves_user_scope_from_xdg_and_reports_missing_home() { } drop(guard); } + +#[test] +fn typed_list_metadata_describes_atof_sinks() { + let sinks = AtofSectionConfig::editor_schema().field("sinks").unwrap(); + let sink_item = sinks.list_item.expect("ATOF sink item metadata"); + assert_eq!(sinks.kind, EditorFieldKind::List); + assert_eq!(sink_item.kind, EditorFieldKind::Section); + assert_eq!( + sink_item + .tagged_union + .expect("ATOF sink tagged union") + .variants + .iter() + .map(|variant| variant.tag) + .collect::>(), + vec!["file", "stream"] + ); +} diff --git a/crates/cli/tests/coverage/session_tests.rs b/crates/cli/tests/coverage/session_tests.rs index 9e2f5cae2..fc528af71 100644 --- a/crates/cli/tests/coverage/session_tests.rs +++ b/crates/cli/tests/coverage/session_tests.rs @@ -3019,7 +3019,7 @@ async fn hermes_orphan_subagent_stop_links_atof_and_openinference_to_turn() { assert!(deregister_subscriber(atof_name).unwrap()); assert!(deregister_subscriber(openinference_name).unwrap()); - let atof_events = read_atof_events(atof_exporter.path()); + let atof_events = read_atof_events(atof_exporter.path().expect("file sink path")); let turn_start = atof_events .iter() .find(|event| { @@ -3162,7 +3162,7 @@ async fn hermes_subagent_child_session_preserves_atof_and_openinference_lineage( assert!(deregister_subscriber(atof_name).unwrap()); assert!(deregister_subscriber(openinference_name).unwrap()); - let atof_events = read_atof_events(atof_exporter.path()); + let atof_events = read_atof_events(atof_exporter.path().expect("file sink path")); let parent_turn = atof_events .iter() .find(|event| { @@ -3506,7 +3506,7 @@ async fn inferred_skill_load_hook_marks_use_the_stable_event_contract() { atof_exporter.force_flush().unwrap(); assert!(deregister_subscriber(subscriber_name).unwrap()); - let events = read_atof_events(atof_exporter.path()); + let events = read_atof_events(atof_exporter.path().expect("file sink path")); let marks = events .iter() .filter(|event| event["name"] == "skill.load.inferred") diff --git a/crates/core/src/observability/atof.rs b/crates/core/src/observability/atof.rs index ffa2013f7..fe978551a 100644 --- a/crates/core/src/observability/atof.rs +++ b/crates/core/src/observability/atof.rs @@ -166,9 +166,9 @@ impl AtofEndpointTransport { } } -/// Streaming destination for raw ATOF events. +/// Streaming sink for raw ATOF events. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub struct AtofEndpointConfig { +pub struct AtofStreamSinkConfig { /// Endpoint URL. pub url: String, /// Endpoint transport. @@ -188,7 +188,7 @@ pub struct AtofEndpointConfig { pub field_name_policy: AtofEndpointFieldNamePolicy, } -impl AtofEndpointConfig { +impl AtofStreamSinkConfig { /// Create a streaming endpoint with defaults. pub fn new(url: impl Into, transport: AtofEndpointTransport) -> Self { Self { @@ -234,9 +234,12 @@ impl AtofEndpointConfig { } } -/// Configuration for [`AtofExporter`]. +/// Backward-compatible name for an ATOF stream sink. +pub type AtofEndpointConfig = AtofStreamSinkConfig; + +/// Filesystem sink for raw ATOF JSONL events. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub struct AtofExporterConfig { +pub struct AtofFileSinkConfig { /// Directory that contains the JSONL output file. #[serde(default = "default_output_directory")] pub output_directory: PathBuf, @@ -246,18 +249,52 @@ pub struct AtofExporterConfig { /// Output filename. #[serde(default = "default_filename")] pub filename: String, - /// Optional streaming endpoints that receive every raw ATOF event. - #[serde(default, skip_serializing_if = "Vec::is_empty")] - pub endpoints: Vec, } -impl Default for AtofExporterConfig { +impl Default for AtofFileSinkConfig { fn default() -> Self { Self { - output_directory: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), + output_directory: default_output_directory(), mode: AtofExporterMode::Append, filename: default_filename(), - endpoints: Vec::new(), + } + } +} + +impl AtofFileSinkConfig { + /// Create a file sink with native defaults. + pub fn new() -> Self { + Self::default() + } + + /// Return the full output path for this sink. + pub fn path(&self) -> PathBuf { + self.output_directory.join(&self.filename) + } +} + +/// One destination for raw ATOF events. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(tag = "type", rename_all = "snake_case")] +pub enum AtofSinkConfig { + /// Write canonical ATOF records to one JSONL file. + File(AtofFileSinkConfig), + /// Send canonical ATOF records to one remote stream. + Stream(AtofStreamSinkConfig), +} + +/// Configuration for [`AtofExporter`]. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct AtofExporterConfig { + /// The one output sink owned by this exporter. + #[serde(flatten)] + pub sink: AtofSinkConfig, +} + +impl Default for AtofExporterConfig { + fn default() -> Self { + Self { + sink: AtofSinkConfig::File(AtofFileSinkConfig::default()), } } } @@ -270,42 +307,53 @@ impl AtofExporterConfig { /// Override the output directory. pub fn with_output_directory(mut self, output_directory: impl Into) -> Self { - self.output_directory = output_directory.into(); + if let AtofSinkConfig::File(file) = &mut self.sink { + file.output_directory = output_directory.into(); + } self } /// Override the output mode. pub fn with_mode(mut self, mode: AtofExporterMode) -> Self { - self.mode = mode; + if let AtofSinkConfig::File(file) = &mut self.sink { + file.mode = mode; + } self } /// Override the output filename. pub fn with_filename(mut self, filename: impl Into) -> Self { - self.filename = filename.into(); + if let AtofSinkConfig::File(file) = &mut self.sink { + file.filename = filename.into(); + } self } - /// Override streaming endpoints. - pub fn with_endpoints(mut self, endpoints: Vec) -> Self { - self.endpoints = endpoints; + /// Select one stream sink. + pub fn with_stream_sink(mut self, sink: AtofStreamSinkConfig) -> Self { + self.sink = AtofSinkConfig::Stream(sink); self } - /// Add one streaming endpoint. - pub fn with_endpoint(mut self, endpoint: AtofEndpointConfig) -> Self { - self.endpoints.push(endpoint); - self + /// Select one stream sink. + /// + /// This compatibility spelling replaces the configured file sink; use the + /// observability plugin's `sinks` array for file-and-stream fan-out. + pub fn with_endpoint(self, endpoint: AtofEndpointConfig) -> Self { + self.with_stream_sink(endpoint) } /// Return the full output path for this config. - pub fn path(&self) -> PathBuf { - self.output_directory.join(&self.filename) + pub fn path(&self) -> Option { + match &self.sink { + AtofSinkConfig::File(file) => Some(file.path()), + AtofSinkConfig::Stream(_) => None, + } } } struct AtofExporterState { - writer: BufWriter, + writer: Option>, last_error: Option, endpoints: Vec, closed: bool, @@ -313,24 +361,34 @@ struct AtofExporterState { /// Filesystem-backed Agent Trajectory Observability Format (ATOF) JSONL event exporter. pub struct AtofExporter { - path: PathBuf, + path: Option, state: Arc>, } impl AtofExporter { /// Create a new exporter from config and open its output file. pub fn new(config: AtofExporterConfig) -> Result { - let path = config.path(); - create_dir_all(&config.output_directory).map_err(|source| AtofExporterError::OpenFile { - path: path.clone(), - source, - })?; - let file = open_file(&path, config.mode)?; - let endpoints = start_endpoint_workers(&config.endpoints)?; + let (path, writer, endpoints) = match config.sink { + AtofSinkConfig::File(file_sink) => { + let path = file_sink.path(); + create_dir_all(&file_sink.output_directory).map_err(|source| { + AtofExporterError::OpenFile { + path: path.clone(), + source, + } + })?; + let file = open_file(&path, file_sink.mode)?; + (Some(path), Some(BufWriter::new(file)), Vec::new()) + } + AtofSinkConfig::Stream(stream_sink) => { + let workers = start_endpoint_workers(&[stream_sink])?; + (None, None, workers) + } + }; Ok(Self { path, state: Arc::new(Mutex::new(AtofExporterState { - writer: BufWriter::new(file), + writer, last_error: None, endpoints, closed: false, @@ -339,8 +397,8 @@ impl AtofExporter { } /// Return the output JSONL path. - pub fn path(&self) -> &Path { - self.path.as_path() + pub fn path(&self) -> Option<&Path> { + self.path.as_deref() } /// Return an event subscriber that writes one JSONL record per observed event. @@ -357,7 +415,9 @@ impl AtofExporter { state.last_error = Some("failed to serialize ATOF event".to_string()); return; }; - if let Err(error) = write_json_value(&mut state.writer, &value) { + if let Some(writer) = &mut state.writer + && let Err(error) = write_json_value(writer, &value) + { state.last_error = Some(error); return; } @@ -389,19 +449,34 @@ impl AtofExporter { .lock() .map_err(|_| AtofExporterError::LockPoisoned)?; if state.closed { - return stored_failure_result(&self.path, &state); + return stored_failure_result( + self.path + .as_deref() + .unwrap_or_else(|| Path::new("")), + &state, + ); } state .writer - .flush() + .as_mut() + .map(|writer| writer.flush()) + .transpose() .map_err(|source| AtofExporterError::Flush { - path: self.path.clone(), + path: self + .path + .clone() + .unwrap_or_else(|| PathBuf::from("")), source, })?; for endpoint in &state.endpoints { endpoint.flush(); } - stored_failure_result(&self.path, &state) + stored_failure_result( + self.path + .as_deref() + .unwrap_or_else(|| Path::new("")), + &state, + ) } /// Shut down the exporter by flushing buffered data and closing endpoints. @@ -412,21 +487,36 @@ impl AtofExporter { .lock() .map_err(|_| AtofExporterError::LockPoisoned)?; if state.closed { - return stored_failure_result(&self.path, &state); + return stored_failure_result( + self.path + .as_deref() + .unwrap_or_else(|| Path::new("")), + &state, + ); } state.closed = true; let flush_result = state .writer - .flush() + .as_mut() + .map(|writer| writer.flush()) + .transpose() .map_err(|source| AtofExporterError::Flush { - path: self.path.clone(), + path: self + .path + .clone() + .unwrap_or_else(|| PathBuf::from("")), source, }); for endpoint in &state.endpoints { endpoint.close(); } flush_result?; - stored_failure_result(&self.path, &state) + stored_failure_result( + self.path + .as_deref() + .unwrap_or_else(|| Path::new("")), + &state, + ) } } @@ -532,7 +622,7 @@ impl AtofEndpointWorker { } #[cfg(feature = "atof-streaming")] -fn start_endpoint_workers(configs: &[AtofEndpointConfig]) -> Result> { +fn start_endpoint_workers(configs: &[AtofStreamSinkConfig]) -> Result> { let mut workers = Vec::with_capacity(configs.len()); for (index, config) in configs.iter().enumerate() { match start_endpoint_worker(index, config.clone()) { diff --git a/crates/core/src/observability/plugin_component.rs b/crates/core/src/observability/plugin_component.rs index f90de36f5..bb7e307f2 100644 --- a/crates/core/src/observability/plugin_component.rs +++ b/crates/core/src/observability/plugin_component.rs @@ -35,12 +35,15 @@ use crate::api::scope::ScopeType; use crate::api::subscriber::{ scope_deregister_subscriber, try_scope_deregister_subscriber, try_scope_register_subscriber, }; +use crate::config_editor::{ + EditorConfig, EditorFieldKind, EditorListItemSpec, EditorTaggedUnionSpec, EditorVariantSpec, +}; use crate::error::FlowError; use crate::observability::atif::{AtifAgentInfo, AtifExporter}; use crate::observability::atof::{ - AtofEndpointConfig as CoreAtofEndpointConfig, AtofEndpointFieldNamePolicy, - AtofEndpointTransport, AtofExporter, AtofExporterConfig as CoreAtofExporterConfig, - AtofExporterMode, + AtofEndpointFieldNamePolicy, AtofEndpointTransport, AtofExporter, + AtofExporterConfig as CoreAtofExporterConfig, AtofExporterMode, AtofFileSinkConfig, + AtofSinkConfig as CoreAtofSinkConfig, AtofStreamSinkConfig, }; #[cfg(feature = "openinference")] use crate::observability::openinference::{ @@ -144,49 +147,53 @@ impl Default for ObservabilityConfig { } } -/// Filesystem-backed ATOF JSONL exporter config. +/// Multi-sink ATOF JSONL exporter config. /// /// When enabled, this section wraps /// [`crate::observability::atof::AtofExporter`] and writes the raw ATOF event -/// stream as JSONL. The exporter uses the current working directory and a -/// timestamped filename when no explicit path settings are supplied. -#[derive(Debug, Clone, Serialize, Deserialize)] +/// stream to one or more explicitly configured file or stream sinks. +#[derive(Debug, Clone, Default, Serialize, Deserialize)] #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] pub struct AtofSectionConfig { /// Whether ATOF JSONL export is active. #[serde(default)] pub enabled: bool, + /// Destinations that each receive every raw ATOF event. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub sinks: Vec, +} + +/// One plugin-managed destination for raw ATOF events. +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] +#[serde(tag = "type", rename_all = "snake_case")] +pub enum AtofSinkSectionConfig { + /// A local JSONL file. + File(AtofFileSinkSectionConfig), + /// A remote stream. + Stream(AtofStreamSinkSectionConfig), +} + +/// File sink settings for the ATOF plugin section. +#[derive(Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] +pub struct AtofFileSinkSectionConfig { /// Directory containing the JSONL output file. #[serde(default, skip_serializing_if = "Option::is_none")] pub output_directory: Option, - /// Output filename. Defaults to the underlying ATOF exporter timestamped filename. + /// Output filename. Defaults to the native timestamped filename. #[serde(default, skip_serializing_if = "Option::is_none")] pub filename: Option, /// File open mode: `append` or `overwrite`. #[serde(default = "default_atof_mode")] #[cfg_attr(feature = "schema", schemars(schema_with = "atof_mode_schema"))] pub mode: String, - /// Optional streaming endpoints that receive every raw ATOF event. - #[serde(default, skip_serializing_if = "Vec::is_empty")] - pub endpoints: Vec, } -impl Default for AtofSectionConfig { - fn default() -> Self { - Self { - enabled: false, - output_directory: None, - filename: None, - mode: default_atof_mode(), - endpoints: Vec::new(), - } - } -} - -/// Streaming destination for raw ATOF events. +/// Stream sink settings for the ATOF plugin section. #[derive(Debug, Clone, Serialize, Deserialize)] #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] -pub struct AtofEndpointSectionConfig { +pub struct AtofStreamSinkSectionConfig { /// Endpoint URL. pub url: String, /// Transport: `http_post`, `websocket`, or `ndjson`. @@ -476,10 +483,26 @@ crate::editor_config! { crate::editor_config! { impl AtofSectionConfig { enabled => { label: "enabled", kind: Boolean }, + sinks => { label: "sinks", kind: List, list: &ATOF_SINK_LIST }, + } +} + +crate::editor_config! { + impl AtofFileSinkSectionConfig { output_directory => { label: "output_directory", kind: String, optional: true }, filename => { label: "filename", kind: String, optional: true }, mode => { label: "mode", kind: Enum, values: ["append", "overwrite"] }, - endpoints => { label: "endpoints", kind: Json, optional: true }, + } +} + +crate::editor_config! { + impl AtofStreamSinkSectionConfig { + url => { label: "url", kind: String }, + transport => { label: "transport", kind: Enum, values: ["http_post", "websocket", "ndjson"] }, + headers => { label: "headers", kind: StringMap }, + header_env => { label: "header_env", kind: StringMap }, + timeout_millis => { label: "timeout_millis", kind: Integer }, + field_name_policy => { label: "field_name_policy", kind: Enum, values: ["preserve", "replace_dots"] }, } } @@ -497,6 +520,50 @@ crate::editor_config! { } } +fn default_atof_file_sink_editor_value() -> Json { + serde_json::json!({"type": "file", "mode": "append"}) +} + +fn default_atof_stream_sink_editor_value() -> Json { + serde_json::json!({ + "type": "stream", + "url": "", + "transport": "http_post", + "headers": {}, + "header_env": {}, + "timeout_millis": 3000, + "field_name_policy": "preserve", + }) +} + +static ATOF_SINK_VARIANTS: [EditorVariantSpec; 2] = [ + EditorVariantSpec { + label: "File", + tag: "file", + schema: ::editor_schema, + default: default_atof_file_sink_editor_value, + }, + EditorVariantSpec { + label: "Stream", + tag: "stream", + schema: ::editor_schema, + default: default_atof_stream_sink_editor_value, + }, +]; + +static ATOF_SINK_TAGGED_UNION: EditorTaggedUnionSpec = EditorTaggedUnionSpec { + discriminator: "type", + variants: &ATOF_SINK_VARIANTS, +}; + +static ATOF_SINK_LIST: EditorListItemSpec = EditorListItemSpec { + kind: EditorFieldKind::Section, + schema: None, + default: None, + tagged_union: Some(&ATOF_SINK_TAGGED_UNION), + list_item: None, +}; + crate::editor_config! { impl OtlpSectionConfig { enabled => { label: "enabled", kind: Boolean }, @@ -640,63 +707,91 @@ fn register_atof_exporter( section: AtofSectionConfig, ctx: &mut PluginRegistrationContext, ) -> PluginResult<()> { - let mode = AtofExporterMode::parse(§ion.mode).ok_or_else(|| { - PluginError::InvalidConfig("ATOF mode must be 'append' or 'overwrite'".to_string()) - })?; - let mut config = CoreAtofExporterConfig::new().with_mode(mode); - if let Some(output_directory) = section.output_directory { - config = config.with_output_directory(output_directory); - } - if let Some(filename) = section.filename { - config = config.with_filename(filename); - } - let endpoints = section - .endpoints + let exporters = section + .sinks .into_iter() .enumerate() - .map(|(index, endpoint)| build_atof_endpoint_config(index, endpoint)) + .map(|(index, sink)| { + let config = CoreAtofExporterConfig { + sink: build_atof_sink_config(index, sink)?, + }; + AtofExporter::new(config) + .map(Arc::new) + .map_err(observability_registration_error) + }) .collect::>>()?; - config = config.with_endpoints(endpoints); + let subscribers = exporters + .iter() + .map(|exporter| exporter.subscriber()) + .collect::>(); + let subscriber: EventSubscriberFn = Arc::new(move |event| { + for subscriber in &subscribers { + subscriber(event); + } + }); - let exporter = Arc::new(AtofExporter::new(config).map_err(observability_registration_error)?); - ctx.register_subscriber("atof", exporter.subscriber())?; + ctx.register_subscriber("atof", subscriber)?; ctx.add_registration(PluginRegistration::new( "observability", ctx.qualify_name("atof.shutdown"), Box::new(move || { - exporter - .shutdown() - .map_err(observability_registration_error) + let mut first_error = None; + for exporter in &exporters { + if let Err(error) = exporter.shutdown() { + first_error.get_or_insert_with(|| observability_registration_error(error)); + } + } + first_error.map_or(Ok(()), Err) }), )); Ok(()) } -fn build_atof_endpoint_config( +fn build_atof_sink_config( index: usize, - endpoint: AtofEndpointSectionConfig, -) -> PluginResult { - let transport = AtofEndpointTransport::parse(&endpoint.transport).ok_or_else(|| { - PluginError::InvalidConfig(format!( - "ATOF endpoints[{index}].transport must be 'http_post', 'websocket', or 'ndjson'" - )) - })?; - let field_name_policy = AtofEndpointFieldNamePolicy::parse(&endpoint.field_name_policy) - .ok_or_else(|| { - PluginError::InvalidConfig(format!( - "ATOF endpoints[{index}].field_name_policy must be 'preserve' or 'replace_dots'" - )) - })?; - let mut config = CoreAtofEndpointConfig::new(endpoint.url, transport) - .with_timeout_millis(endpoint.timeout_millis) - .with_field_name_policy(field_name_policy); - for (key, value) in endpoint.headers { - config = config.with_header(key, value); - } - for (key, variable) in endpoint.header_env { - config = config.with_header_env(key, variable); + sink: AtofSinkSectionConfig, +) -> PluginResult { + match sink { + AtofSinkSectionConfig::File(file) => { + let mode = AtofExporterMode::parse(&file.mode).ok_or_else(|| { + PluginError::InvalidConfig(format!( + "ATOF sinks[{index}].mode must be 'append' or 'overwrite'" + )) + })?; + let mut sink = AtofFileSinkConfig::new(); + sink.mode = mode; + if let Some(output_directory) = file.output_directory { + sink.output_directory = output_directory; + } + if let Some(filename) = file.filename { + sink.filename = filename; + } + Ok(CoreAtofSinkConfig::File(sink)) + } + AtofSinkSectionConfig::Stream(stream) => { + let transport = AtofEndpointTransport::parse(&stream.transport).ok_or_else(|| { + PluginError::InvalidConfig(format!( + "ATOF sinks[{index}].transport must be 'http_post', 'websocket', or 'ndjson'" + )) + })?; + let field_name_policy = AtofEndpointFieldNamePolicy::parse(&stream.field_name_policy) + .ok_or_else(|| { + PluginError::InvalidConfig(format!( + "ATOF sinks[{index}].field_name_policy must be 'preserve' or 'replace_dots'" + )) + })?; + let mut config = AtofStreamSinkConfig::new(stream.url, transport) + .with_timeout_millis(stream.timeout_millis) + .with_field_name_policy(field_name_policy); + for (key, value) in stream.headers { + config = config.with_header(key, value); + } + for (key, variable) in stream.header_env { + config = config.with_header_env(key, variable); + } + Ok(CoreAtofSinkConfig::Stream(config)) + } } - Ok(config) } type AtifStorageList = Arc>>; @@ -1495,14 +1590,24 @@ fn validate_observability_section_fields( policy, plugin_config, "atof", - &[ - "enabled", - "output_directory", - "filename", - "mode", - "endpoints", - ], + &["enabled", "sinks"], ); + if let Some(atof) = plugin_config.get("atof").and_then(Json::as_object) { + for legacy_field in ["output_directory", "filename", "mode", "endpoints"] { + if atof.contains_key(legacy_field) { + push_policy_diag( + diagnostics, + UnsupportedBehavior::Error, + "observability.legacy_atof_field", + Some("atof".to_string()), + Some(legacy_field.to_string()), + format!( + "ATOF {legacy_field} was removed in observability config version 2; configure typed ATOF sinks instead" + ), + ); + } + } + } validate_section_fields( diagnostics, policy, @@ -1595,14 +1700,19 @@ fn validate_atof_feature_support( policy: &ConfigPolicy, section: &AtofSectionConfig, ) { - if section.enabled && !section.endpoints.is_empty() { + if section.enabled + && section + .sinks + .iter() + .any(|sink| matches!(sink, AtofSinkSectionConfig::Stream(_))) + { push_policy_diag( diagnostics, policy.unsupported_value, "observability.unsupported_value", Some("atof".to_string()), - Some("endpoints".to_string()), - "ATOF streaming endpoints are not enabled in this build".to_string(), + Some("sinks".to_string()), + "ATOF stream sinks are not enabled in this build".to_string(), ); } } @@ -1729,14 +1839,16 @@ fn validate_openinference_feature_support( } fn validate_version(diagnostics: &mut Vec, policy: &ConfigPolicy, version: u32) { - if version != 1 { + if version != 2 { push_policy_diag( diagnostics, policy.unsupported_value, "observability.unsupported_config_version", Some(OBSERVABILITY_PLUGIN_KIND.to_string()), Some("version".to_string()), - format!("observability config version {version} is unsupported"), + format!( + "observability config version {version} is unsupported; use version 2 and migrate ATOF output_directory, filename, mode, and endpoints into sinks" + ), ); } } @@ -1780,56 +1892,75 @@ fn validate_atof_values( policy: &ConfigPolicy, section: &AtofSectionConfig, ) { - if AtofExporterMode::parse(§ion.mode).is_none() { + if section.enabled && section.sinks.is_empty() { push_policy_diag( diagnostics, policy.unsupported_value, "observability.unsupported_value", Some("atof".to_string()), - Some("mode".to_string()), - "ATOF mode must be 'append' or 'overwrite'".to_string(), + Some("sinks".to_string()), + "ATOF requires at least one configured sink when enabled".to_string(), ); } - for (index, endpoint) in section.endpoints.iter().enumerate() { - validate_atof_endpoint_values(diagnostics, policy, index, endpoint); + for (index, sink) in section.sinks.iter().enumerate() { + match sink { + AtofSinkSectionConfig::File(file) => { + if AtofExporterMode::parse(&file.mode).is_none() { + push_policy_diag( + diagnostics, + policy.unsupported_value, + "observability.unsupported_value", + Some("atof".to_string()), + Some(format!("sinks[{index}].mode")), + format!("ATOF sinks[{index}].mode must be 'append' or 'overwrite'"), + ); + } + } + AtofSinkSectionConfig::Stream(stream) => { + validate_atof_stream_sink_values(diagnostics, policy, index, stream); + } + } } } -fn validate_atof_endpoint_values( +fn validate_atof_stream_sink_values( diagnostics: &mut Vec, policy: &ConfigPolicy, index: usize, - endpoint: &AtofEndpointSectionConfig, + endpoint: &AtofStreamSinkSectionConfig, ) { + let transport = AtofEndpointTransport::parse(&endpoint.transport); if endpoint.url.trim().is_empty() { push_policy_diag( diagnostics, policy.unsupported_value, "observability.unsupported_value", Some("atof".to_string()), - Some(format!("endpoints[{index}].url")), - format!("ATOF endpoints[{index}].url must be non-empty"), + Some(format!("sinks[{index}].url")), + format!("ATOF sinks[{index}].url must be non-empty"), ); - } else if !is_valid_atof_endpoint_url(&endpoint.url) { + } else if transport.is_some_and(|transport| !is_valid_atof_stream_url(&endpoint.url, transport)) + { push_policy_diag( diagnostics, policy.unsupported_value, "observability.unsupported_value", Some("atof".to_string()), - Some(format!("endpoints[{index}].url")), - format!("ATOF endpoints[{index}].url must be a valid URL"), + Some(format!("sinks[{index}].url")), + format!( + "ATOF sinks[{index}].url must be a valid URL for transport {:?}", + endpoint.transport + ), ); } - if AtofEndpointTransport::parse(&endpoint.transport).is_none() { + if transport.is_none() { push_policy_diag( diagnostics, policy.unsupported_value, "observability.unsupported_value", Some("atof".to_string()), - Some(format!("endpoints[{index}].transport")), - format!( - "ATOF endpoints[{index}].transport must be 'http_post', 'websocket', or 'ndjson'" - ), + Some(format!("sinks[{index}].transport")), + format!("ATOF sinks[{index}].transport must be 'http_post', 'websocket', or 'ndjson'"), ); } if endpoint.timeout_millis == 0 { @@ -1838,8 +1969,8 @@ fn validate_atof_endpoint_values( policy.unsupported_value, "observability.unsupported_value", Some("atof".to_string()), - Some(format!("endpoints[{index}].timeout_millis")), - format!("ATOF endpoints[{index}].timeout_millis must be greater than 0"), + Some(format!("sinks[{index}].timeout_millis")), + format!("ATOF sinks[{index}].timeout_millis must be greater than 0"), ); } if AtofEndpointFieldNamePolicy::parse(&endpoint.field_name_policy).is_none() { @@ -1848,22 +1979,160 @@ fn validate_atof_endpoint_values( policy.unsupported_value, "observability.unsupported_value", Some("atof".to_string()), - Some(format!("endpoints[{index}].field_name_policy")), - format!( - "ATOF endpoints[{index}].field_name_policy must be 'preserve' or 'replace_dots'" - ), + Some(format!("sinks[{index}].field_name_policy")), + format!("ATOF sinks[{index}].field_name_policy must be 'preserve' or 'replace_dots'"), ); } + for (header, value) in &endpoint.headers { + validate_atof_stream_header( + diagnostics, + policy, + &format!("sinks[{index}].headers.{header}"), + header, + value, + ); + } + for (header, variable) in &endpoint.header_env { + let field = format!("sinks[{index}].header_env.{header}"); + validate_atof_stream_header_name(diagnostics, policy, &field, header); + if endpoint + .headers + .keys() + .any(|configured| configured.eq_ignore_ascii_case(header)) + { + push_policy_diag( + diagnostics, + policy.unsupported_value, + "observability.unsupported_value", + Some("atof".to_string()), + Some(field.clone()), + format!( + "ATOF sinks[{index}] header {header:?} cannot appear in both headers and header_env" + ), + ); + } + validate_atof_stream_header_env(diagnostics, policy, &field, variable); + } } #[cfg(feature = "atof-streaming")] -fn is_valid_atof_endpoint_url(url: &str) -> bool { - reqwest::Url::parse(url).is_ok() +fn is_valid_atof_stream_url(url: &str, transport: AtofEndpointTransport) -> bool { + let Ok(url) = reqwest::Url::parse(url) else { + return false; + }; + url.host_str().is_some() + && match transport { + AtofEndpointTransport::HttpPost | AtofEndpointTransport::Ndjson => { + matches!(url.scheme(), "http" | "https") + } + AtofEndpointTransport::Websocket => matches!(url.scheme(), "ws" | "wss"), + } } #[cfg(not(feature = "atof-streaming"))] -fn is_valid_atof_endpoint_url(_url: &str) -> bool { - true +fn is_valid_atof_stream_url(url: &str, transport: AtofEndpointTransport) -> bool { + let Some((scheme, rest)) = url.split_once("://") else { + return false; + }; + !rest.is_empty() + && !rest.starts_with('/') + && match transport { + AtofEndpointTransport::HttpPost | AtofEndpointTransport::Ndjson => { + matches!(scheme, "http" | "https") + } + AtofEndpointTransport::Websocket => matches!(scheme, "ws" | "wss"), + } +} + +fn validate_atof_stream_header( + diagnostics: &mut Vec, + policy: &ConfigPolicy, + field: &str, + header: &str, + value: &str, +) { + validate_atof_stream_header_name(diagnostics, policy, field, header); + #[cfg(feature = "atof-streaming")] + if let Err(error) = reqwest::header::HeaderValue::from_str(value) { + push_policy_diag( + diagnostics, + policy.unsupported_value, + "observability.unsupported_value", + Some("atof".to_string()), + Some(field.to_string()), + format!("ATOF {field} value is invalid: {error}"), + ); + } +} + +fn validate_atof_stream_header_name( + diagnostics: &mut Vec, + policy: &ConfigPolicy, + field: &str, + header: &str, +) { + #[cfg(feature = "atof-streaming")] + let is_valid = reqwest::header::HeaderName::from_bytes(header.as_bytes()).is_ok(); + #[cfg(not(feature = "atof-streaming"))] + let is_valid = !header.trim().is_empty() && header.trim() == header; + if !is_valid { + push_policy_diag( + diagnostics, + policy.unsupported_value, + "observability.unsupported_value", + Some("atof".to_string()), + Some(field.to_string()), + format!("ATOF {field} header name '{header}' is invalid"), + ); + } +} + +fn validate_atof_stream_header_env( + diagnostics: &mut Vec, + policy: &ConfigPolicy, + field: &str, + variable: &str, +) { + let trimmed = variable.trim(); + if trimmed.is_empty() { + push_policy_diag( + diagnostics, + policy.unsupported_value, + "observability.unsupported_value", + Some("atof".to_string()), + Some(field.to_string()), + format!("ATOF {field} must name a non-empty environment variable"), + ); + } else if trimmed != variable { + push_policy_diag( + diagnostics, + policy.unsupported_value, + "observability.unsupported_value", + Some("atof".to_string()), + Some(field.to_string()), + format!("ATOF {field} must not have surrounding whitespace; got '{variable}'"), + ); + } else { + match std::env::var(variable) { + Ok(value) if value.trim().is_empty() => push_policy_diag( + diagnostics, + policy.unsupported_value, + "observability.unsupported_value", + Some("atof".to_string()), + Some(field.to_string()), + format!("ATOF {field} references an environment variable that is blank"), + ), + Ok(_) => {} + Err(error) => push_policy_diag( + diagnostics, + policy.unsupported_value, + "observability.unsupported_value", + Some("atof".to_string()), + Some(field.to_string()), + format!("ATOF {field} references an environment variable that is not set: {error}"), + ), + } + } } fn validate_atif_values( @@ -2171,7 +2440,7 @@ fn observability_registration_error(error: impl std::fmt::Display) -> PluginErro } fn default_observability_config_version() -> u32 { - 1 + 2 } fn default_atof_mode() -> String { diff --git a/crates/core/tests/unit/observability/atof_tests.rs b/crates/core/tests/unit/observability/atof_tests.rs index e89d34a68..4d5dc84f9 100644 --- a/crates/core/tests/unit/observability/atof_tests.rs +++ b/crates/core/tests/unit/observability/atof_tests.rs @@ -351,15 +351,18 @@ fn start_websocket_capture_server( #[test] fn default_config_uses_cwd_append_and_timestamped_filename() { let config = AtofExporterConfig::default(); + let AtofSinkConfig::File(file) = config.sink else { + panic!("default ATOF config must use a file sink"); + }; - assert_eq!(config.output_directory, std::env::current_dir().unwrap()); - assert_eq!(config.mode, AtofExporterMode::Append); + assert_eq!(file.output_directory, std::env::current_dir().unwrap()); + assert_eq!(file.mode, AtofExporterMode::Append); assert_eq!(AtofExporterMode::Append.as_str(), "append"); assert_eq!(AtofExporterMode::Overwrite.as_str(), "overwrite"); - assert!(config.filename.starts_with("nemo-relay-events-")); - assert!(config.filename.ends_with(".jsonl")); + assert!(file.filename.starts_with("nemo-relay-events-")); + assert!(file.filename.ends_with(".jsonl")); assert_eq!( - config.filename.len(), + file.filename.len(), "nemo-relay-events-YYYY-MM-DD-HH.MM.SS.jsonl".len() ); } @@ -376,7 +379,7 @@ fn endpoint_and_exporter_config_builders_preserve_values() { .with_output_directory(&dir) .with_mode(AtofExporterMode::Overwrite) .with_filename("custom.jsonl") - .with_endpoints(vec![endpoint.clone()]); + .with_stream_sink(endpoint.clone()); assert_eq!( endpoint.headers.get("x-test").map(String::as_str), @@ -392,8 +395,8 @@ fn endpoint_and_exporter_config_builders_preserve_values() { AtofEndpointFieldNamePolicy::parse("replace_dots"), Some(AtofEndpointFieldNamePolicy::ReplaceDots) ); - assert_eq!(config.path(), dir.join("custom.jsonl")); - assert_eq!(config.endpoints, vec![endpoint]); + assert_eq!(config.path(), None); + assert_eq!(config.sink, AtofSinkConfig::Stream(endpoint)); } #[test] @@ -514,7 +517,7 @@ fn subscriber_writes_scope_and_mark_events_as_raw_jsonl() { subscriber(&make_mark_event("checkpoint")); exporter.force_flush().unwrap(); - let lines = read_jsonl(exporter.path()); + let lines = read_jsonl(exporter.path().expect("file sink path")); assert_eq!(lines.len(), 2); assert_eq!(lines[0]["kind"], "scope"); assert_eq!(lines[0]["scope_category"], "start"); @@ -539,7 +542,7 @@ fn shutdown_is_idempotent_and_subscriber_noops_after_close() { subscriber(&make_mark_event("after-close")); exporter.shutdown().unwrap(); - let lines = read_jsonl(exporter.path()); + let lines = read_jsonl(exporter.path().expect("file sink path")); assert_eq!(lines.len(), 1); assert_eq!(lines[0]["name"], "before-close"); } @@ -558,7 +561,7 @@ fn subscriber_writes_canonical_event_jsonl() { (exporter.subscriber())(&event); exporter.force_flush().unwrap(); - let lines = read_jsonl(exporter.path()); + let lines = read_jsonl(exporter.path().expect("file sink path")); assert_eq!(lines.len(), 1); assert_eq!(lines[0], event.try_to_json_value().unwrap()); assert!(lines[0].get("annotated_request").is_none()); @@ -570,9 +573,9 @@ fn subscriber_writes_canonical_event_jsonl() { #[test] #[cfg(feature = "atof-streaming")] -fn streaming_endpoints_receive_raw_atof_events_and_file_output_remains() { +fn streaming_sink_receives_raw_atof_events() { let dir = temp_dir("atof-streaming-http"); - let (url, captures) = start_http_capture_server(4); + let (url, captures) = start_http_capture_server(3); let exporter = AtofExporter::new( AtofExporterConfig::new() .with_output_directory(&dir) @@ -580,8 +583,7 @@ fn streaming_endpoints_receive_raw_atof_events_and_file_output_remains() { .with_endpoint(AtofEndpointConfig::new( url.clone(), AtofEndpointTransport::HttpPost, - )) - .with_endpoint(AtofEndpointConfig::new(url, AtofEndpointTransport::Ndjson)), + )), ) .unwrap(); let subscriber = exporter.subscriber(); @@ -592,14 +594,9 @@ fn streaming_endpoints_receive_raw_atof_events_and_file_output_remains() { subscriber(&make_mark_event("after-flush")); exporter.shutdown().unwrap(); - let lines = read_jsonl(exporter.path()); - assert_eq!(lines.len(), 3); - assert_eq!(lines[0]["name"], "first"); - assert_eq!(lines[1]["name"], "second"); - assert_eq!(lines[2]["name"], "after-flush"); - - let bodies = wait_for_captures(&captures, 4); - assert_eq!(bodies.len(), 4, "captured bodies: {bodies:?}"); + assert_eq!(exporter.path(), None); + let bodies = wait_for_captures(&captures, 3); + assert_eq!(bodies.len(), 3, "captured bodies: {bodies:?}"); let all_streamed = bodies.join(""); assert!(all_streamed.contains("\"name\":\"first\"")); assert!(all_streamed.contains("\"name\":\"second\"")); @@ -609,7 +606,7 @@ fn streaming_endpoints_receive_raw_atof_events_and_file_output_remains() { .lines() .filter(|line| line.contains("\"kind\":\"mark\"")) .count(), - 6, + 3, "three HTTP POST records plus three NDJSON records: {bodies:?}" ); } @@ -819,7 +816,7 @@ fn subscriber_preserves_wire_format_llm_lifecycle_payloads_as_raw_jsonl() { } exporter.force_flush().unwrap(); - let lines = read_jsonl(exporter.path()); + let lines = read_jsonl(exporter.path().expect("file sink path")); assert_eq!(lines.len(), events.len()); for (line, event) in lines.iter().zip(events.iter()) { assert_eq!(line, &event.try_to_json_value().unwrap()); @@ -941,7 +938,7 @@ fn openclaw_subagent_events_preserve_nested_and_fallback_parent_uuid() { } exporter.force_flush().unwrap(); - let lines = read_jsonl(exporter.path()); + let lines = read_jsonl(exporter.path().expect("file sink path")); let nested_start = lines .iter() .find(|line| { @@ -1024,7 +1021,7 @@ fn subscriber_preserves_openclaw_placeholder_replay_payloads_as_raw_jsonl() { } exporter.force_flush().unwrap(); - let lines = read_jsonl(exporter.path()); + let lines = read_jsonl(exporter.path().expect("file sink path")); assert_eq!(lines.len(), events.len()); for (line, event) in lines.iter().zip(events.iter()) { assert_eq!(line, &event.try_to_json_value().unwrap()); @@ -1088,7 +1085,7 @@ fn subscriber_preserves_openclaw_model_timing_marks_as_raw_jsonl() { } exporter.force_flush().unwrap(); - let lines = read_jsonl(exporter.path()); + let lines = read_jsonl(exporter.path().expect("file sink path")); assert_eq!(lines.len(), events.len()); for (line, event) in lines.iter().zip(events.iter()) { assert_eq!(line, &event.try_to_json_value().unwrap()); @@ -1185,7 +1182,7 @@ fn subscriber_preserves_openclaw_hook_only_fallback_payloads_as_raw_jsonl() { } exporter.force_flush().unwrap(); - let lines = read_jsonl(exporter.path()); + let lines = read_jsonl(exporter.path().expect("file sink path")); assert_eq!(lines.len(), events.len()); for (line, event) in lines.iter().zip(events.iter()) { assert_eq!(line, &event.try_to_json_value().unwrap()); @@ -1246,7 +1243,7 @@ fn register_deregister_flush_and_shutdown_work_with_runtime_events() { exporter.shutdown().unwrap(); exporter.shutdown().unwrap(); - let lines = read_jsonl(exporter.path()); + let lines = read_jsonl(exporter.path().expect("file sink path")); assert_eq!(lines.len(), 3); assert_eq!(lines[0]["name"], "atof_scope"); assert_eq!(lines[1]["name"], "atof_mark"); @@ -1284,7 +1281,7 @@ fn missing_output_directory_is_created() { .unwrap(); let output_path = output_dir.join("events.jsonl"); - assert_eq!(exporter.path(), output_path.as_path()); + assert_eq!(exporter.path(), Some(output_path.as_path())); assert!(output_dir.is_dir()); assert!(output_path.exists()); } @@ -1669,7 +1666,7 @@ fn force_flush_keeps_exporter_open_and_shutdown_is_terminal() { subscriber(&make_mark_event("after_shutdown")); exporter.shutdown().unwrap(); - let lines = read_jsonl(exporter.path()); + let lines = read_jsonl(exporter.path().expect("file sink path")); assert_eq!(lines.len(), 2); assert_eq!(lines[0]["name"], "before_flush"); assert_eq!(lines[1]["name"], "after_flush"); diff --git a/crates/core/tests/unit/observability/plugin_component_tests.rs b/crates/core/tests/unit/observability/plugin_component_tests.rs index 318b1b417..56a790c10 100644 --- a/crates/core/tests/unit/observability/plugin_component_tests.rs +++ b/crates/core/tests/unit/observability/plugin_component_tests.rs @@ -18,6 +18,12 @@ use crate::plugin::{ }; use serde_json::json; use std::fs; +#[cfg(feature = "atof-streaming")] +use std::io::{Read, Write}; +#[cfg(feature = "atof-streaming")] +use std::net::TcpListener; +#[cfg(feature = "atof-streaming")] +use std::sync::{Arc, Mutex}; use std::time::{SystemTime, UNIX_EPOCH}; fn temp_dir(prefix: &str) -> PathBuf { @@ -30,6 +36,59 @@ fn temp_dir(prefix: &str) -> PathBuf { path } +#[cfg(feature = "atof-streaming")] +fn start_http_capture_server(expected_requests: usize) -> (String, Arc>>) { + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let url = format!("http://{}", listener.local_addr().unwrap()); + let captures = Arc::new(Mutex::new(Vec::new())); + let thread_captures = Arc::clone(&captures); + std::thread::spawn(move || { + for _ in 0..expected_requests { + let (mut stream, _) = listener.accept().unwrap(); + let mut request = Vec::new(); + let mut byte = [0_u8; 1]; + while !request.ends_with(b"\r\n\r\n") { + stream.read_exact(&mut byte).unwrap(); + request.push(byte[0]); + } + let headers = String::from_utf8_lossy(&request); + let length = headers + .lines() + .find_map(|line| { + line.split_once(':').and_then(|(name, value)| { + name.eq_ignore_ascii_case("content-length") + .then_some(value.trim()) + }) + }) + .unwrap() + .parse::() + .unwrap(); + let mut body = vec![0_u8; length]; + stream.read_exact(&mut body).unwrap(); + thread_captures + .lock() + .unwrap() + .push(String::from_utf8(body).unwrap()); + stream + .write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 0\r\nConnection: close\r\n\r\n") + .unwrap(); + } + }); + (url, captures) +} + +#[cfg(feature = "atof-streaming")] +fn wait_for_captures(captures: &Arc>>, expected: usize) -> Vec { + for _ in 0..100 { + let snapshot = captures.lock().unwrap().clone(); + if snapshot.len() >= expected { + return snapshot; + } + std::thread::sleep(std::time::Duration::from_millis(20)); + } + captures.lock().unwrap().clone() +} + fn reset_runtime() { let _ = clear_plugin_configuration(); crate::shared_runtime::reset_runtime_owner_for_tests(); @@ -65,14 +124,18 @@ fn editor_schema_tracks_observability_config_types() { assert!(atof.optional); let atof_schema = atof.schema().expect("atof editor schema"); - let mode = atof_schema.field("mode").expect("atof mode field"); - assert_eq!(mode.kind, EditorFieldKind::Enum); - assert_eq!(mode.enum_values, &["append", "overwrite"]); - let endpoints = atof_schema - .field("endpoints") - .expect("atof endpoints field"); - assert_eq!(endpoints.kind, EditorFieldKind::Json); - assert!(endpoints.optional); + let sinks = atof_schema.field("sinks").expect("atof sinks field"); + assert_eq!(sinks.kind, EditorFieldKind::List); + let sink = sinks.list_item.expect("ATOF sink list metadata"); + assert_eq!(sink.kind, EditorFieldKind::Section); + assert_eq!( + sink.tagged_union.map(|metadata| metadata.discriminator), + Some("type") + ); + assert_eq!( + sink.tagged_union.expect("sink tagged union").variants.len(), + 2 + ); let otlp = schema .field("openinference") @@ -192,7 +255,7 @@ fn default_config_and_component_conversion_cover_public_shape() { reset_runtime(); let defaults = ObservabilityConfig::default(); - assert_eq!(defaults.version, 1); + assert_eq!(defaults.version, 2); assert!(defaults.atof.is_none()); assert!(defaults.atif.is_none()); assert!(defaults.opentelemetry.is_none()); @@ -200,16 +263,17 @@ fn default_config_and_component_conversion_cover_public_shape() { let atof = AtofSectionConfig::default(); assert!(!atof.enabled); - assert_eq!(atof.mode, "append"); - assert!(atof.output_directory.is_none()); - assert!(atof.filename.is_none()); + assert!(atof.sinks.is_empty()); let parsed_atof: AtofSectionConfig = serde_json::from_value(json!({ - "endpoints": [{"url": "http://localhost/events"}] + "sinks": [{"type": "stream", "url": "http://localhost/events"}] })) .unwrap(); - assert_eq!(parsed_atof.endpoints[0].transport, "http_post"); - assert_eq!(parsed_atof.endpoints[0].field_name_policy, "preserve"); + let AtofSinkSectionConfig::Stream(stream) = &parsed_atof.sinks[0] else { + panic!("expected stream sink"); + }; + assert_eq!(stream.transport, "http_post"); + assert_eq!(stream.field_name_policy, "preserve"); let atif = AtifSectionConfig::default(); assert!(!atif.enabled); @@ -236,7 +300,7 @@ fn default_config_and_component_conversion_cover_public_shape() { .into(); assert_eq!(generic.kind, OBSERVABILITY_PLUGIN_KIND); assert!(generic.enabled); - assert_eq!(generic.config["version"], json!(1)); + assert_eq!(generic.config["version"], json!(2)); assert_eq!(generic.config["atif"]["agent_name"], json!("NeMo Relay")); } @@ -304,7 +368,10 @@ fn schema_contains_every_supported_observability_option() { "output_directory", "filename", "mode", - "endpoints", + "sinks", + "type", + "url", + "field_name_policy", "agent_name", "agent_version", "model_name", @@ -408,7 +475,7 @@ fn empty_and_disabled_config_register_nothing() { reset_runtime(); let config = plugin_config(json!({ - "atof": {"enabled": false, "mode": "overwrite"}, + "atof": {"enabled": false}, "atif": {"enabled": false}, "opentelemetry": {"enabled": false, "transport": "grpc"}, "openinference": {"enabled": false, "transport": "grpc"} @@ -429,8 +496,7 @@ fn disabled_file_sections_do_not_create_files() { let config = plugin_config(json!({ "atof": { "enabled": false, - "output_directory": dir, - "filename": "events.jsonl" + "sinks": [{"type": "file", "output_directory": dir, "filename": "events.jsonl"}] }, "atif": { "enabled": false, @@ -475,7 +541,7 @@ fn unknown_fields_and_bad_values_follow_policy() { reset_runtime(); let warn_report = validate_plugin_config(&plugin_config(json!({ - "atof": {"bogus": true, "mode": "invalid"}, + "atof": {"bogus": true, "sinks": [{"type": "file", "mode": "invalid"}]}, "atif": {"filename_template": "missing-session"} }))); assert!(warn_report.has_errors()); @@ -489,7 +555,7 @@ fn unknown_fields_and_bad_values_follow_policy() { warn_report .diagnostics .iter() - .any(|diag| diag.field.as_deref() == Some("mode")) + .any(|diag| diag.field.as_deref() == Some("sinks[0].mode")) ); assert!( warn_report @@ -500,7 +566,7 @@ fn unknown_fields_and_bad_values_follow_policy() { let ignore_report = validate_plugin_config(&plugin_config(json!({ "policy": {"unknown_field": "ignore", "unsupported_value": "ignore"}, - "atof": {"bogus": true, "mode": "invalid"}, + "atof": {"bogus": true, "sinks": [{"type": "file", "mode": "invalid"}]}, "atif": {"filename_template": "missing-session"} }))); assert!(!ignore_report.has_errors()); @@ -524,7 +590,7 @@ fn invalid_shapes_and_strict_policy_are_reported() { ); let unsupported_version = validate_plugin_config(&plugin_config(json!({ - "version": 2, + "version": 1, }))); assert!(unsupported_version.has_errors()); assert!(unsupported_version.diagnostics.iter().any(|diag| diag.code @@ -565,12 +631,14 @@ fn atof_endpoint_validation_rejects_bad_values() { let report = validate_plugin_config(&plugin_config(json!({ "atof": { "enabled": true, - "endpoints": [ - {"url": "", "transport": "http_post"}, - {"url": "http://localhost/events", "transport": "bogus"}, - {"url": "http://localhost/events", "transport": "ndjson", "timeout_millis": 0}, - {"url": "not a url", "transport": "http_post"}, - {"url": "http://localhost/events", "transport": "http_post", "field_name_policy": "bogus"} + "sinks": [ + {"type": "stream", "url": "", "transport": "http_post"}, + {"type": "stream", "url": "http://localhost/events", "transport": "bogus"}, + {"type": "stream", "url": "http://localhost/events", "transport": "ndjson", "timeout_millis": 0}, + {"type": "stream", "url": "not a url", "transport": "http_post"}, + {"type": "stream", "url": "http://localhost/events", "transport": "http_post", "field_name_policy": "bogus"}, + {"type": "stream", "url": "http://localhost/events", "transport": "websocket"}, + {"type": "stream", "url": "http://localhost/events", "headers": {"invalid header": "value", "x-api-key": "value"}, "header_env": {"X-Api-Key": "NEMO_RELAY_TEST_MISSING_ATOF_HEADER_ENV"}} ] } }))); @@ -580,42 +648,61 @@ fn atof_endpoint_validation_rejects_bad_values() { report .diagnostics .iter() - .any(|diag| { diag.field.as_deref() == Some("endpoints[0].url") }) + .any(|diag| { diag.field.as_deref() == Some("sinks[0].url") }) ); assert!( report .diagnostics .iter() - .any(|diag| { diag.field.as_deref() == Some("endpoints[1].transport") }) + .any(|diag| { diag.field.as_deref() == Some("sinks[1].transport") }) ); assert!( report .diagnostics .iter() - .any(|diag| { diag.field.as_deref() == Some("endpoints[2].timeout_millis") }) + .any(|diag| { diag.field.as_deref() == Some("sinks[2].timeout_millis") }) ); #[cfg(feature = "atof-streaming")] assert!( report .diagnostics .iter() - .any(|diag| { diag.field.as_deref() == Some("endpoints[3].url") }) + .any(|diag| { diag.field.as_deref() == Some("sinks[3].url") }) ); assert!( report .diagnostics .iter() - .any(|diag| { diag.field.as_deref() == Some("endpoints[4].field_name_policy") }) + .any(|diag| { diag.field.as_deref() == Some("sinks[4].field_name_policy") }) + ); + assert!( + report + .diagnostics + .iter() + .any(|diag| { diag.field.as_deref() == Some("sinks[5].url") }) + ); + assert!( + report + .diagnostics + .iter() + .any(|diag| { diag.field.as_deref() == Some("sinks[6].header_env.X-Api-Key") }) + ); + #[cfg(feature = "atof-streaming")] + assert!( + report + .diagnostics + .iter() + .any(|diag| { diag.field.as_deref() == Some("sinks[6].headers.invalid header") }) ); } #[test] -fn build_atof_endpoint_config_maps_headers_timeout_and_rejects_transport() { +fn build_atof_sink_config_maps_headers_timeout_and_rejects_transport() { let mut headers = std::collections::HashMap::new(); headers.insert("authorization".to_string(), "token".to_string()); - let config = build_atof_endpoint_config( + let config = build_atof_sink_config( 2, - AtofEndpointSectionConfig { + AtofSinkSectionConfig::Stream(AtofStreamSinkSectionConfig { url: "ws://127.0.0.1:47632/events".into(), transport: "websocket".into(), headers: headers.clone(), @@ -625,10 +712,13 @@ fn build_atof_endpoint_config_maps_headers_timeout_and_rejects_transport() { )]), timeout_millis: 123, field_name_policy: "replace_dots".into(), - }, + }), ) .unwrap(); + let CoreAtofSinkConfig::Stream(config) = config else { + panic!("expected stream sink") + }; assert_eq!(config.url, "ws://127.0.0.1:47632/events"); assert_eq!( config.transport, @@ -645,33 +735,33 @@ fn build_atof_endpoint_config_maps_headers_timeout_and_rejects_transport() { crate::observability::atof::AtofEndpointFieldNamePolicy::ReplaceDots ); - let error = build_atof_endpoint_config( + let error = build_atof_sink_config( 3, - AtofEndpointSectionConfig { + AtofSinkSectionConfig::Stream(AtofStreamSinkSectionConfig { url: "http://127.0.0.1:47632/events".into(), transport: "smtp".into(), headers: std::collections::HashMap::new(), header_env: std::collections::HashMap::new(), timeout_millis: 3_000, field_name_policy: "preserve".into(), - }, + }), ) .unwrap_err(); - assert!(error.to_string().contains("endpoints[3].transport")); + assert!(error.to_string().contains("sinks[3].transport")); - let error = build_atof_endpoint_config( + let error = build_atof_sink_config( 4, - AtofEndpointSectionConfig { + AtofSinkSectionConfig::Stream(AtofStreamSinkSectionConfig { url: "http://127.0.0.1:47632/events".into(), transport: "http_post".into(), headers: std::collections::HashMap::new(), header_env: std::collections::HashMap::new(), timeout_millis: 3_000, field_name_policy: "bogus".into(), - }, + }), ) .unwrap_err(); - assert!(error.to_string().contains("endpoints[4].field_name_policy")); + assert!(error.to_string().contains("sinks[4].field_name_policy")); } #[test] @@ -686,13 +776,11 @@ fn initialization_fails_for_invalid_enabled_file_exporters() { "policy": {"unsupported_value": "ignore"}, "atof": { "enabled": true, - "mode": "invalid", - "output_directory": dir, - "filename": "events.jsonl" + "sinks": [{"type": "file", "mode": "invalid", "output_directory": dir, "filename": "events.jsonl"}] } })); let error = futures::executor::block_on(initialize_plugins_exact(invalid_atof)).unwrap_err(); - assert!(error.to_string().contains("ATOF mode")); + assert!(error.to_string().contains("ATOF sinks[0].mode")); let invalid_atif_template = plugin_config(json!({ "policy": {"unsupported_value": "ignore"}, @@ -709,8 +797,7 @@ fn initialization_fails_for_invalid_enabled_file_exporters() { let invalid_path = plugin_config(json!({ "atof": { "enabled": true, - "output_directory": not_a_directory, - "filename": "events.jsonl" + "sinks": [{"type": "file", "output_directory": not_a_directory, "filename": "events.jsonl"}] } })); let error = futures::executor::block_on(initialize_plugins_exact(invalid_path)).unwrap_err(); @@ -749,9 +836,10 @@ fn atof_enabled_writes_jsonl_and_teardown_flushes() { let config = plugin_config(json!({ "atof": { "enabled": true, - "output_directory": dir, - "filename": "events.jsonl", - "mode": "overwrite" + "sinks": [ + {"type": "file", "output_directory": dir, "filename": "events.jsonl", "mode": "overwrite"}, + {"type": "file", "output_directory": dir, "filename": "events-copy.jsonl", "mode": "overwrite"} + ] } })); futures::executor::block_on(initialize_plugins_exact(config)).unwrap(); @@ -786,6 +874,54 @@ fn atof_enabled_writes_jsonl_and_teardown_flushes() { assert!(lines[0].contains("\"kind\":\"scope\"")); assert!(lines[1].contains("\"kind\":\"mark\"")); assert!(lines[2].contains("\"scope_category\":\"end\"")); + assert_eq!( + fs::read_to_string(dir.join("events-copy.jsonl")) + .unwrap() + .lines() + .count(), + 3 + ); +} + +#[test] +#[cfg(feature = "atof-streaming")] +fn atof_stream_sinks_fan_out_and_teardown_all_workers() { + let _guard = crate::observability::test_mutex().lock().unwrap(); + reset_runtime(); + let (first_url, first_captures) = start_http_capture_server(3); + let (second_url, second_captures) = start_http_capture_server(3); + + let config = plugin_config(json!({ + "atof": { + "enabled": true, + "sinks": [ + {"type": "stream", "url": first_url, "transport": "http_post"}, + {"type": "stream", "url": second_url, "transport": "http_post"} + ] + } + })); + futures::executor::block_on(initialize_plugins_exact(config)).unwrap(); + + let agent = push_agent("atof-stream-agent"); + crate::api::scope::event( + crate::api::scope::EmitMarkEventParams::builder() + .name("checkpoint") + .parent(&agent) + .data(json!({"step": 1})) + .build(), + ) + .unwrap(); + pop(&agent); + clear_plugin_configuration().unwrap(); + + for captures in [&first_captures, &second_captures] { + let bodies = wait_for_captures(captures, 3); + assert_eq!(bodies.len(), 3, "captured bodies: {bodies:?}"); + let events = bodies.join(""); + assert!(events.contains("\"scope_category\":\"start\"")); + assert!(events.contains("\"name\":\"checkpoint\"")); + assert!(events.contains("\"scope_category\":\"end\"")); + } } #[test] diff --git a/crates/ffi/nemo_relay.h b/crates/ffi/nemo_relay.h index 6355d579c..2229ac0df 100644 --- a/crates/ffi/nemo_relay.h +++ b/crates/ffi/nemo_relay.h @@ -1293,7 +1293,7 @@ NemoRelayStatus nemo_relay_atof_exporter_force_flush(const struct FfiAtofExporte NemoRelayStatus nemo_relay_atof_exporter_shutdown(const struct FfiAtofExporter *exporter); /** - * Returns the ATOF exporter output path as a string. + * Returns the ATOF exporter output path as a string when its sink is a file. * * # Safety * `exporter` and `out` must be valid, non-null pointers. diff --git a/crates/ffi/src/api/observability.rs b/crates/ffi/src/api/observability.rs index 2713c394e..61ba1eb50 100644 --- a/crates/ffi/src/api/observability.rs +++ b/crates/ffi/src/api/observability.rs @@ -468,7 +468,7 @@ pub unsafe extern "C" fn nemo_relay_atof_exporter_shutdown( } } -/// Returns the ATOF exporter output path as a string. +/// Returns the ATOF exporter output path as a string when its sink is a file. /// /// # Safety /// `exporter` and `out` must be valid, non-null pointers. @@ -486,7 +486,11 @@ pub unsafe extern "C" fn nemo_relay_atof_exporter_path( set_last_error("out pointer is null"); return NemoRelayStatus::NullPointer; } - let path = unsafe { &*exporter }.0.path().to_string_lossy(); + let Some(path) = unsafe { &*exporter }.0.path() else { + unsafe { *out = std::ptr::null_mut() }; + return NemoRelayStatus::Ok; + }; + let path = path.to_string_lossy(); unsafe { *out = str_to_c_string(&path) }; NemoRelayStatus::Ok } diff --git a/crates/ffi/tests/integration/api/coverage_sweeps_tests.rs b/crates/ffi/tests/integration/api/coverage_sweeps_tests.rs index ad730dfd1..a7794ed3d 100644 --- a/crates/ffi/tests/integration/api/coverage_sweeps_tests.rs +++ b/crates/ffi/tests/integration/api/coverage_sweeps_tests.rs @@ -1083,7 +1083,7 @@ fn test_ffi_adaptive_and_observability_entry_points_from_integration_binary() { nemo_relay_observability_default_config_json(&mut out_json), NemoRelayStatus::Ok ); - assert_eq!(returned_json(out_json)["version"], json!(1)); + assert_eq!(returned_json(out_json)["version"], json!(2)); assert_eq!( nemo_relay_observability_default_config_json(ptr::null_mut()), NemoRelayStatus::NullPointer diff --git a/crates/ffi/tests/integration/api_tests.rs b/crates/ffi/tests/integration/api_tests.rs index 8d9e11b8d..5fd7ebde3 100644 --- a/crates/ffi/tests/integration/api_tests.rs +++ b/crates/ffi/tests/integration/api_tests.rs @@ -809,7 +809,7 @@ fn atof_exporter_create_from_json_reports_string_statuses() { ); let invalid_endpoint = - cstring(r#"{"endpoints":[{"url":"http://localhost/events","transport":"websocket"}]}"#); + cstring(r#"{"type":"stream","url":"http://localhost/events","transport":"websocket"}"#); assert_eq!( unsafe { api::nemo_relay_atof_exporter_create_from_json(invalid_endpoint.as_ptr(), &mut exporter) diff --git a/crates/ffi/tests/unit/api/core_tests.rs b/crates/ffi/tests/unit/api/core_tests.rs index 4977e0540..35e1907b1 100644 --- a/crates/ffi/tests/unit/api/core_tests.rs +++ b/crates/ffi/tests/unit/api/core_tests.rs @@ -201,12 +201,15 @@ fn test_ffi_observability_plugin_file_sinks() { "kind": "observability", "enabled": true, "config": { - "version": 1, + "version": 2, "atof": { "enabled": true, - "output_directory": dir_text, - "filename": "events.jsonl", - "mode": "overwrite" + "sinks": [{ + "type": "file", + "output_directory": dir_text, + "filename": "events.jsonl", + "mode": "overwrite" + }] }, "atif": { "enabled": true, @@ -235,7 +238,7 @@ fn test_ffi_observability_plugin_file_sinks() { nemo_relay_observability_default_config_json(&mut default_config_json), NemoRelayStatus::Ok ); - assert_eq!(returned_json(default_config_json)["version"], json!(1)); + assert_eq!(returned_json(default_config_json)["version"], json!(2)); let mut component_json = ptr::null_mut(); assert_eq!( nemo_relay_observability_component_spec_json(ptr::null(), true, &mut component_json), @@ -326,7 +329,7 @@ fn test_ffi_observability_plugin_atif_splits_multiple_top_level_agents() { "kind": "observability", "enabled": true, "config": { - "version": 1, + "version": 2, "atif": { "enabled": true, "output_directory": dir_text, diff --git a/crates/node/observability.d.ts b/crates/node/observability.d.ts index eb15be134..d083ae7ed 100644 --- a/crates/node/observability.d.ts +++ b/crates/node/observability.d.ts @@ -8,20 +8,31 @@ export { ConfigPolicy, ConfigDiagnostic, ConfigReport }; export interface AtofConfig { enabled?: boolean; + sinks?: AtofSinkConfig[]; +} + +export type AtofSinkConfig = AtofFileSinkConfig | AtofStreamSinkConfig; + +export interface AtofFileSinkConfig { + type: 'file'; output_directory?: string; filename?: string; mode?: 'append' | 'overwrite' | string; - endpoints?: AtofEndpointConfig[]; } -export interface AtofEndpointConfig { +export interface AtofStreamSinkConfig { + type: 'stream'; url: string; transport?: 'http_post' | 'websocket' | 'ndjson' | string; headers?: Record; + header_env?: Record; timeout_millis?: number; field_name_policy?: 'preserve' | 'replace_dots' | string; } +/** @deprecated Use AtofStreamSinkConfig. */ +export type AtofEndpointConfig = AtofStreamSinkConfig; + export interface S3StorageConfig { type: 's3'; bucket: string; diff --git a/crates/node/observability.js b/crates/node/observability.js index 97e5c8415..72a814c10 100644 --- a/crates/node/observability.js +++ b/crates/node/observability.js @@ -10,16 +10,16 @@ const OBSERVABILITY_PLUGIN_KIND = 'observability'; /** * Create a default observability component config. * - * @returns {object} The minimal observability config with schema version 1. + * @returns {object} The minimal observability config with schema version 2. */ function defaultConfig() { return { - version: 1, + version: 2, }; } /** - * Create filesystem-backed ATOF JSONL settings with defaults applied. + * Create multi-sink ATOF settings with defaults applied. * * @param {object} [config={}] - Partial ATOF settings to override. * @returns {object} A normalized ATOF config object. @@ -27,7 +27,6 @@ function defaultConfig() { function atofConfig(config = {}) { return { enabled: false, - mode: 'append', ...config, }; } diff --git a/crates/node/src/api/mod.rs b/crates/node/src/api/mod.rs index 9f06b649f..f3642127a 100644 --- a/crates/node/src/api/mod.rs +++ b/crates/node/src/api/mod.rs @@ -176,59 +176,67 @@ fn build_atof_config( options: Option, ) -> napi::Result { let options = options.unwrap_or_default(); - let mut config = nemo_relay::observability::atof::AtofExporterConfig::new(); - - if let Some(output_directory) = options.output_directory { - config = config.with_output_directory(PathBuf::from(output_directory)); - } - if let Some(filename) = options.filename { - config = config.with_filename(filename); - } - if let Some(mode) = options.mode { - let Some(mode) = nemo_relay::observability::atof::AtofExporterMode::parse(&mode) else { - return Err(napi::Error::from_reason( - "mode must be 'append' or 'overwrite'", - )); - }; - config = config.with_mode(mode); - } - let mut endpoints = Vec::new(); - for endpoint in options.endpoints.unwrap_or_default() { - let transport = endpoint - .transport - .unwrap_or_else(|| "http_post".to_string()); - let Some(transport) = - nemo_relay::observability::atof::AtofEndpointTransport::parse(&transport) - else { - return Err(napi::Error::from_reason( - "endpoint transport must be 'http_post', 'websocket', or 'ndjson'", - )); - }; - let mut endpoint_config = - nemo_relay::observability::atof::AtofEndpointConfig::new(endpoint.url, transport); - if let Some(timeout_millis) = endpoint.timeout_millis { - endpoint_config = endpoint_config.with_timeout_millis(timeout_millis.into()); + match options.r#type.as_deref().unwrap_or("file") { + "file" => { + let mut config = nemo_relay::observability::atof::AtofExporterConfig::new(); + if let Some(output_directory) = options.output_directory { + config = config.with_output_directory(PathBuf::from(output_directory)); + } + if let Some(filename) = options.filename { + config = config.with_filename(filename); + } + if let Some(mode) = options.mode { + let Some(mode) = nemo_relay::observability::atof::AtofExporterMode::parse(&mode) + else { + return Err(napi::Error::from_reason( + "mode must be 'append' or 'overwrite'", + )); + }; + config = config.with_mode(mode); + } + Ok(config) } - if let Some(field_name_policy) = endpoint.field_name_policy { - let Some(field_name_policy) = - nemo_relay::observability::atof::AtofEndpointFieldNamePolicy::parse( - &field_name_policy, - ) + "stream" => { + let url = options + .url + .ok_or_else(|| napi::Error::from_reason("stream sink requires url"))?; + let transport = options.transport.unwrap_or_else(|| "http_post".to_string()); + let Some(transport) = + nemo_relay::observability::atof::AtofEndpointTransport::parse(&transport) else { return Err(napi::Error::from_reason( - "endpoint field_name_policy must be 'preserve' or 'replace_dots'", + "stream transport must be 'http_post', 'websocket', or 'ndjson'", )); }; - endpoint_config = endpoint_config.with_field_name_policy(field_name_policy); - } - for (key, value) in parse_string_map(endpoint.headers, "endpoint.headers")? { - endpoint_config = endpoint_config.with_header(key, value); + let mut sink = + nemo_relay::observability::atof::AtofStreamSinkConfig::new(url, transport); + if let Some(timeout_millis) = options.timeout_millis { + sink = sink.with_timeout_millis(timeout_millis.into()); + } + if let Some(field_name_policy) = options.field_name_policy { + let Some(field_name_policy) = + nemo_relay::observability::atof::AtofEndpointFieldNamePolicy::parse( + &field_name_policy, + ) + else { + return Err(napi::Error::from_reason( + "stream field_name_policy must be 'preserve' or 'replace_dots'", + )); + }; + sink = sink.with_field_name_policy(field_name_policy); + } + for (key, value) in parse_string_map(options.headers, "headers")? { + sink = sink.with_header(key, value); + } + for (key, variable) in parse_string_map(options.header_env, "headerEnv")? { + sink = sink.with_header_env(key, variable); + } + Ok(nemo_relay::observability::atof::AtofExporterConfig::new().with_stream_sink(sink)) } - endpoints.push(endpoint_config); + _ => Err(napi::Error::from_reason( + "ATOF sink type must be 'file' or 'stream'", + )), } - config = config.with_endpoints(endpoints); - - Ok(config) } fn build_openinference_config( @@ -3341,37 +3349,33 @@ impl AtifExporter { } } -/// Mutable configuration object for `AtofExporter`. +/// One tagged sink configuration for `AtofExporter`. #[napi(object)] #[derive(Default)] pub struct AtofExporterConfig { + /// Sink type: `"file"` (default) or `"stream"`. + pub r#type: Option, /// Output directory. Defaults to the current working directory. pub output_directory: Option, /// `"append"` (default) or `"overwrite"`. pub mode: Option, /// Output filename. Defaults to `nemo-relay-events-YYYY-MM-DD-HH.MM.SS.jsonl`. pub filename: Option, - /// Streaming endpoints that receive every raw ATOF event. - pub endpoints: Option>, -} - -/// Mutable configuration object for one ATOF streaming endpoint. -#[napi(object)] -#[derive(Default)] -pub struct AtofEndpointConfig { - /// Endpoint URL. - pub url: String, + /// Stream endpoint URL. Required when `type` is `"stream"`. + pub url: Option, /// `"http_post"` (default), `"websocket"`, or `"ndjson"`. pub transport: Option, - /// Extra endpoint headers as string key/value pairs. + /// Extra stream headers as string key/value pairs. pub headers: Option, - /// Per-endpoint timeout in milliseconds. + /// Header names mapped to environment variables that supply their values. + pub header_env: Option, + /// Per-stream timeout in milliseconds. pub timeout_millis: Option, - /// Field name policy applied before sending events. + /// Field name policy applied before sending stream events. pub field_name_policy: Option, } -/// Filesystem-backed Agent Trajectory Observability Format (ATOF) JSONL event exporter. +/// Single-sink Agent Trajectory Observability Format (ATOF) exporter. #[napi] pub struct AtofExporter { inner: nemo_relay::observability::atof::AtofExporter, @@ -3388,10 +3392,12 @@ impl AtofExporter { Ok(Self { inner }) } - /// Return the JSONL output path. + /// Return the JSONL output path, or `null` for a stream sink. #[napi(getter)] - pub fn path(&self) -> String { - self.inner.path().to_string_lossy().into_owned() + pub fn path(&self) -> Option { + self.inner + .path() + .map(|path| path.to_string_lossy().into_owned()) } /// Register this exporter globally with the given name. diff --git a/crates/node/tests/atof_tests.mjs b/crates/node/tests/atof_tests.mjs index 00bda759d..ab4ccbf13 100644 --- a/crates/node/tests/atof_tests.mjs +++ b/crates/node/tests/atof_tests.mjs @@ -33,19 +33,42 @@ describe('AtofExporter', () => { assert.throws( () => new AtofExporter({ - outputDirectory: tempDir('node-atof-invalid-endpoint'), - endpoints: [{ url: 'http://localhost:8080/events', transport: 'bogus' }], + type: 'stream', + url: 'http://localhost:8080/events', + transport: 'bogus', }), - /endpoint transport/i, + /stream transport/i, ); assert.throws( () => new AtofExporter({ - outputDirectory: tempDir('node-atof-invalid-field-policy'), - endpoints: [{ url: 'http://localhost:8080/events', fieldNamePolicy: 'bogus' }], + type: 'stream', + url: 'http://localhost:8080/events', + fieldNamePolicy: 'bogus', }), /field_name_policy/i, ); + assert.throws( + () => + new AtofExporter({ + type: 'stream', + url: 'http://localhost:8080/events', + headerEnv: { authorization: '' }, + }), + /environment variable/i, + ); + }); + + it('returns null path for a stream sink', () => { + const exporter = new AtofExporter({ + type: 'stream', + url: 'http://localhost:8080/events', + }); + try { + assert.equal(exporter.path, null); + } finally { + exporter.shutdown(); + } }); it('writes raw ATOF JSONL events and supports lifecycle methods', () => { diff --git a/crates/node/tests/observability_plugin_tests.mjs b/crates/node/tests/observability_plugin_tests.mjs index 94b825031..dd4c63d71 100644 --- a/crates/node/tests/observability_plugin_tests.mjs +++ b/crates/node/tests/observability_plugin_tests.mjs @@ -19,8 +19,8 @@ function tempDir(prefix) { describe('observability plugin helpers', () => { it('builds defaults and plugin component shape', () => { - assert.deepEqual(observability.defaultConfig(), { version: 1 }); - assert.deepEqual(observability.atofConfig(), { enabled: false, mode: 'append' }); + assert.deepEqual(observability.defaultConfig(), { version: 2 }); + assert.deepEqual(observability.atofConfig(), { enabled: false }); assert.deepEqual(observability.atifConfig(), { enabled: false, agent_name: 'NeMo Relay', @@ -39,7 +39,7 @@ describe('observability plugin helpers', () => { }); assert.equal(observability.otlpConfig({ mark_projection: 'tool' }).mark_projection, 'tool'); - const component = observability.ComponentSpec({ version: 1, atof: observability.atofConfig() }); + const component = observability.ComponentSpec({ version: 2, atof: observability.atofConfig() }); assert.equal(component.kind, observability.OBSERVABILITY_PLUGIN_KIND); assert.equal(component.enabled, true); }); @@ -50,19 +50,20 @@ describe('observability plugin helpers', () => { version: 1, components: [ observability.ComponentSpec({ - version: 1, - atof: observability.atofConfig({ mode: 'bad' }), + version: 2, + atof: observability.atofConfig({ sinks: [{ type: 'file', mode: 'bad' }] }), atif: observability.atifConfig({ filename_template: 'missing-placeholder.json' }), }), ], }); - assert.deepEqual(report.diagnostics.map((diagnostic) => diagnostic.field).sort(), ['filename_template', 'mode']); + assert.deepEqual(report.diagnostics.map((diagnostic) => diagnostic.field).sort(), ['filename_template', 'sinks[0].mode']); }); - it('serializes ATOF streaming endpoints', () => { + it('serializes ATOF stream sinks', () => { const config = observability.atofConfig({ - endpoints: [ + sinks: [ { + type: 'stream', url: 'http://localhost:8080/events', transport: 'http_post', headers: { 'X-Test': 'yes' }, @@ -72,8 +73,9 @@ describe('observability plugin helpers', () => { ], }); - assert.deepEqual(config.endpoints, [ + assert.deepEqual(config.sinks, [ { + type: 'stream', url: 'http://localhost:8080/events', transport: 'http_post', headers: { 'X-Test': 'yes' }, @@ -106,12 +108,10 @@ describe('observability plugin helpers', () => { it('activates ATOF and ATIF file sinks', async () => { const outputDirectory = tempDir('node-observability-plugin'); const config = { - version: 1, + version: 2, atof: observability.atofConfig({ enabled: true, - output_directory: outputDirectory, - filename: 'events.jsonl', - mode: 'overwrite', + sinks: [{ type: 'file', output_directory: outputDirectory, filename: 'events.jsonl', mode: 'overwrite' }], }), atif: observability.atifConfig({ enabled: true, @@ -160,7 +160,7 @@ describe('observability plugin helpers', () => { it('splits ATIF files for multiple top-level agent scopes', async () => { const outputDirectory = tempDir('node-observability-plugin-multi-agent'); const config = { - version: 1, + version: 2, atif: observability.atifConfig({ enabled: true, output_directory: outputDirectory, diff --git a/crates/pii-redaction/tests/unit/component_tests.rs b/crates/pii-redaction/tests/unit/component_tests.rs index 8af9b2465..e0fc352d7 100644 --- a/crates/pii-redaction/tests/unit/component_tests.rs +++ b/crates/pii-redaction/tests/unit/component_tests.rs @@ -669,7 +669,7 @@ fn sanitized_pii_never_reaches_subscribers_or_exporters() { openinference.force_flush().unwrap(); let subscriber_json = serde_json::to_string(&captured_events_snapshot(&captured)).unwrap(); - let atof_json = std::fs::read_to_string(atof.path()).unwrap(); + let atof_json = std::fs::read_to_string(atof.path().expect("file sink path")).unwrap(); let atif_json = serde_json::to_string(&trajectory).unwrap(); let otel_debug = format!("{:?}", otel_exporter.get_finished_spans().unwrap()); let openinference_debug = format!("{:?}", openinference_exporter.get_finished_spans().unwrap()); diff --git a/crates/python/src/py_types/observability.rs b/crates/python/src/py_types/observability.rs index 1f7e33b0a..8d7d20ada 100644 --- a/crates/python/src/py_types/observability.rs +++ b/crates/python/src/py_types/observability.rs @@ -166,7 +166,7 @@ impl From for PyAtofExporterM /// /// Configures a remote endpoint URL, transport (`http_post`, `websocket`, or /// `ndjson`), optional string headers, and a positive timeout in milliseconds. -#[pyclass(name = "AtofEndpointConfig", from_py_object)] +#[pyclass(name = "AtofStreamSinkConfig", from_py_object)] #[derive(Clone)] pub struct PyAtofEndpointConfig { #[pyo3(get, set)] @@ -176,6 +176,8 @@ pub struct PyAtofEndpointConfig { #[pyo3(get, set)] pub(crate) headers: HashMap, #[pyo3(get, set)] + pub(crate) header_env: HashMap, + #[pyo3(get, set)] pub(crate) timeout_millis: u64, #[pyo3(get, set)] pub(crate) field_name_policy: String, @@ -206,6 +208,9 @@ impl PyAtofEndpointConfig { for (key, value) in &self.headers { config = config.with_header(key.clone(), value.clone()); } + for (key, variable) in &self.header_env { + config = config.with_header_env(key.clone(), variable.clone()); + } Ok(config) } } @@ -213,11 +218,12 @@ impl PyAtofEndpointConfig { #[pymethods] impl PyAtofEndpointConfig { #[new] - #[pyo3(signature = (url, *, transport="http_post".to_string(), headers=None, timeout_millis=3000, field_name_policy="preserve".to_string()))] + #[pyo3(signature = (url, *, transport="http_post".to_string(), headers=None, header_env=None, timeout_millis=3000, field_name_policy="preserve".to_string()))] pub(crate) fn new( url: String, transport: String, headers: Option<&Bound<'_, PyAny>>, + header_env: Option<&Bound<'_, PyAny>>, timeout_millis: u64, field_name_policy: String, ) -> PyResult { @@ -225,10 +231,15 @@ impl PyAtofEndpointConfig { Some(headers) if !headers.is_none() => py_string_map(headers, "headers")?, _ => HashMap::new(), }; + let header_env = match header_env { + Some(header_env) if !header_env.is_none() => py_string_map(header_env, "header_env")?, + _ => HashMap::new(), + }; Ok(Self { url, transport, headers, + header_env, timeout_millis, field_name_policy, }) @@ -236,15 +247,17 @@ impl PyAtofEndpointConfig { pub(crate) fn __repr__(&self) -> String { format!( - "", + "", self.transport, self.url ) } } -/// Mutable configuration object for the filesystem-backed ATOF JSONL exporter. +/// One tagged ATOF sink configuration for the manual exporter API. #[pyclass(name = "AtofExporterConfig")] pub struct PyAtofExporterConfig { + #[pyo3(get, set)] + pub(crate) sink_type: String, #[pyo3(get, set)] pub(crate) output_directory: String, #[pyo3(get, set)] @@ -252,21 +265,42 @@ pub struct PyAtofExporterConfig { #[pyo3(get, set)] pub(crate) filename: String, #[pyo3(get, set)] - pub(crate) endpoints: Vec, + pub(crate) url: String, + #[pyo3(get, set)] + pub(crate) transport: String, + #[pyo3(get, set)] + pub(crate) headers: HashMap, + #[pyo3(get, set)] + pub(crate) header_env: HashMap, + #[pyo3(get, set)] + pub(crate) timeout_millis: u64, + #[pyo3(get, set)] + pub(crate) field_name_policy: String, } impl PyAtofExporterConfig { fn to_rust_config(&self) -> PyResult { - let endpoints = self - .endpoints - .iter() - .map(PyAtofEndpointConfig::to_rust_config) - .collect::>>()?; - Ok(nemo_relay::observability::atof::AtofExporterConfig::new() - .with_output_directory(PathBuf::from(self.output_directory.clone())) - .with_mode(self.mode.clone().into()) - .with_filename(self.filename.clone()) - .with_endpoints(endpoints)) + match self.sink_type.as_str() { + "file" => Ok(nemo_relay::observability::atof::AtofExporterConfig::new() + .with_output_directory(PathBuf::from(self.output_directory.clone())) + .with_mode(self.mode.clone().into()) + .with_filename(self.filename.clone())), + "stream" => PyAtofEndpointConfig { + url: self.url.clone(), + transport: self.transport.clone(), + headers: self.headers.clone(), + header_env: self.header_env.clone(), + timeout_millis: self.timeout_millis, + field_name_policy: self.field_name_policy.clone(), + } + .to_rust_config() + .map(|sink| { + nemo_relay::observability::atof::AtofExporterConfig::new().with_stream_sink(sink) + }), + _ => Err(pyo3::exceptions::PyValueError::new_err( + "sink_type must be 'file' or 'stream'", + )), + } } } @@ -274,24 +308,27 @@ impl PyAtofExporterConfig { impl PyAtofExporterConfig { #[new] pub(crate) fn new() -> Self { - let config = nemo_relay::observability::atof::AtofExporterConfig::new(); + let config = nemo_relay::observability::atof::AtofFileSinkConfig::new(); Self { + sink_type: "file".to_string(), output_directory: config.output_directory.to_string_lossy().into_owned(), mode: config.mode.into(), filename: config.filename, - endpoints: Vec::new(), + url: String::new(), + transport: "http_post".to_string(), + headers: HashMap::new(), + header_env: HashMap::new(), + timeout_millis: 3000, + field_name_policy: "preserve".to_string(), } } pub(crate) fn __repr__(&self) -> String { - format!( - "", - self.output_directory, self.filename - ) + format!("", self.sink_type) } } -/// Filesystem-backed ATOF JSONL exporter. +/// Single-sink ATOF exporter. /// /// Register the exporter under a subscriber name, run instrumented application /// code, then deregister and shut down the exporter to flush output. @@ -309,10 +346,12 @@ impl PyAtofExporter { Ok(Self { inner }) } - /// Return the JSONL output path. + /// Return the JSONL output path, or ``None`` for a stream sink. #[getter] - pub(crate) fn path(&self) -> String { - self.inner.path().to_string_lossy().into_owned() + pub(crate) fn path(&self) -> Option { + self.inner + .path() + .map(|path| path.to_string_lossy().into_owned()) } /// Register this exporter globally under ``name``. diff --git a/go/nemo_relay/atof_test.go b/go/nemo_relay/atof_test.go index 2ec7a3da6..a873bfc86 100644 --- a/go/nemo_relay/atof_test.go +++ b/go/nemo_relay/atof_test.go @@ -16,52 +16,49 @@ const eventsJSONLFilename = "events.jsonl" func TestNewAtofExporterConfigDefaults(t *testing.T) { config := NewAtofExporterConfig() - - if config.Mode != AtofExporterModeAppend { - t.Fatalf("expected append mode default, got %q", config.Mode) - } - if config.OutputDirectory != "" { - t.Fatalf("expected empty output directory default override, got %q", config.OutputDirectory) - } - if config.Filename != "" { - t.Fatalf("expected empty filename default override, got %q", config.Filename) + file, ok := config.Sink.(AtofFileSinkConfig) + if !ok || file.Mode != AtofExporterModeAppend { + t.Fatalf("expected default append file sink, got %#v", config.Sink) } - if len(config.Endpoints) != 0 { - t.Fatalf("expected no streaming endpoints by default, got %#v", config.Endpoints) - } - config.Endpoints = []AtofEndpointConfig{{ + config.Sink = AtofStreamSinkConfig{ URL: "http://localhost:8080/events", Transport: AtofEndpointTransportHTTPPost, Headers: map[string]string{"X-Test": "yes"}, + HeaderEnv: map[string]string{"authorization": "NEMO_RELAY_ATOF_AUTH"}, TimeoutMillis: 1000, FieldNamePolicy: AtofEndpointFieldNamePolicyReplaceDots, - }} - if config.Endpoints[0].Transport != AtofEndpointTransportHTTPPost || - config.Endpoints[0].FieldNamePolicy != AtofEndpointFieldNamePolicyReplaceDots { - t.Fatalf("unexpected endpoint config: %#v", config.Endpoints[0]) + } + stream := config.Sink.(AtofStreamSinkConfig) + if stream.Transport != AtofEndpointTransportHTTPPost || + stream.FieldNamePolicy != AtofEndpointFieldNamePolicyReplaceDots { + t.Fatalf("unexpected stream sink: %#v", stream) } serialized, err := json.Marshal(config) if err != nil { t.Fatalf("marshal config failed: %v", err) } - if !strings.Contains(string(serialized), `"field_name_policy":"replace_dots"`) { - t.Fatalf("expected field_name_policy in serialized config, got %s", serialized) + if !strings.Contains(string(serialized), `"field_name_policy":"replace_dots"`) || + !strings.Contains(string(serialized), `"header_env":{"authorization":"NEMO_RELAY_ATOF_AUTH"}`) { + t.Fatalf("expected stream sink settings in serialized config, got %s", serialized) } } func TestAtofExporterLifecycleWritesRawJSONL(t *testing.T) { dir := t.TempDir() - exporter, err := NewAtofExporter(AtofExporterConfig{ + exporter, err := NewAtofExporter(AtofExporterConfig{Sink: AtofFileSinkConfig{ OutputDirectory: dir, Mode: AtofExporterModeOverwrite, Filename: eventsJSONLFilename, - }) + }}) requireNoError(t, err, "NewAtofExporter failed") defer exporter.Close() path, err := exporter.Path() requireNoError(t, err, "Path failed") - requireEqual(t, filepath.Base(path), eventsJSONLFilename, "expected %s path", eventsJSONLFilename) + if path == nil { + t.Fatal("expected file exporter path") + } + requireEqual(t, filepath.Base(*path), eventsJSONLFilename, "expected %s path", eventsJSONLFilename) name := "go_atof_" + time.Now().Format("150405.000000") requireNoError(t, exporter.Register(name), "Register failed") @@ -87,7 +84,7 @@ func TestAtofExporterLifecycleWritesRawJSONL(t *testing.T) { requireNoError(t, exporter.ForceFlush(), "ForceFlush failed") requireNoError(t, exporter.Shutdown(), "Shutdown failed") - records := readAtofRecords(t, path) + records := readAtofRecords(t, *path) if len(records) != 3 { t.Fatalf("expected 3 records, got %d", len(records)) } @@ -109,10 +106,10 @@ func TestAtofExporterAppendAndOverwriteModes(t *testing.T) { t.Fatalf("write seed file: %v", err) } - appendExporter, err := NewAtofExporter(AtofExporterConfig{ + appendExporter, err := NewAtofExporter(AtofExporterConfig{Sink: AtofFileSinkConfig{ OutputDirectory: dir, Filename: eventsJSONLFilename, - }) + }}) if err != nil { t.Fatalf("append NewAtofExporter failed: %v", err) } @@ -124,11 +121,11 @@ func TestAtofExporterAppendAndOverwriteModes(t *testing.T) { t.Fatalf("append mode changed file: %q", got) } - overwriteExporter, err := NewAtofExporter(AtofExporterConfig{ + overwriteExporter, err := NewAtofExporter(AtofExporterConfig{Sink: AtofFileSinkConfig{ OutputDirectory: dir, Mode: AtofExporterModeOverwrite, Filename: eventsJSONLFilename, - }) + }}) if err != nil { t.Fatalf("overwrite NewAtofExporter failed: %v", err) } diff --git a/go/nemo_relay/nemo_relay.go b/go/nemo_relay/nemo_relay.go index b09718ff5..b3ab6f02b 100644 --- a/go/nemo_relay/nemo_relay.go +++ b/go/nemo_relay/nemo_relay.go @@ -335,38 +335,14 @@ var ( return checkedValue(int32(status), &AtifExporter{ptr: ptr}) } newAtofExporterFunc = func(config AtofExporterConfig) (*AtofExporter, error) { - if config.Mode == "" { - config.Mode = AtofExporterModeAppend + payload, err := json.Marshal(config) + if err != nil { + return nil, err } - if len(config.Endpoints) > 0 { - payload, err := json.Marshal(config) - if err != nil { - return nil, err - } - cConfig := C.CString(string(payload)) - defer C.free(unsafe.Pointer(cConfig)) - var ptr unsafe.Pointer - status := C.nemo_relay_atof_exporter_create_from_json(cConfig, &ptr) - return checkedValue(int32(status), &AtofExporter{ptr: ptr}) - } - - var cOutputDirectory *C.char - if config.OutputDirectory != "" { - cOutputDirectory = C.CString(config.OutputDirectory) - defer C.free(unsafe.Pointer(cOutputDirectory)) - } - - cMode := C.CString(string(config.Mode)) - defer C.free(unsafe.Pointer(cMode)) - - var cFilename *C.char - if config.Filename != "" { - cFilename = C.CString(config.Filename) - defer C.free(unsafe.Pointer(cFilename)) - } - + cConfig := C.CString(string(payload)) + defer C.free(unsafe.Pointer(cConfig)) var ptr unsafe.Pointer - status := C.nemo_relay_atof_exporter_create(cOutputDirectory, cMode, cFilename, &ptr) + status := C.nemo_relay_atof_exporter_create_from_json(cConfig, &ptr) return checkedValue(int32(status), &AtofExporter{ptr: ptr}) } ) @@ -1704,12 +1680,42 @@ const ( AtofExporterModeOverwrite AtofExporterMode = "overwrite" ) -// AtofExporterConfig configures the filesystem-backed ATOF JSONL exporter. +// AtofExporterConfig configures one tagged ATOF sink. type AtofExporterConfig struct { - OutputDirectory string `json:"output_directory,omitempty"` - Mode AtofExporterMode `json:"mode,omitempty"` - Filename string `json:"filename,omitempty"` - Endpoints []AtofEndpointConfig `json:"endpoints,omitempty"` + Sink AtofSinkConfigurer `json:"-"` +} + +// MarshalJSON serializes the selected tagged sink directly, matching the Rust API. +func (config AtofExporterConfig) MarshalJSON() ([]byte, error) { + if config.Sink == nil { + return json.Marshal(NewAtofFileSinkConfig()) + } + return json.Marshal(config.Sink) +} + +// AtofSinkConfigurer is one ATOF exporter destination. +type AtofSinkConfigurer interface { + atofExporterSink() +} + +// AtofFileSinkConfig configures one filesystem ATOF JSONL destination. +type AtofFileSinkConfig struct { + OutputDirectory string `json:"output_directory,omitempty"` + Mode AtofExporterMode `json:"mode,omitempty"` + Filename string `json:"filename,omitempty"` +} + +func (AtofFileSinkConfig) atofExporterSink() {} + +// MarshalJSON serializes the fixed file sink discriminator. +func (config AtofFileSinkConfig) MarshalJSON() ([]byte, error) { + type fileSinkJSON struct { + Type string `json:"type"` + OutputDirectory string `json:"output_directory,omitempty"` + Mode AtofExporterMode `json:"mode,omitempty"` + Filename string `json:"filename,omitempty"` + } + return json.Marshal(fileSinkJSON{"file", config.OutputDirectory, config.Mode, config.Filename}) } // AtofEndpointTransport controls how an ATOF streaming endpoint receives events. @@ -1734,19 +1740,49 @@ const ( AtofEndpointFieldNamePolicyReplaceDots AtofEndpointFieldNamePolicy = "replace_dots" ) -// AtofEndpointConfig configures one streaming destination for raw ATOF events. -type AtofEndpointConfig struct { +// AtofStreamSinkConfig configures one streaming destination for raw ATOF events. +type AtofStreamSinkConfig struct { URL string `json:"url"` Transport AtofEndpointTransport `json:"transport,omitempty"` Headers map[string]string `json:"headers,omitempty"` + HeaderEnv map[string]string `json:"header_env,omitempty"` TimeoutMillis uint64 `json:"timeout_millis,omitempty"` FieldNamePolicy AtofEndpointFieldNamePolicy `json:"field_name_policy,omitempty"` } +func (AtofStreamSinkConfig) atofExporterSink() {} + +// MarshalJSON serializes the fixed stream sink discriminator. +func (config AtofStreamSinkConfig) MarshalJSON() ([]byte, error) { + type streamSinkJSON struct { + Type string `json:"type"` + URL string `json:"url"` + Transport AtofEndpointTransport `json:"transport,omitempty"` + Headers map[string]string `json:"headers,omitempty"` + HeaderEnv map[string]string `json:"header_env,omitempty"` + TimeoutMillis uint64 `json:"timeout_millis,omitempty"` + FieldNamePolicy AtofEndpointFieldNamePolicy `json:"field_name_policy,omitempty"` + } + return json.Marshal(streamSinkJSON{"stream", config.URL, config.Transport, config.Headers, config.HeaderEnv, config.TimeoutMillis, config.FieldNamePolicy}) +} + // NewAtofExporterConfig returns a config initialized with native defaults. func NewAtofExporterConfig() AtofExporterConfig { - return AtofExporterConfig{ - Mode: AtofExporterModeAppend, + return AtofExporterConfig{Sink: NewAtofFileSinkConfig()} +} + +// NewAtofFileSinkConfig returns a file sink initialized with native defaults. +func NewAtofFileSinkConfig() AtofFileSinkConfig { + return AtofFileSinkConfig{Mode: AtofExporterModeAppend} +} + +// NewAtofStreamSinkConfig returns an HTTP POST stream sink with native defaults. +func NewAtofStreamSinkConfig(url string) AtofStreamSinkConfig { + return AtofStreamSinkConfig{ + URL: url, + Transport: AtofEndpointTransportHTTPPost, + TimeoutMillis: 3000, + FieldNamePolicy: AtofEndpointFieldNamePolicyPreserve, } } @@ -1755,20 +1791,24 @@ type AtofExporter struct { ptr unsafe.Pointer } -// NewAtofExporter creates a new filesystem-backed ATOF JSONL exporter. +// NewAtofExporter creates a new single-sink ATOF exporter. func NewAtofExporter(config AtofExporterConfig) (*AtofExporter, error) { return newAtofExporterFunc(config) } -// Path returns the JSONL output path. -func (e *AtofExporter) Path() (string, error) { +// Path returns the JSONL output path, or nil for a stream-backed exporter. +func (e *AtofExporter) Path() (*string, error) { var cOut *C.char status := C.nemo_relay_atof_exporter_path(e.ptr, &cOut) if err := checkStatus(status); err != nil { - return "", err + return nil, err + } + if cOut == nil { + return nil, nil } defer C.nemo_relay_string_free(cOut) - return C.GoString(cOut), nil + path := C.GoString(cOut) + return &path, nil } // Register registers the exporter as a global event subscriber. diff --git a/go/nemo_relay/observability_plugin.go b/go/nemo_relay/observability_plugin.go index c79488919..ae731cf4b 100644 --- a/go/nemo_relay/observability_plugin.go +++ b/go/nemo_relay/observability_plugin.go @@ -32,11 +32,59 @@ type ObservabilityConfig struct { // ObservabilityAtofConfig configures filesystem-backed raw ATOF JSONL export. type ObservabilityAtofConfig struct { - Enabled bool `json:"enabled,omitempty"` - OutputDirectory string `json:"output_directory,omitempty"` - Filename string `json:"filename,omitempty"` - Mode string `json:"mode,omitempty"` - Endpoints []ObservabilityAtofEndpoint `json:"endpoints,omitempty"` + Enabled bool `json:"enabled,omitempty"` + Sinks []ObservabilityAtofSinkConfigurer `json:"sinks,omitempty"` +} + +// ObservabilityAtofSinkConfigurer is one ATOF destination. +type ObservabilityAtofSinkConfigurer interface { + atofSinkConfig() +} + +// ObservabilityAtofFileSinkConfig configures one filesystem ATOF JSONL destination. +type ObservabilityAtofFileSinkConfig struct { + OutputDirectory string `json:"output_directory,omitempty"` + Filename string `json:"filename,omitempty"` + Mode string `json:"mode,omitempty"` +} + +func (ObservabilityAtofFileSinkConfig) atofSinkConfig() {} + +// MarshalJSON serializes the fixed file sink discriminator. +func (config ObservabilityAtofFileSinkConfig) MarshalJSON() ([]byte, error) { + type fileSinkJSON struct { + Type string `json:"type"` + OutputDirectory string `json:"output_directory,omitempty"` + Filename string `json:"filename,omitempty"` + Mode string `json:"mode,omitempty"` + } + return json.Marshal(fileSinkJSON{"file", config.OutputDirectory, config.Filename, config.Mode}) +} + +// ObservabilityAtofStreamSinkConfig configures one remote ATOF destination. +type ObservabilityAtofStreamSinkConfig struct { + URL string `json:"url"` + Transport string `json:"transport,omitempty"` + Headers map[string]string `json:"headers,omitempty"` + HeaderEnv map[string]string `json:"header_env,omitempty"` + TimeoutMillis uint64 `json:"timeout_millis,omitempty"` + FieldNamePolicy string `json:"field_name_policy,omitempty"` +} + +func (ObservabilityAtofStreamSinkConfig) atofSinkConfig() {} + +// MarshalJSON serializes the fixed stream sink discriminator. +func (config ObservabilityAtofStreamSinkConfig) MarshalJSON() ([]byte, error) { + type streamSinkJSON struct { + Type string `json:"type"` + URL string `json:"url"` + Transport string `json:"transport,omitempty"` + Headers map[string]string `json:"headers,omitempty"` + HeaderEnv map[string]string `json:"header_env,omitempty"` + TimeoutMillis uint64 `json:"timeout_millis,omitempty"` + FieldNamePolicy string `json:"field_name_policy,omitempty"` + } + return json.Marshal(streamSinkJSON{"stream", config.URL, config.Transport, config.Headers, config.HeaderEnv, config.TimeoutMillis, config.FieldNamePolicy}) } // ObservabilityAtofEndpoint configures one streaming destination for raw ATOF events. @@ -184,16 +232,24 @@ type ObservabilityComponentSpec struct { Config ObservabilityConfig `json:"config"` } -// NewObservabilityConfig returns a default observability config with version 1. +// NewObservabilityConfig returns a default observability config with version 2. func NewObservabilityConfig() ObservabilityConfig { - return ObservabilityConfig{Version: 1} + return ObservabilityConfig{Version: 2} } // NewObservabilityAtofConfig returns disabled ATOF JSONL settings with native defaults. func NewObservabilityAtofConfig() ObservabilityAtofConfig { - return ObservabilityAtofConfig{ - Mode: "append", - } + return ObservabilityAtofConfig{} +} + +// NewObservabilityAtofFileSinkConfig returns one file ATOF sink with native defaults. +func NewObservabilityAtofFileSinkConfig() ObservabilityAtofFileSinkConfig { + return ObservabilityAtofFileSinkConfig{Mode: "append"} +} + +// NewObservabilityAtofStreamSinkConfig returns one stream ATOF sink. +func NewObservabilityAtofStreamSinkConfig(url string) ObservabilityAtofStreamSinkConfig { + return ObservabilityAtofStreamSinkConfig{URL: url, Transport: "http_post", TimeoutMillis: 3000, FieldNamePolicy: "preserve"} } // NewObservabilityAtifConfig returns disabled ATIF settings with core defaults. diff --git a/go/nemo_relay/observability_plugin_test.go b/go/nemo_relay/observability_plugin_test.go index 7c94c33b9..664b93cca 100644 --- a/go/nemo_relay/observability_plugin_test.go +++ b/go/nemo_relay/observability_plugin_test.go @@ -28,17 +28,18 @@ const ( func TestObservabilityConfigHelpers(t *testing.T) { config := NewObservabilityConfig() - if config.Version != 1 { - t.Fatalf("expected version 1, got %d", config.Version) + if config.Version != 2 { + t.Fatalf("expected version 2, got %d", config.Version) } atof := NewObservabilityAtofConfig() - if atof.Enabled || atof.Mode != "append" { + if atof.Enabled || len(atof.Sinks) != 0 { t.Fatalf("unexpected ATOF defaults: %#v", atof) } - atof.Endpoints = []ObservabilityAtofEndpoint{{ + atof.Sinks = []ObservabilityAtofSinkConfigurer{ObservabilityAtofStreamSinkConfig{ URL: "http://localhost:8080/events", Transport: "http_post", Headers: map[string]string{"X-Test": "yes"}, + HeaderEnv: map[string]string{"authorization": "NEMO_RELAY_ATOF_AUTH"}, TimeoutMillis: 1000, FieldNamePolicy: "replace_dots", }} @@ -80,13 +81,14 @@ func TestObservabilityConfigHelpers(t *testing.T) { t.Fatalf("expected serialized ATOF config object, got %#v", wrapped.Config) } atofConfig := wrapped.Config["atof"].(map[string]any) - endpoints, ok := atofConfig["endpoints"].([]any) + sinks, ok := atofConfig["sinks"].([]any) if !ok { - t.Fatalf("expected serialized ATOF endpoints, got %#v", atofConfig) + t.Fatalf("expected serialized ATOF sinks, got %#v", atofConfig) } - firstEndpoint, ok := endpoints[0].(map[string]any) - if !ok || firstEndpoint["field_name_policy"] != "replace_dots" { - t.Fatalf("expected serialized ATOF endpoint field name policy, got %#v", endpoints) + firstEndpoint, ok := sinks[0].(map[string]any) + if !ok || firstEndpoint["field_name_policy"] != "replace_dots" || + firstEndpoint["header_env"].(map[string]any)["authorization"] != "NEMO_RELAY_ATOF_AUTH" { + t.Fatalf("expected serialized ATOF stream sink settings, got %#v", sinks) } serialized, err := json.Marshal(wrapped) if err != nil { @@ -228,9 +230,7 @@ func NewAtofAndAtifTestConfig(dir string) ObservabilityConfig { config := NewObservabilityConfig() atof := NewObservabilityAtofConfig() atof.Enabled = true - atof.OutputDirectory = dir - atof.Filename = eventsJSONLFilename - atof.Mode = "overwrite" + atof.Sinks = []ObservabilityAtofSinkConfigurer{ObservabilityAtofFileSinkConfig{OutputDirectory: dir, Filename: eventsJSONLFilename, Mode: "overwrite"}} config.Atof = &atof atif := NewObservabilityAtifConfig() @@ -322,7 +322,7 @@ func TestObservabilityPluginAtifSplitsMultipleTopLevelAgents(t *testing.T) { func TestObservabilityPluginValidationRejectsBadValues(t *testing.T) { config := NewObservabilityConfig() atof := NewObservabilityAtofConfig() - atof.Mode = "bad" + atof.Sinks = []ObservabilityAtofSinkConfigurer{ObservabilityAtofFileSinkConfig{Mode: "bad"}} config.Atof = &atof atif := NewObservabilityAtifConfig() atif.FilenameTemplate = "missing-placeholder.json" diff --git a/justfile b/justfile index b7666532d..1b456428f 100644 --- a/justfile +++ b/justfile @@ -1124,6 +1124,9 @@ test-python: junit_out="" rust_coverage_out="" cd "$NEMO_RELAY_REPO_ROOT" + test_config_home="$(mktemp -d)" + trap 'rm -rf "$test_config_home"' EXIT + export XDG_CONFIG_HOME="$test_config_home" if is_true "{{ ci }}"; then coverage_out="$(prepare_artifact python-coverage.xml)" junit_out="$(prepare_artifact python-junit.xml)" @@ -1363,6 +1366,9 @@ test-node: junit_out="" rust_coverage_out="" cd "$NEMO_RELAY_REPO_ROOT" + test_config_home="$(mktemp -d)" + trap 'rm -rf "$test_config_home"' EXIT + export XDG_CONFIG_HOME="$test_config_home" if is_true "{{ ci }}"; then coverage_out="$(prepare_artifact node-coverage.xml)" junit_out="$(prepare_artifact node-junit.xml)" diff --git a/python/nemo_relay/__init__.py b/python/nemo_relay/__init__.py index afdb79170..b54404246 100644 --- a/python/nemo_relay/__init__.py +++ b/python/nemo_relay/__init__.py @@ -91,10 +91,10 @@ async def main(): AnnotatedLLMRequest, AnnotatedLLMResponse, AtifExporter, - AtofEndpointConfig, AtofExporter, AtofExporterConfig, AtofExporterMode, + AtofStreamSinkConfig, LLMAttributes, LLMHandle, LLMRequest, @@ -478,7 +478,7 @@ def worker() -> None: "AnnotatedLLMRequest", "AnnotatedLLMResponse", "AtifExporter", - "AtofEndpointConfig", + "AtofStreamSinkConfig", "AtofExporterMode", "AtofExporterConfig", "AtofExporter", diff --git a/python/nemo_relay/__init__.pyi b/python/nemo_relay/__init__.pyi index 4a95aea2a..04cb3429d 100644 --- a/python/nemo_relay/__init__.pyi +++ b/python/nemo_relay/__init__.pyi @@ -48,9 +48,6 @@ from nemo_relay._native import ( from nemo_relay._native import ( AtifExporter as AtifExporter, ) -from nemo_relay._native import ( - AtofEndpointConfig as AtofEndpointConfig, -) from nemo_relay._native import ( AtofExporter as AtofExporter, ) @@ -60,6 +57,9 @@ from nemo_relay._native import ( from nemo_relay._native import ( AtofExporterMode as AtofExporterMode, ) +from nemo_relay._native import ( + AtofStreamSinkConfig as AtofStreamSinkConfig, +) from nemo_relay._native import ( LLMAttributes as LLMAttributes, ) diff --git a/python/nemo_relay/_native.pyi b/python/nemo_relay/_native.pyi index 400307416..50bb8a854 100644 --- a/python/nemo_relay/_native.pyi +++ b/python/nemo_relay/_native.pyi @@ -830,12 +830,13 @@ class AtofExporterMode: Append: ClassVar[AtofExporterMode] Overwrite: ClassVar[AtofExporterMode] -class AtofEndpointConfig: - """Streaming destination for raw ATOF events.""" +class AtofStreamSinkConfig: + """One stream sink for raw ATOF events.""" url: str transport: str headers: dict[str, str] + header_env: dict[str, str] timeout_millis: int field_name_policy: str @@ -845,36 +846,43 @@ class AtofEndpointConfig: *, transport: str = "http_post", headers: dict[str, str] | None = None, + header_env: dict[str, str] | None = None, timeout_millis: int = 3000, field_name_policy: str = "preserve", ) -> None: - """Create an ATOF streaming endpoint config. + """Create an ATOF stream sink config. ``headers=None`` is converted to an empty dict; the instance field is always non-optional. """ class AtofExporterConfig: - """Mutable configuration for the filesystem-backed ATOF JSONL exporter.""" + """One tagged sink configuration for the manual ATOF exporter.""" + sink_type: str output_directory: str mode: AtofExporterMode filename: str - endpoints: list[AtofEndpointConfig] + url: str + transport: str + headers: dict[str, str] + header_env: dict[str, str] + timeout_millis: int + field_name_policy: str def __init__(self) -> None: """Create an ATOF exporter config with native defaults.""" ... class AtofExporter: - """Filesystem-backed exporter that writes raw ATOF events as JSONL.""" + """Single-sink exporter that writes or streams raw ATOF events.""" def __init__(self, config: AtofExporterConfig) -> None: """Create an ATOF JSONL exporter from config.""" ... @property - def path(self) -> str: - """Return the JSONL output path.""" + def path(self) -> str | None: + """Return the JSONL output path, or ``None`` for a stream sink.""" ... def register(self, name: str) -> None: """Register the exporter under ``name``.""" diff --git a/python/nemo_relay/observability.py b/python/nemo_relay/observability.py index ad837a0a1..1495ba9f7 100644 --- a/python/nemo_relay/observability.py +++ b/python/nemo_relay/observability.py @@ -55,22 +55,25 @@ def to_dict(self) -> JsonObject: @dataclass(slots=True) -class AtofEndpointConfig: - """Streaming destination for raw ATOF events.""" +class AtofStreamSinkConfig: + """Stream sink for raw ATOF events.""" url: str transport: Literal["http_post", "websocket", "ndjson"] = "http_post" headers: dict[str, str] = field(default_factory=dict) + header_env: dict[str, str] = field(default_factory=dict) timeout_millis: int = 3000 field_name_policy: Literal["preserve", "replace_dots"] = "preserve" def to_dict(self) -> JsonObject: - """Serialize this ATOF endpoint config to the canonical JSON object shape.""" + """Serialize this ATOF stream sink to the canonical JSON object shape.""" return _normalize_object( { + "type": "stream", "url": self.url, "transport": self.transport, "headers": self.headers, + "header_env": self.header_env, "timeout_millis": self.timeout_millis, "field_name_policy": self.field_name_policy, } @@ -79,27 +82,44 @@ def to_dict(self) -> JsonObject: @dataclass(slots=True) class AtofConfig: - """Filesystem-backed raw ATOF JSONL export settings.""" + """Multi-sink raw ATOF export settings.""" enabled: bool = False + sinks: list["AtofFileSinkConfig | AtofStreamSinkConfig"] | None = None + + def to_dict(self) -> JsonObject: + """Serialize this ATOF config to the canonical JSON object shape.""" + return _normalize_object( + { + "enabled": self.enabled, + "sinks": self.sinks, + } + ) + + +@dataclass(slots=True) +class AtofFileSinkConfig: + """Filesystem destination for raw ATOF JSONL events.""" + output_directory: str | None = None filename: str | None = None mode: Literal["append", "overwrite"] = "append" - endpoints: list[AtofEndpointConfig] | None = None def to_dict(self) -> JsonObject: - """Serialize this ATOF config to the canonical JSON object shape.""" return _normalize_object( { - "enabled": self.enabled, + "type": "file", "output_directory": self.output_directory, "filename": self.filename, "mode": self.mode, - "endpoints": self.endpoints, } ) +# Compatibility alias for the former plugin helper name. +AtofEndpointConfig = AtofStreamSinkConfig + + @dataclass(slots=True) class S3StorageConfig: """S3-compatible remote storage settings for ATIF trajectory upload. @@ -232,7 +252,7 @@ def to_dict(self) -> JsonObject: class ObservabilityConfig: """Canonical config document for the top-level observability component.""" - version: int = 1 + version: int = 2 atof: AtofConfig | None = None atif: AtifConfig | None = None opentelemetry: OtlpConfig | None = None @@ -275,6 +295,8 @@ def to_dict(self) -> JsonObject: __all__ = [ "ConfigPolicy", "AtofEndpointConfig", + "AtofFileSinkConfig", + "AtofStreamSinkConfig", "AtofConfig", "AtifConfig", "HttpStorageConfig", diff --git a/python/nemo_relay/observability.pyi b/python/nemo_relay/observability.pyi index f562a10a1..bdeaf236b 100644 --- a/python/nemo_relay/observability.pyi +++ b/python/nemo_relay/observability.pyi @@ -20,10 +20,11 @@ class ConfigPolicy: def to_dict(self) -> JsonObject: ... @dataclass(slots=True) -class AtofEndpointConfig: +class AtofStreamSinkConfig: url: str = ... transport: Literal["http_post", "websocket", "ndjson"] = ... headers: dict[str, str] = field(default_factory=dict) + header_env: dict[str, str] = field(default_factory=dict) timeout_millis: int = ... field_name_policy: Literal["preserve", "replace_dots"] = ... def to_dict(self) -> JsonObject: ... @@ -31,12 +32,18 @@ class AtofEndpointConfig: @dataclass(slots=True) class AtofConfig: enabled: bool = ... + sinks: list[AtofFileSinkConfig | AtofStreamSinkConfig] | None = ... + def to_dict(self) -> JsonObject: ... + +@dataclass(slots=True) +class AtofFileSinkConfig: output_directory: str | None = ... filename: str | None = ... mode: Literal["append", "overwrite"] = ... - endpoints: list[AtofEndpointConfig] | None = ... def to_dict(self) -> JsonObject: ... +AtofEndpointConfig = AtofStreamSinkConfig + @dataclass(slots=True) class S3StorageConfig: bucket: str = ... diff --git a/python/tests/test_observability_plugin.py b/python/tests/test_observability_plugin.py index e536fcaf8..7839f16e3 100644 --- a/python/tests/test_observability_plugin.py +++ b/python/tests/test_observability_plugin.py @@ -15,7 +15,8 @@ OBSERVABILITY_PLUGIN_KIND, AtifConfig, AtofConfig, - AtofEndpointConfig, + AtofFileSinkConfig, + AtofStreamSinkConfig, ComponentSpec, HttpStorageConfig, ObservabilityConfig, @@ -29,7 +30,7 @@ class TestObservabilityConfigHelpers: def test_defaults_and_component_wrapper(self): - assert AtofConfig().to_dict() == {"enabled": False, "mode": "append"} + assert AtofConfig().to_dict() == {"enabled": False} assert AtifConfig().to_dict() == { "enabled": False, "agent_name": "NeMo Relay", @@ -53,7 +54,7 @@ def test_defaults_and_component_wrapper(self): assert wrapped["enabled"] is True wrapped_config = wrapped["config"] assert isinstance(wrapped_config, dict) - assert wrapped_config["version"] == 1 + assert wrapped_config["version"] == 2 def test_validation_rejects_bad_values(self): report = plugin.validate( @@ -61,8 +62,8 @@ def test_validation_rejects_bad_values(self): components=[ ComponentSpec( { - "version": 1, - "atof": {"mode": "bad"}, + "version": 2, + "atof": {"sinks": [{"type": "file", "mode": "bad"}]}, "atif": {"filename_template": "missing-placeholder"}, } ) @@ -70,7 +71,7 @@ def test_validation_rejects_bad_values(self): ) ) fields = {diag.get("field") for diag in report["diagnostics"]} - assert {"mode", "filename_template"} <= fields + assert {"sinks[0].mode", "filename_template"} <= fields def test_list_kinds_includes_builtin_observability(self): assert OBSERVABILITY_PLUGIN_KIND in plugin.list_kinds() @@ -100,22 +101,25 @@ def test_s3_storage_config_serializes_credential_fields(self): atif = AtifConfig(enabled=True, storage=[storage]) assert atif.to_dict()["storage"] == [storage.to_dict()] - def test_atof_endpoint_config_serializes_streaming_fields(self): - endpoint = AtofEndpointConfig( + def test_atof_sink_config_serializes_streaming_fields(self): + sink = AtofStreamSinkConfig( url="http://localhost:8080/events", transport="http_post", headers={"X-Test": "yes"}, + header_env={"authorization": "NEMO_RELAY_ATOF_AUTH"}, timeout_millis=1000, field_name_policy="replace_dots", ) - assert endpoint.to_dict() == { + assert sink.to_dict() == { + "type": "stream", "url": "http://localhost:8080/events", "transport": "http_post", "headers": {"X-Test": "yes"}, + "header_env": {"authorization": "NEMO_RELAY_ATOF_AUTH"}, "timeout_millis": 1000, "field_name_policy": "replace_dots", } - assert AtofConfig(endpoints=[endpoint]).to_dict()["endpoints"] == [endpoint.to_dict()] + assert AtofConfig(sinks=[sink]).to_dict()["sinks"] == [sink.to_dict()] def test_http_storage_config_serializes_headers(self): s3 = S3StorageConfig(bucket="archive") @@ -140,9 +144,13 @@ async def test_atof_and_atif_file_outputs(self, tmp_path: Path, use_context_mana config = ObservabilityConfig( atof=AtofConfig( enabled=True, - output_directory=str(tmp_path), - filename="events.jsonl", - mode="overwrite", + sinks=[ + AtofFileSinkConfig( + output_directory=str(tmp_path), + filename="events.jsonl", + mode="overwrite", + ) + ], ), atif=AtifConfig( enabled=True, diff --git a/python/tests/test_types.py b/python/tests/test_types.py index 7cf13d3c5..c66f658e9 100644 --- a/python/tests/test_types.py +++ b/python/tests/test_types.py @@ -13,7 +13,6 @@ from nemo_relay import ( AtifExporter, - AtofEndpointConfig, AtofExporter, AtofExporterConfig, AtofExporterMode, @@ -458,39 +457,35 @@ def test_config_defaults_mutation_and_repr(self, tmp_path): assert config.mode == AtofExporterMode.Append assert config.filename.startswith("nemo-relay-events-") assert config.filename.endswith(".jsonl") - assert config.endpoints == [] + assert config.sink_type == "file" assert "AtofExporterConfig" in repr(config) config.output_directory = str(tmp_path) config.mode = AtofExporterMode.Overwrite config.filename = "events.jsonl" - endpoint = AtofEndpointConfig( - "http://localhost:8080/events", - transport="http_post", - headers={"X-Test": "yes"}, - timeout_millis=1000, - field_name_policy="replace_dots", - ) - config.endpoints = [endpoint] + config.sink_type = "stream" + config.url = "http://localhost:8080/events" + config.transport = "http_post" + config.headers = {"X-Test": "yes"} + config.header_env = {"authorization": "NEMO_RELAY_ATOF_AUTH"} + config.timeout_millis = 1000 + config.field_name_policy = "replace_dots" assert config.output_directory == str(tmp_path) assert config.mode == AtofExporterMode.Overwrite assert config.filename == "events.jsonl" - assert config.endpoints[0].url == "http://localhost:8080/events" - assert config.endpoints[0].transport == "http_post" - assert config.endpoints[0].headers == {"X-Test": "yes"} - assert config.endpoints[0].timeout_millis == 1000 - assert config.endpoints[0].field_name_policy == "replace_dots" + assert config.url == "http://localhost:8080/events" + assert config.transport == "http_post" + assert config.headers == {"X-Test": "yes"} + assert config.header_env == {"authorization": "NEMO_RELAY_ATOF_AUTH"} + assert config.timeout_millis == 1000 + assert config.field_name_policy == "replace_dots" def test_endpoint_field_name_policy_is_validated(self, tmp_path): config = AtofExporterConfig() - config.output_directory = str(tmp_path) - config.endpoints = [ - AtofEndpointConfig( - "http://localhost:8080/events", - field_name_policy="bogus", # type: ignore[arg-type] - ) - ] + config.sink_type = "stream" + config.url = "http://localhost:8080/events" + config.field_name_policy = "bogus" with pytest.raises(ValueError, match="field_name_policy"): AtofExporter(config) @@ -503,6 +498,7 @@ def test_exporter_lifecycle_writes_raw_jsonl_events(self, tmp_path): exporter = AtofExporter(config) assert "" in repr(exporter) + assert exporter.path is not None assert exporter.path.endswith("events.jsonl") subscriber_name = f"py_atof_{uuid4().hex}" From f2b2a95392e5cfbc8a616f8c02aaf61a38b854bf Mon Sep 17 00:00:00 2001 From: Will Killian Date: Tue, 14 Jul 2026 09:16:12 -0400 Subject: [PATCH 2/3] fix: align ATOF v2 fixtures with CI Signed-off-by: Will Killian --- crates/cli/tests/cli_tests.rs | 5 +- crates/cli/tests/coverage/doctor_tests.rs | 18 +-- crates/cli/tests/coverage/launcher_tests.rs | 9 +- crates/cli/tests/coverage/plugins_tests.rs | 127 +++++++++++------- crates/cli/tests/coverage/server_tests.rs | 86 +++++++----- crates/cli/tests/coverage/session_tests.rs | 2 +- .../src/observability/plugin_component.rs | 2 + integrations/openclaw/test/live-smoke.test.ts | 2 +- 8 files changed, 161 insertions(+), 90 deletions(-) diff --git a/crates/cli/tests/cli_tests.rs b/crates/cli/tests/cli_tests.rs index 91dfa0b71..e926613d8 100644 --- a/crates/cli/tests/cli_tests.rs +++ b/crates/cli/tests/cli_tests.rs @@ -1549,10 +1549,13 @@ kind = "observability" enabled = true [components.config] -version = 1 +version = 2 [components.config.atof] enabled = true + +[[components.config.atof.sinks]] +type = "file" output_directory = "logs" filename = "events.jsonl" mode = "append" diff --git a/crates/cli/tests/coverage/doctor_tests.rs b/crates/cli/tests/coverage/doctor_tests.rs index cec9772e8..f784fb065 100644 --- a/crates/cli/tests/coverage/doctor_tests.rs +++ b/crates/cli/tests/coverage/doctor_tests.rs @@ -1103,10 +1103,11 @@ async fn collect_observability_probes_atof_streaming_endpoint() { "kind": "observability", "enabled": true, "config": { - "version": 1, + "version": 2, "atof": { "enabled": true, - "endpoints": [{ + "sinks": [{ + "type": "stream", "url": url, "transport": "http_post", "headers": {"X-Test": "doctor"} @@ -1135,8 +1136,8 @@ async fn collect_observability_probes_atof_streaming_endpoint() { let endpoint = checks .iter() - .find(|check| check.name == "ATOF endpoint") - .expect("ATOF endpoint check"); + .find(|check| check.name == "ATOF stream sink") + .expect("ATOF stream sink check"); assert_eq!(endpoint.status, Status::Pass); assert!(body.contains("\"kind\":\"mark\"")); assert!(body.contains("\"name\":\"nemo_relay.doctor.atof_probe\"")); @@ -1187,10 +1188,11 @@ async fn collect_observability_rejects_websocket_endpoint_http_scheme() { "kind": "observability", "enabled": true, "config": { - "version": 1, + "version": 2, "atof": { "enabled": true, - "endpoints": [{ + "sinks": [{ + "type": "stream", "url": "http://localhost:9/events", "transport": "websocket" }] @@ -1205,8 +1207,8 @@ async fn collect_observability_rejects_websocket_endpoint_http_scheme() { let endpoint = checks .iter() - .find(|check| check.name == "ATOF endpoint") - .expect("ATOF endpoint check"); + .find(|check| check.name == "ATOF stream sink") + .expect("ATOF stream sink check"); assert_eq!(endpoint.status, Status::Fail); assert!(endpoint.details.contains("invalid scheme")); assert!(endpoint.details.contains("must be ws or wss")); diff --git a/crates/cli/tests/coverage/launcher_tests.rs b/crates/cli/tests/coverage/launcher_tests.rs index d0231a541..660ca0268 100644 --- a/crates/cli/tests/coverage/launcher_tests.rs +++ b/crates/cli/tests/coverage/launcher_tests.rs @@ -344,11 +344,14 @@ fn exporter_destinations_describe_observability_outputs() { "kind": OBSERVABILITY_PLUGIN_KIND, "enabled": true, "config": { - "version": 1, + "version": 2, "atof": { "enabled": true, - "output_directory": "logs", - "filename": "events.jsonl" + "sinks": [{ + "type": "file", + "output_directory": "logs", + "filename": "events.jsonl" + }] }, "atif": { "enabled": true, diff --git a/crates/cli/tests/coverage/plugins_tests.rs b/crates/cli/tests/coverage/plugins_tests.rs index bc264b416..fe36af1c7 100644 --- a/crates/cli/tests/coverage/plugins_tests.rs +++ b/crates/cli/tests/coverage/plugins_tests.rs @@ -190,7 +190,15 @@ fn typed_editor_model_contains_observability_sections() { let atof = schema.field("atof").unwrap().schema().unwrap(); let atif = schema.field("atif").unwrap().schema().unwrap(); let openinference = schema.field("openinference").unwrap().schema().unwrap(); - assert!(atof.fields.iter().any(|field| field.name == "mode")); + let sinks = atof.field("sinks").expect("ATOF sinks field"); + assert_eq!(sinks.kind, EditorFieldKind::List); + assert_eq!( + sinks + .list_item + .and_then(|item| item.tagged_union) + .map(|union| union.discriminator), + Some("type") + ); assert!( atif.fields .iter() @@ -644,17 +652,21 @@ fn plugin_cancellation_paths_share_message() { fn plugin_menu_marks_configured_sections_and_fields() { let mut observability = ObservabilityConfig::default(); let atof = ObservabilityConfig::editor_schema().field("atof").unwrap(); - let mode = atof.schema().unwrap().field("mode").unwrap(); - let output_directory = atof.schema().unwrap().field("output_directory").unwrap(); + let sinks = atof.schema().unwrap().field("sinks").unwrap(); assert!(!section_configured(&observability, atof)); ensure_section(&mut observability, atof); assert!(section_configured(&observability, atof)); - assert!(!section_field_configured(&observability, atof, mode).unwrap()); - assert!(!section_field_configured(&observability, atof, output_directory).unwrap()); + assert!(!section_field_configured(&observability, atof, sinks).unwrap()); - set_section_field(&mut observability, atof, "output_directory", json!("logs")).unwrap(); - assert!(section_field_configured(&observability, atof, output_directory).unwrap()); + set_section_field( + &mut observability, + atof, + "sinks", + json!([{ "type": "file", "output_directory": "logs" }]), + ) + .unwrap(); + assert!(section_field_configured(&observability, atof, sinks).unwrap()); assert!(configured_label(true, "Edit ATOF").contains('✓')); assert!(!configured_label(false, "Edit ATIF").contains('✓')); } @@ -669,15 +681,12 @@ fn editor_model_renders_valid_observability_plugin_config() { set_section_field( &mut observability.config, atof, - "output_directory", - json!("logs"), - ) - .unwrap(); - set_section_field( - &mut observability.config, - atof, - "filename", - json!("events.jsonl"), + "sinks", + json!([{ + "type": "file", + "output_directory": "logs", + "filename": "events.jsonl" + }]), ) .unwrap(); store_observability_state(&mut config, &observability).unwrap(); @@ -760,7 +769,13 @@ fn typed_editor_serializes_explicit_observability_overrides() { let mut observability = ObservabilityConfig::default(); let atof = ObservabilityConfig::editor_schema().field("atof").unwrap(); toggle_section(&mut observability, atof); - set_section_field(&mut observability, atof, "output_directory", json!("logs")).unwrap(); + set_section_field( + &mut observability, + atof, + "sinks", + json!([{ "type": "file", "output_directory": "logs" }]), + ) + .unwrap(); let map = observability_config_map(&observability).unwrap(); let atof = map @@ -768,8 +783,14 @@ fn typed_editor_serializes_explicit_observability_overrides() { .and_then(Value::as_object) .expect("atof section is serialized"); assert_eq!(atof.get("enabled"), Some(&Value::Bool(true))); - assert_eq!(atof.get("output_directory"), Some(&json!("logs"))); - assert_eq!(atof.get("mode"), Some(&json!("append"))); + assert_eq!( + atof.get("sinks"), + Some(&json!([{ + "type": "file", + "output_directory": "logs", + "mode": "append" + }])) + ); assert!(map.contains_key("policy")); } @@ -799,11 +820,14 @@ fn editor_save_preserves_unknown_observability_fields() { kind: OBSERVABILITY_PLUGIN_KIND.to_string(), enabled: true, config: json!({ - "version": 1, + "version": 2, "future_top_level": "preserve", "atof": { "enabled": true, - "output_directory": "old-logs", + "sinks": [{ + "type": "file", + "output_directory": "old-logs" + }], "future_atof_field": "preserve" } }) @@ -815,12 +839,15 @@ fn editor_save_preserves_unknown_observability_fields() { }; let mut observability = component_observability_state(&config).unwrap(); let atof = ObservabilityConfig::editor_schema().field("atof").unwrap(); - remove_section_field(&mut observability.config, atof, "output_directory").unwrap(); set_section_field( &mut observability.config, atof, - "filename", - json!("events.jsonl"), + "sinks", + json!([{ + "type": "file", + "output_directory": "old-logs", + "filename": "events.jsonl" + }]), ) .unwrap(); @@ -844,8 +871,15 @@ fn editor_save_preserves_unknown_observability_fields() { atof_config.get("future_atof_field"), Some(&json!("preserve")) ); - assert_eq!(atof_config.get("filename"), Some(&json!("events.jsonl"))); - assert!(!atof_config.contains_key("output_directory")); + assert_eq!( + atof_config.get("sinks"), + Some(&json!([{ + "type": "file", + "output_directory": "old-logs", + "filename": "events.jsonl", + "mode": "append" + }])) + ); } #[test] @@ -1446,23 +1480,26 @@ fn reset_selected_field_accounts_for_section_toggle_offset() { let atof = ObservabilityConfig::editor_schema().field("atof").unwrap(); let fields = atof.schema().unwrap().fields; - set_section_field(&mut observability, atof, "output_directory", json!("logs")).unwrap(); + set_section_field( + &mut observability, + atof, + "sinks", + json!([{ "type": "file", "output_directory": "logs" }]), + ) + .unwrap(); assert!( - section_field_value(&observability, atof, "output_directory") + section_field_value(&observability, atof, "sinks") .unwrap() .is_some() ); - let output_directory_index = fields + let sinks_index = fields .iter() - .position(|field| field.name == "output_directory") + .position(|field| field.name == "sinks") .unwrap(); - assert!( - reset_selected_field(&mut observability, atof, fields, output_directory_index + 1,) - .unwrap() - ); + assert!(reset_selected_field(&mut observability, atof, fields, sinks_index + 1,).unwrap()); assert_eq!( - section_field_value(&observability, atof, "output_directory").unwrap(), + section_field_value(&observability, atof, "sinks").unwrap(), None ); assert!(!reset_selected_field(&mut observability, atof, fields, 0).unwrap()); @@ -1951,10 +1988,13 @@ fn validate_config_reports_plugin_diagnostics() { kind: OBSERVABILITY_PLUGIN_KIND.to_string(), enabled: true, config: json!({ - "version": 1, + "version": 2, "atof": { "enabled": true, - "mode": "not-a-mode" + "sinks": [{ + "type": "file", + "mode": "not-a-mode" + }] } }) .as_object() @@ -1970,7 +2010,7 @@ fn validate_config_reports_plugin_diagnostics() { error.contains("plugin validation failed"), "error was: {error}" ); - assert!(error.contains("ATOF mode"), "error was: {error}"); + assert!(error.contains("ATOF sinks[0].mode"), "error was: {error}"); } #[test] @@ -2248,14 +2288,11 @@ fn display_helpers_render_scalars_json_and_defaults() { assert_eq!(display_value(&json!({ "a": 1 })), r#"{"a":1}"#); let atof = ObservabilityConfig::editor_schema().field("atof").unwrap(); - let mode = atof.schema().unwrap().field("mode").unwrap(); - assert_eq!( - display_field_value(atof, mode, &json!("append")), - "append (default)" - ); + let sinks = atof.schema().unwrap().field("sinks").unwrap(); + assert_eq!(display_field_value(atof, sinks, &json!([])), "0 items"); assert_eq!( - display_field_value(atof, mode, &json!("overwrite")), - "overwrite" + display_field_value(atof, sinks, &json!([{ "type": "file" }])), + "1 item" ); } diff --git a/crates/cli/tests/coverage/server_tests.rs b/crates/cli/tests/coverage/server_tests.rs index b1161a444..a1311572d 100644 --- a/crates/cli/tests/coverage/server_tests.rs +++ b/crates/cli/tests/coverage/server_tests.rs @@ -587,12 +587,15 @@ async fn serve_listener_activates_plugin_config_and_clears_on_shutdown() { "kind": "observability", "enabled": true, "config": { - "version": 1, + "version": 2, "atof": { "enabled": true, - "output_directory": atof_dir, - "filename": "events.jsonl", - "mode": "overwrite" + "sinks": [{ + "type": "file", + "output_directory": atof_dir, + "filename": "events.jsonl", + "mode": "overwrite" + }] }, "atif": { "enabled": true, @@ -693,12 +696,15 @@ async fn serve_listener_observability_plugin_records_non_hermes_hooks() { "kind": "observability", "enabled": true, "config": { - "version": 1, + "version": 2, "atof": { "enabled": true, - "output_directory": atof_dir, - "filename": "events.jsonl", - "mode": "overwrite" + "sinks": [{ + "type": "file", + "output_directory": atof_dir, + "filename": "events.jsonl", + "mode": "overwrite" + }] } } } @@ -779,12 +785,15 @@ async fn serve_listener_hermes_api_hooks_write_atof_category_profile_and_fidelit "kind": "observability", "enabled": true, "config": { - "version": 1, + "version": 2, "atof": { "enabled": true, - "output_directory": atof_dir, - "filename": "events.jsonl", - "mode": "overwrite" + "sinks": [{ + "type": "file", + "output_directory": atof_dir, + "filename": "events.jsonl", + "mode": "overwrite" + }] } } } @@ -979,12 +988,15 @@ async fn serve_listener_hermes_api_request_error_writes_lossy_atof_error_event() "kind": "observability", "enabled": true, "config": { - "version": 1, + "version": 2, "atof": { "enabled": true, - "output_directory": atof_dir, - "filename": "events.jsonl", - "mode": "overwrite" + "sinks": [{ + "type": "file", + "output_directory": atof_dir, + "filename": "events.jsonl", + "mode": "overwrite" + }] } } } @@ -1125,12 +1137,15 @@ async fn serve_listener_hermes_post_tool_call_writes_atof_tool_events() { "kind": "observability", "enabled": true, "config": { - "version": 1, + "version": 2, "atof": { "enabled": true, - "output_directory": atof_dir, - "filename": "events.jsonl", - "mode": "overwrite" + "sinks": [{ + "type": "file", + "output_directory": atof_dir, + "filename": "events.jsonl", + "mode": "overwrite" + }] } } } @@ -1349,12 +1364,15 @@ async fn serve_listener_routed_gateway_wire_formats_write_atof_category_profile_ "kind": "observability", "enabled": true, "config": { - "version": 1, + "version": 2, "atof": { "enabled": true, - "output_directory": atof_dir, - "filename": "events.jsonl", - "mode": "overwrite" + "sinks": [{ + "type": "file", + "output_directory": atof_dir, + "filename": "events.jsonl", + "mode": "overwrite" + }] } } } @@ -1533,12 +1551,15 @@ async fn serve_listener_records_codex_stop_atof_contract() { "kind": "observability", "enabled": true, "config": { - "version": 1, + "version": 2, "atof": { "enabled": true, - "output_directory": atof_dir, - "filename": "events.jsonl", - "mode": "overwrite" + "sinks": [{ + "type": "file", + "output_directory": atof_dir, + "filename": "events.jsonl", + "mode": "overwrite" + }] } } } @@ -1827,10 +1848,13 @@ async fn serve_listener_rejects_invalid_plugin_config() { "kind": "observability", "enabled": true, "config": { - "version": 1, + "version": 2, "atof": { "enabled": true, - "mode": "invalid" + "sinks": [{ + "type": "file", + "mode": "invalid" + }] } } } @@ -1842,7 +1866,7 @@ async fn serve_listener_rejects_invalid_plugin_config() { .await .unwrap_err(); - assert!(error.to_string().contains("ATOF mode")); + assert!(error.to_string().contains("ATOF sinks[0].mode")); assert!(nemo_relay::plugin::active_plugin_report().is_none()); } diff --git a/crates/cli/tests/coverage/session_tests.rs b/crates/cli/tests/coverage/session_tests.rs index fc528af71..938cd296d 100644 --- a/crates/cli/tests/coverage/session_tests.rs +++ b/crates/cli/tests/coverage/session_tests.rs @@ -105,7 +105,7 @@ async fn install_test_atif_plugin(output_directory: &Path) { "kind": "observability", "enabled": true, "config": { - "version": 1, + "version": 2, "atif": { "enabled": true, "output_directory": output_directory, diff --git a/crates/core/src/observability/plugin_component.rs b/crates/core/src/observability/plugin_component.rs index bb7e307f2..53820bd71 100644 --- a/crates/core/src/observability/plugin_component.rs +++ b/crates/core/src/observability/plugin_component.rs @@ -2052,6 +2052,8 @@ fn validate_atof_stream_header( value: &str, ) { validate_atof_stream_header_name(diagnostics, policy, field, header); + #[cfg(not(feature = "atof-streaming"))] + let _ = value; #[cfg(feature = "atof-streaming")] if let Err(error) = reqwest::header::HeaderValue::from_str(value) { push_policy_diag( diff --git a/integrations/openclaw/test/live-smoke.test.ts b/integrations/openclaw/test/live-smoke.test.ts index 98026f40c..0b480cbdf 100644 --- a/integrations/openclaw/test/live-smoke.test.ts +++ b/integrations/openclaw/test/live-smoke.test.ts @@ -32,7 +32,7 @@ it( kind: 'observability', enabled: true, config: { - version: 1, + version: 2, atif: { enabled: true, agent_name: 'openclaw', From dbb11053371934cc446bf743da2f92860074d4d9 Mon Sep 17 00:00:00 2001 From: Will Killian Date: Tue, 14 Jul 2026 10:23:00 -0400 Subject: [PATCH 3/3] fix(observability): address ATOF sink review feedback Signed-off-by: Will Killian --- crates/core/src/observability/atof.rs | 15 ++++++++-- .../observability/plugin_component_tests.rs | 1 + crates/ffi/tests/integration/api_tests.rs | 4 +-- crates/python/src/py_types/observability.rs | 30 ++++++++++++------- go/nemo_relay/nemo_relay.go | 27 +++++++---------- go/nemo_relay/observability_plugin.go | 27 +++++++---------- python/tests/test_types.py | 9 +++++- 7 files changed, 62 insertions(+), 51 deletions(-) diff --git a/crates/core/src/observability/atof.rs b/crates/core/src/observability/atof.rs index fe978551a..d92671cf7 100644 --- a/crates/core/src/observability/atof.rs +++ b/crates/core/src/observability/atof.rs @@ -305,7 +305,10 @@ impl AtofExporterConfig { Self::default() } - /// Override the output directory. + /// Override the file sink output directory. + /// + /// This has no effect after selecting a stream sink with + /// [`Self::with_stream_sink`] or [`Self::with_endpoint`]. pub fn with_output_directory(mut self, output_directory: impl Into) -> Self { if let AtofSinkConfig::File(file) = &mut self.sink { file.output_directory = output_directory.into(); @@ -313,7 +316,10 @@ impl AtofExporterConfig { self } - /// Override the output mode. + /// Override the file sink output mode. + /// + /// This has no effect after selecting a stream sink with + /// [`Self::with_stream_sink`] or [`Self::with_endpoint`]. pub fn with_mode(mut self, mode: AtofExporterMode) -> Self { if let AtofSinkConfig::File(file) = &mut self.sink { file.mode = mode; @@ -321,7 +327,10 @@ impl AtofExporterConfig { self } - /// Override the output filename. + /// Override the file sink output filename. + /// + /// This has no effect after selecting a stream sink with + /// [`Self::with_stream_sink`] or [`Self::with_endpoint`]. pub fn with_filename(mut self, filename: impl Into) -> Self { if let AtofSinkConfig::File(file) = &mut self.sink { file.filename = filename.into(); diff --git a/crates/core/tests/unit/observability/plugin_component_tests.rs b/crates/core/tests/unit/observability/plugin_component_tests.rs index 56a790c10..cd4bebef2 100644 --- a/crates/core/tests/unit/observability/plugin_component_tests.rs +++ b/crates/core/tests/unit/observability/plugin_component_tests.rs @@ -372,6 +372,7 @@ fn schema_contains_every_supported_observability_option() { "type", "url", "field_name_policy", + "header_env", "agent_name", "agent_version", "model_name", diff --git a/crates/ffi/tests/integration/api_tests.rs b/crates/ffi/tests/integration/api_tests.rs index 5fd7ebde3..c819addaf 100644 --- a/crates/ffi/tests/integration/api_tests.rs +++ b/crates/ffi/tests/integration/api_tests.rs @@ -808,11 +808,11 @@ fn atof_exporter_create_from_json_reports_string_statuses() { NemoRelayStatus::InvalidJson ); - let invalid_endpoint = + let invalid_sink = cstring(r#"{"type":"stream","url":"http://localhost/events","transport":"websocket"}"#); assert_eq!( unsafe { - api::nemo_relay_atof_exporter_create_from_json(invalid_endpoint.as_ptr(), &mut exporter) + api::nemo_relay_atof_exporter_create_from_json(invalid_sink.as_ptr(), &mut exporter) }, NemoRelayStatus::InvalidArg ); diff --git a/crates/python/src/py_types/observability.rs b/crates/python/src/py_types/observability.rs index 8d7d20ada..f51ed5990 100644 --- a/crates/python/src/py_types/observability.rs +++ b/crates/python/src/py_types/observability.rs @@ -285,18 +285,26 @@ impl PyAtofExporterConfig { .with_output_directory(PathBuf::from(self.output_directory.clone())) .with_mode(self.mode.clone().into()) .with_filename(self.filename.clone())), - "stream" => PyAtofEndpointConfig { - url: self.url.clone(), - transport: self.transport.clone(), - headers: self.headers.clone(), - header_env: self.header_env.clone(), - timeout_millis: self.timeout_millis, - field_name_policy: self.field_name_policy.clone(), + "stream" => { + if self.url.trim().is_empty() { + return Err(pyo3::exceptions::PyValueError::new_err( + "stream sink requires url", + )); + } + PyAtofEndpointConfig { + url: self.url.clone(), + transport: self.transport.clone(), + headers: self.headers.clone(), + header_env: self.header_env.clone(), + timeout_millis: self.timeout_millis, + field_name_policy: self.field_name_policy.clone(), + } + .to_rust_config() + .map(|sink| { + nemo_relay::observability::atof::AtofExporterConfig::new() + .with_stream_sink(sink) + }) } - .to_rust_config() - .map(|sink| { - nemo_relay::observability::atof::AtofExporterConfig::new().with_stream_sink(sink) - }), _ => Err(pyo3::exceptions::PyValueError::new_err( "sink_type must be 'file' or 'stream'", )), diff --git a/go/nemo_relay/nemo_relay.go b/go/nemo_relay/nemo_relay.go index b3ab6f02b..b4b884ec2 100644 --- a/go/nemo_relay/nemo_relay.go +++ b/go/nemo_relay/nemo_relay.go @@ -1709,13 +1709,11 @@ func (AtofFileSinkConfig) atofExporterSink() {} // MarshalJSON serializes the fixed file sink discriminator. func (config AtofFileSinkConfig) MarshalJSON() ([]byte, error) { - type fileSinkJSON struct { - Type string `json:"type"` - OutputDirectory string `json:"output_directory,omitempty"` - Mode AtofExporterMode `json:"mode,omitempty"` - Filename string `json:"filename,omitempty"` - } - return json.Marshal(fileSinkJSON{"file", config.OutputDirectory, config.Mode, config.Filename}) + type alias AtofFileSinkConfig + return json.Marshal(struct { + Type string `json:"type"` + alias + }{Type: "file", alias: alias(config)}) } // AtofEndpointTransport controls how an ATOF streaming endpoint receives events. @@ -1754,16 +1752,11 @@ func (AtofStreamSinkConfig) atofExporterSink() {} // MarshalJSON serializes the fixed stream sink discriminator. func (config AtofStreamSinkConfig) MarshalJSON() ([]byte, error) { - type streamSinkJSON struct { - Type string `json:"type"` - URL string `json:"url"` - Transport AtofEndpointTransport `json:"transport,omitempty"` - Headers map[string]string `json:"headers,omitempty"` - HeaderEnv map[string]string `json:"header_env,omitempty"` - TimeoutMillis uint64 `json:"timeout_millis,omitempty"` - FieldNamePolicy AtofEndpointFieldNamePolicy `json:"field_name_policy,omitempty"` - } - return json.Marshal(streamSinkJSON{"stream", config.URL, config.Transport, config.Headers, config.HeaderEnv, config.TimeoutMillis, config.FieldNamePolicy}) + type alias AtofStreamSinkConfig + return json.Marshal(struct { + Type string `json:"type"` + alias + }{Type: "stream", alias: alias(config)}) } // NewAtofExporterConfig returns a config initialized with native defaults. diff --git a/go/nemo_relay/observability_plugin.go b/go/nemo_relay/observability_plugin.go index ae731cf4b..b25baee77 100644 --- a/go/nemo_relay/observability_plugin.go +++ b/go/nemo_relay/observability_plugin.go @@ -52,13 +52,11 @@ func (ObservabilityAtofFileSinkConfig) atofSinkConfig() {} // MarshalJSON serializes the fixed file sink discriminator. func (config ObservabilityAtofFileSinkConfig) MarshalJSON() ([]byte, error) { - type fileSinkJSON struct { - Type string `json:"type"` - OutputDirectory string `json:"output_directory,omitempty"` - Filename string `json:"filename,omitempty"` - Mode string `json:"mode,omitempty"` - } - return json.Marshal(fileSinkJSON{"file", config.OutputDirectory, config.Filename, config.Mode}) + type alias ObservabilityAtofFileSinkConfig + return json.Marshal(struct { + Type string `json:"type"` + alias + }{Type: "file", alias: alias(config)}) } // ObservabilityAtofStreamSinkConfig configures one remote ATOF destination. @@ -75,16 +73,11 @@ func (ObservabilityAtofStreamSinkConfig) atofSinkConfig() {} // MarshalJSON serializes the fixed stream sink discriminator. func (config ObservabilityAtofStreamSinkConfig) MarshalJSON() ([]byte, error) { - type streamSinkJSON struct { - Type string `json:"type"` - URL string `json:"url"` - Transport string `json:"transport,omitempty"` - Headers map[string]string `json:"headers,omitempty"` - HeaderEnv map[string]string `json:"header_env,omitempty"` - TimeoutMillis uint64 `json:"timeout_millis,omitempty"` - FieldNamePolicy string `json:"field_name_policy,omitempty"` - } - return json.Marshal(streamSinkJSON{"stream", config.URL, config.Transport, config.Headers, config.HeaderEnv, config.TimeoutMillis, config.FieldNamePolicy}) + type alias ObservabilityAtofStreamSinkConfig + return json.Marshal(struct { + Type string `json:"type"` + alias + }{Type: "stream", alias: alias(config)}) } // ObservabilityAtofEndpoint configures one streaming destination for raw ATOF events. diff --git a/python/tests/test_types.py b/python/tests/test_types.py index c66f658e9..9ef4a45b5 100644 --- a/python/tests/test_types.py +++ b/python/tests/test_types.py @@ -481,7 +481,14 @@ def test_config_defaults_mutation_and_repr(self, tmp_path): assert config.timeout_millis == 1000 assert config.field_name_policy == "replace_dots" - def test_endpoint_field_name_policy_is_validated(self, tmp_path): + def test_stream_sink_requires_url(self): + config = AtofExporterConfig() + config.sink_type = "stream" + + with pytest.raises(ValueError, match="stream sink requires url"): + AtofExporter(config) + + def test_endpoint_field_name_policy_is_validated(self): config = AtofExporterConfig() config.sink_type = "stream" config.url = "http://localhost:8080/events"