diff --git a/Cargo.lock b/Cargo.lock index b53a2f58f0..8eb762790a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2450,6 +2450,7 @@ dependencies = [ "figment", "futures", "lazy_static", + "logfmt", "mac_address", "metrics-endpoint", "rand 0.10.1", diff --git a/crates/api-integration-tests/tests/lib.rs b/crates/api-integration-tests/tests/lib.rs index 0731000c5a..d06d59f572 100644 --- a/crates/api-integration-tests/tests/lib.rs +++ b/crates/api-integration-tests/tests/lib.rs @@ -23,7 +23,7 @@ use std::time::{self, Duration}; use ::carbide_utils::HostPortPair; use ::machine_a_tron::{ - BmcMockRegistry, DeviceHandle, DhcpType, MachineATronConfig, MachineConfig, + BmcMockRegistry, DeviceHandle, DhcpType, LogFormat, MachineATronConfig, MachineConfig, }; use api_test_helper::utils::TestApiServerArgs; use api_test_helper::{ @@ -1403,6 +1403,7 @@ where carbide_api_url: format!("https://{}:{}", api_addr.ip(), api_addr.port()), dhcp: DhcpType::Api {}, log_file: None, + log_format: LogFormat::Compact, bmc_mock_port: 0, // unused, we're using dynamic ports on localhost bmc_mock_certs_dir: None, interface: String::from("UNUSED"), // unused, we're using dynamic ports on localhost diff --git a/crates/api-integration-tests/tests/rack.rs b/crates/api-integration-tests/tests/rack.rs index 71592953f4..7b591886a3 100644 --- a/crates/api-integration-tests/tests/rack.rs +++ b/crates/api-integration-tests/tests/rack.rs @@ -28,7 +28,7 @@ use carbide_uuid::rack::{RackId, RackProfileId}; use eyre::ContextCompat; use futures::future::join_all; use machine_a_tron::{ - BmcMockRegistry, DhcpType, LenovoGb300RackConfig, MachineATronConfig, RackConfig, + BmcMockRegistry, DhcpType, LenovoGb300RackConfig, LogFormat, MachineATronConfig, RackConfig, RackModelConfig, WiwynnGb200RackConfig, }; use tokio_util::sync::CancellationToken; @@ -163,6 +163,7 @@ async fn run_machine_a_tron_racks_test( carbide_api_url: format!("https://{}:{}", api_addr.ip(), api_addr.port()), dhcp: DhcpType::Api {}, log_file: None, + log_format: LogFormat::Compact, bmc_mock_port: 0, bmc_mock_certs_dir: None, interface: String::from("UNUSED"), diff --git a/crates/machine-a-tron/Cargo.toml b/crates/machine-a-tron/Cargo.toml index 9c7a8e35f7..8a314871cf 100644 --- a/crates/machine-a-tron/Cargo.toml +++ b/crates/machine-a-tron/Cargo.toml @@ -34,6 +34,7 @@ path = "src/main.rs" [dependencies] clap = { features = ["derive", "env"], workspace = true } lazy_static = { workspace = true } +logfmt = { path = "../logfmt" } tracing = { workspace = true } tracing-subscriber = { features = ["env-filter"], workspace = true } uuid = { features = ["v4"], workspace = true } diff --git a/crates/machine-a-tron/config/mac.toml b/crates/machine-a-tron/config/mac.toml index cbb41fd776..55cb32c709 100644 --- a/crates/machine-a-tron/config/mac.toml +++ b/crates/machine-a-tron/config/mac.toml @@ -32,6 +32,10 @@ carbide_api_url = "https://127.0.0.1:1079" # can omit this if tui_enabled = false log_file = "/tmp/mat.log" +# Log format can be "compact" (the default) or "logfmt". Kubernetes deployments +# use logfmt on stdout so log collectors can preserve structured tracing fields. +log_format = "compact" + # Network interface to bind to when creating addresses for BMC mocks. BMC mocks # will be configured with whatever IP address carbide assigns them, so they will # assign new addresses to this interface via `ip address add`. diff --git a/crates/machine-a-tron/config/mat.toml b/crates/machine-a-tron/config/mat.toml index dfc4e0a2fa..2c31b582e6 100644 --- a/crates/machine-a-tron/config/mat.toml +++ b/crates/machine-a-tron/config/mat.toml @@ -31,6 +31,10 @@ carbide_api_url = "https://carbide-api.forge:443" # can omit this if tui_enabled = false log_file = "/tmp/mat.log" +# Log format can be "compact" (the default) or "logfmt". Kubernetes deployments +# use logfmt on stdout so log collectors can preserve structured tracing fields. +log_format = "compact" + # Network interface to bind to when creating addresses for BMC mocks. BMC mocks # will be configured with whatever IP address carbide assigns them, so they will # assign new addresses to this interface via `ip address add`. diff --git a/crates/machine-a-tron/src/config.rs b/crates/machine-a-tron/src/config.rs index 40b62095da..9212a40400 100644 --- a/crates/machine-a-tron/src/config.rs +++ b/crates/machine-a-tron/src/config.rs @@ -393,6 +393,14 @@ impl DpuFirmwareVersions { } } +#[derive(Clone, Copy, Debug, Default, Deserialize, Serialize, Eq, PartialEq)] +#[serde(rename_all = "lowercase")] +pub enum LogFormat { + #[default] + Compact, + Logfmt, +} + #[derive(Clone, Debug, Deserialize, Serialize, Eq, PartialEq)] pub struct MachineATronConfig { #[serde(default)] @@ -405,6 +413,9 @@ pub struct MachineATronConfig { pub machines: BTreeMap>, pub carbide_api_url: String, pub log_file: Option, + /// Format used for logs written to stdout or `log_file`. + #[serde(default)] + pub log_format: LogFormat, pub interface: String, /// How machine-a-tron obtains DHCP leases for BMCs and directly attached hosts. @@ -1246,6 +1257,45 @@ scout_run_interval = "5s" assert_eq!(rack_config().dhcp, DhcpType::Api {}); } + #[test] + fn log_format_configuration() { + #[derive(Deserialize)] + struct LoggingConfig { + #[serde(default)] + log_format: LogFormat, + } + + check_values( + [ + Check { + scenario: "format omitted", + input: "", + expect: Some(LogFormat::Compact), + }, + Check { + scenario: "compact format", + input: r#"log_format = "compact""#, + expect: Some(LogFormat::Compact), + }, + Check { + scenario: "logfmt format", + input: r#"log_format = "logfmt""#, + expect: Some(LogFormat::Logfmt), + }, + Check { + scenario: "unknown format", + input: r#"log_format = "json""#, + expect: None, + }, + ], + |serialized| { + toml::from_str::(serialized) + .ok() + .map(|config| config.log_format) + }, + ); + } + #[test] fn udp_relay_configuration_requires_all_addresses() { check_values( diff --git a/crates/machine-a-tron/src/lib.rs b/crates/machine-a-tron/src/lib.rs index c8d545ae4f..0003c3acb3 100644 --- a/crates/machine-a-tron/src/lib.rs +++ b/crates/machine-a-tron/src/lib.rs @@ -49,9 +49,9 @@ use std::time::{Duration, Instant}; pub use bmc_mock_wrapper::BmcMockRegistry; pub use config::{ - DhcpType, LenovoGb300RackConfig, MachineATronArgs, MachineATronConfig, MachineATronContext, - MachineConfig, PersistedDevice, PersistedDpuMachine, RackConfig, RackModelConfig, - WiwynnGb200RackConfig, + DhcpType, LenovoGb300RackConfig, LogFormat, MachineATronArgs, MachineATronConfig, + MachineATronContext, MachineConfig, PersistedDevice, PersistedDpuMachine, RackConfig, + RackModelConfig, WiwynnGb200RackConfig, }; pub use control_router::{ControlState, append as append_control_routes}; pub use device_handle::DeviceHandle; diff --git a/crates/machine-a-tron/src/logging.rs b/crates/machine-a-tron/src/logging.rs new file mode 100644 index 0000000000..6f2bd78e4e --- /dev/null +++ b/crates/machine-a-tron/src/logging.rs @@ -0,0 +1,142 @@ +/* + * SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +use std::error::Error; +use std::fs::File; +use std::io::{self, Write}; +use std::sync::Arc; + +use machine_a_tron::{LogFormat, TuiHostLogs}; +use tracing::Subscriber; +use tracing_subscriber::filter::{EnvFilter, LevelFilter}; +use tracing_subscriber::prelude::*; +use tracing_subscriber::registry::LookupSpan; +use tracing_subscriber::{fmt, registry}; + +#[derive(Clone)] +enum LogWriter { + Stdout, + File(Arc), +} + +impl LogWriter { + fn new(filename: Option<&str>) -> io::Result { + match filename { + Some(filename) => Ok(Self::File(Arc::new(File::create(filename)?))), + None => Ok(Self::Stdout), + } + } +} + +impl Write for LogWriter { + fn write(&mut self, buffer: &[u8]) -> io::Result { + match self { + Self::Stdout => io::stdout().write(buffer), + Self::File(file) => { + let mut file = file.as_ref(); + file.write(buffer) + } + } + } + + fn flush(&mut self) -> io::Result<()> { + match self { + Self::Stdout => io::stdout().flush(), + Self::File(file) => { + let mut file = file.as_ref(); + file.flush() + } + } + } +} + +pub(super) fn init_logging( + format: LogFormat, + filename: Option<&str>, + tui_host_logs: Option<&TuiHostLogs>, +) -> Result<(), Box> { + let writer = LogWriter::new(filename)?; + let env_filter = env_filter(); + + match format { + LogFormat::Compact => registry() + .with( + fmt::Layer::default() + .compact() + .with_writer(move || writer.clone()), + ) + .with(env_filter) + .with(tui_host_logs.map(TuiHostLogs::make_tracing_layer)) + .try_init()?, + LogFormat::Logfmt => registry() + .with(logfmt_layer(writer)) + .with(env_filter) + .with(tui_host_logs.map(TuiHostLogs::make_tracing_layer)) + .try_init()?, + } + + Ok(()) +} + +fn logfmt_layer(writer: LogWriter) -> logfmt::LogFmtLayer +where + S: Subscriber + for<'span> LookupSpan<'span>, +{ + logfmt::layer() + .with_event_fields([logfmt::EventField::with_default( + "component", + "nico-machine-a-tron", + )]) + .with_writer(Arc::new(move || Box::new(writer.clone()))) +} + +fn env_filter() -> EnvFilter { + EnvFilter::builder() + .with_default_directive(LevelFilter::INFO.into()) + .from_env_lossy() + .add_directive("tower=warn".parse().unwrap()) + .add_directive("rustls=warn".parse().unwrap()) + .add_directive("hyper=warn".parse().unwrap()) + .add_directive("hickory_proto=warn".parse().unwrap()) + .add_directive("hickory_resolver=warn".parse().unwrap()) + .add_directive("h2=warn".parse().unwrap()) +} + +#[cfg(test)] +mod tests { + use std::fs; + + use tempfile::NamedTempFile; + + use super::*; + + #[test] + fn logfmt_output_contains_structured_fields() { + let output = NamedTempFile::new().unwrap(); + let writer = LogWriter::new(Some(output.path().to_str().unwrap())).unwrap(); + let subscriber = registry().with(logfmt_layer(writer)); + + tracing::subscriber::with_default(subscriber, || { + tracing::info!(answer = 42, "hello from machine-a-tron"); + }); + + let output = fs::read_to_string(output.path()).unwrap(); + assert!(output.starts_with( + "level=INFO component=nico-machine-a-tron msg=\"hello from machine-a-tron\" answer=42" + )); + } +} diff --git a/crates/machine-a-tron/src/main.rs b/crates/machine-a-tron/src/main.rs index 9f3494392a..34169b03c9 100644 --- a/crates/machine-a-tron/src/main.rs +++ b/crates/machine-a-tron/src/main.rs @@ -16,6 +16,7 @@ */ #![cfg_attr(not(test), deny(dead_code_pub_in_binary))] +mod logging; mod ufm_mock; use std::borrow::Cow; @@ -46,45 +47,10 @@ use rpc::forge_tls_client::{ApiConfig, ForgeClientConfig}; use rpc::protos::forge_api_client::ForgeApiClient; use tokio::signal::unix::{SignalKind, signal}; use tokio::sync::mpsc; -use tracing_subscriber::filter::{EnvFilter, LevelFilter}; -use tracing_subscriber::prelude::*; -use tracing_subscriber::{fmt, registry}; +use crate::logging::init_logging; use crate::ufm_mock::HostedUfmMock; -fn init_log( - filename: &Option, - tui_host_logs: Option<&TuiHostLogs>, -) -> Result<(), Box> { - let env_filter = EnvFilter::builder() - .with_default_directive(LevelFilter::INFO.into()) - .from_env_lossy() - .add_directive("tower=warn".parse().unwrap()) - .add_directive("rustls=warn".parse().unwrap()) - .add_directive("hyper=warn".parse().unwrap()) - .add_directive("hickory_proto=warn".parse().unwrap()) - .add_directive("hickory_resolver=warn".parse().unwrap()) - .add_directive("h2=warn".parse().unwrap()); - - match filename { - Some(filename) => { - let log_file = std::sync::Arc::new(std::fs::File::create(filename)?); - registry() - .with(fmt::Layer::default().compact().with_writer(log_file)) - .with(env_filter) - .with(tui_host_logs.map(|l| l.make_tracing_layer())) - .try_init()?; - } - None => registry() - .with(fmt::Layer::default().compact().with_writer(std::io::stdout)) - .with(env_filter) - .with(tui_host_logs.map(|l| l.make_tracing_layer())) - .try_init()?, - } - - Ok(()) -} - #[tokio::main(flavor = "multi_thread", worker_threads = 32)] async fn main() -> Result<(), Box> { let args = MachineATronArgs::parse(); @@ -102,7 +68,11 @@ async fn main() -> Result<(), Box> { None }; - init_log(&app_config.log_file, tui_host_logs.as_ref())?; + init_logging( + app_config.log_format, + app_config.log_file.as_deref(), + tui_host_logs.as_ref(), + )?; let file_config = get_config_from_file(); diff --git a/dev/docker-env/mat.toml b/dev/docker-env/mat.toml index 535b64f1e3..6bf140fc2f 100644 --- a/dev/docker-env/mat.toml +++ b/dev/docker-env/mat.toml @@ -31,6 +31,10 @@ carbide_api_url = "https://127.0.0.1:1079" # can omit this if tui_enabled = false log_file = "/tmp/mat.log" +# Log format can be "compact" (the default) or "logfmt". Kubernetes deployments +# use logfmt on stdout so log collectors can preserve structured tracing fields. +log_format = "compact" + # Network interface to bind to when creating addresses for BMC mocks. BMC mocks # will be configured with whatever IP address carbide assigns them, so they will # assign new addresses to this interface via `ip address add`. diff --git a/helm/charts/nico-machine-a-tron/README.md b/helm/charts/nico-machine-a-tron/README.md index c780b00267..b2900a07f5 100644 --- a/helm/charts/nico-machine-a-tron/README.md +++ b/helm/charts/nico-machine-a-tron/README.md @@ -60,6 +60,14 @@ The chart does not aggregate InfiniBand inventory across multiple machine-a-tron pods. A full `configFiles.matConfigs` override owns the complete MAT configuration, including its `[ufm_mock]` section. +## Logging + +The chart defaults `machineATron.logFormat` to `logfmt`, so machine-a-tron emits +structured logs to stdout for Kubernetes log collectors. Set it to `compact` +for the human-oriented tracing format. `machineATron.logFile` independently +redirects either format to a file when set. A full `configFiles.matConfigs` +override must set `log_format = "logfmt"` itself if structured output is wanted. + --- ## Mode 1: Override Mode (Development) diff --git a/helm/charts/nico-machine-a-tron/templates/configmap.yaml b/helm/charts/nico-machine-a-tron/templates/configmap.yaml index 7e6e5f3ef4..f77e924144 100644 --- a/helm/charts/nico-machine-a-tron/templates/configmap.yaml +++ b/helm/charts/nico-machine-a-tron/templates/configmap.yaml @@ -75,6 +75,7 @@ data: carbide_api_url = {{ $root.Values.machineATron.nicoApiUrl | quote }} interface = {{ $root.Values.machineATron.interface | quote }} tui_enabled = {{ $root.Values.machineATron.tuiEnabled }} + log_format = {{ $root.Values.machineATron.logFormat | quote }} {{- if $root.Values.machineATron.logFile }} log_file = {{ $root.Values.machineATron.logFile | quote }} {{- end }} diff --git a/helm/charts/nico-machine-a-tron/tests/configmap_test.yaml b/helm/charts/nico-machine-a-tron/tests/configmap_test.yaml index 6ad7f87d37..79fb308eba 100644 --- a/helm/charts/nico-machine-a-tron/tests/configmap_test.yaml +++ b/helm/charts/nico-machine-a-tron/tests/configmap_test.yaml @@ -15,6 +15,18 @@ tests: - matchRegex: path: data["mat.toml"] pattern: "use_single_bmc_mock = true" + - matchRegex: + path: data["mat.toml"] + pattern: 'log_format = "logfmt"' + + - it: should allow compact log output + set: + machineATron: + logFormat: compact + asserts: + - matchRegex: + path: data["mat.toml"] + pattern: 'log_format = "compact"' - it: should configure custom host count via pods structure set: diff --git a/helm/charts/nico-machine-a-tron/values.yaml b/helm/charts/nico-machine-a-tron/values.yaml index c536f4c840..f70d96c82c 100644 --- a/helm/charts/nico-machine-a-tron/values.yaml +++ b/helm/charts/nico-machine-a-tron/values.yaml @@ -153,6 +153,8 @@ machineATron: interface: "NOTUSED" tuiEnabled: false logFile: "" + # Kubernetes log collectors consume structured logs from stdout. + logFormat: "logfmt" useSingleBmcMock: true mockBmcSshServer: true mockBmcSshPort: 2222 @@ -348,6 +350,7 @@ configFiles: # carbide_api_url = "https://nico-api.nico-system.svc.cluster.local:1079" # interface = "NOTUSED" # tui_enabled = false + # log_format = "logfmt" # bmc_mock_port = 1266 # use_single_bmc_mock = true # mock_bmc_ssh_server = true