diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b2a76e523d..df4b2f973f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,12 @@ and this project adheres to ### Added +- [#5983](https://github.com/firecracker-microvm/firecracker/pull/5983): Add two + optional metrics fields, set via `PUT /metrics` or a config file: `emit_id` + emits the microVM instance id under a top-level `id` field, and `properties` + emits operator-defined key-value pairs under a top-level `properties` field. + Each is opt-in and independent. See [metrics documentation](docs/metrics.md). + ### Changed ### Deprecated diff --git a/docs/metrics.md b/docs/metrics.md index 98047e19f55..4544a250764 100644 --- a/docs/metrics.md +++ b/docs/metrics.md @@ -47,6 +47,79 @@ Details about this configuration can be found in the The metrics are written to the `metrics_path` in JSON format. +## Optional metrics fields + +Firecracker can emit additional top-level fields on every metrics line. Each is +opt-in and off by default, so the default output is unchanged. They are +configured alongside `metrics_path`, through the `PUT /metrics` API request or +the `metrics` block of a configuration file; the `--metrics-path` CLI option +configures only the path. Like the rest of the metrics configuration, they are +set once before boot and fixed for the lifetime of the microVM. + +### `id` + +Set `emit_id` to `true` to emit the microVM instance id (the value passed to +`--id`, defaulting to `anonymous-instance`) under a top-level `id` field. This +lets lines collected from multiple microVMs into one destination be attributed +to their source. + +### `properties` + +Set `properties` to a map of operator-defined key-value pairs to emit them under +a top-level `properties` field. The map is bounded: + +- up to 10 entries +- keys up to 64 bytes +- values up to 512 bytes + +### Examples + +Configure the fields via the API by adding them to the request body: + +```bash +curl --unix-socket /tmp/firecracker.socket -i \ + -X PUT "http://localhost/metrics" \ + -H "accept: application/json" \ + -H "Content-Type: application/json" \ + -d "{ + \"metrics_path\": \"metrics.fifo\", + \"emit_id\": true, + \"properties\": { + \"customer_id\": \"1234\", + \"bundle_id\": \"fn-abc\" + } + }" +``` + +The same fields can be provided in the `metrics` block of a configuration file +passed via `--config-file`: + +```json +{ + "metrics": { + "metrics_path": "metrics.fifo", + "emit_id": true, + "properties": { + "customer_id": "1234", + "bundle_id": "fn-abc" + } + } +} +``` + +With neither field configured, a line carries only the default keys: + +```json +{"utc_timestamp_ms": 1739000000000, "api_server": {"...": 0}} +``` + +With both fields configured as above, the same line also carries the instance id +and the properties: + +```json +{"utc_timestamp_ms": 1739000000000, "id": "my-instance", "properties": {"bundle_id": "fn-abc", "customer_id": "1234"}, "api_server": {"...": 0}} +``` + ## Flushing the metrics The metrics get flushed in two ways: diff --git a/src/firecracker/src/api_server/request/metrics.rs b/src/firecracker/src/api_server/request/metrics.rs index 054ece19422..8762d21c779 100644 --- a/src/firecracker/src/api_server/request/metrics.rs +++ b/src/firecracker/src/api_server/request/metrics.rs @@ -31,6 +31,8 @@ mod tests { }"#; let expected_config = MetricsConfig { metrics_path: PathBuf::from("metrics"), + emit_id: false, + properties: None, }; assert_eq!( vmm_action_from_request(parse_put_metrics(&Body::new(body)).unwrap()), @@ -42,4 +44,44 @@ mod tests { }"#; parse_put_metrics(&Body::new(invalid_body)).unwrap_err(); } + + #[test] + fn test_parse_put_metrics_request_with_emit_id() { + let body = r#"{ + "metrics_path": "metrics", + "emit_id": true + }"#; + let expected_config = MetricsConfig { + metrics_path: PathBuf::from("metrics"), + emit_id: true, + properties: None, + }; + assert_eq!( + vmm_action_from_request(parse_put_metrics(&Body::new(body)).unwrap()), + VmmAction::ConfigureMetrics(expected_config) + ); + } + + #[test] + fn test_parse_put_metrics_request_with_properties() { + let body = r#"{ + "metrics_path": "metrics", + "properties": { + "customer_id": "1234", + "bundle_id": "fn-abc" + } + }"#; + let mut properties = std::collections::BTreeMap::new(); + properties.insert("customer_id".to_string(), "1234".to_string()); + properties.insert("bundle_id".to_string(), "fn-abc".to_string()); + let expected_config = MetricsConfig { + metrics_path: PathBuf::from("metrics"), + emit_id: false, + properties: Some(properties), + }; + assert_eq!( + vmm_action_from_request(parse_put_metrics(&Body::new(body)).unwrap()), + VmmAction::ConfigureMetrics(expected_config) + ); + } } diff --git a/src/firecracker/src/main.rs b/src/firecracker/src/main.rs index bfeda48f71f..8baa66ac6a8 100644 --- a/src/firecracker/src/main.rs +++ b/src/firecracker/src/main.rs @@ -371,6 +371,8 @@ fn main_exec() -> Result<(), MainError> { if let Some(metrics_path) = arguments.single_value("metrics-path") { let metrics_config = MetricsConfig { metrics_path: PathBuf::from(metrics_path), + emit_id: false, + properties: None, }; init_metrics(metrics_config).map_err(MainError::MetricsInitialization)?; } diff --git a/src/firecracker/swagger/firecracker.yaml b/src/firecracker/swagger/firecracker.yaml index d1ac91bdb6a..f342ad47434 100644 --- a/src/firecracker/swagger/firecracker.yaml +++ b/src/firecracker/swagger/firecracker.yaml @@ -1474,6 +1474,20 @@ definitions: metrics_path: type: string description: Path to the named pipe or file where the JSON-formatted metrics are flushed. + emit_id: + type: boolean + description: + Whether to emit the microVM instance id (the value passed to the + jailer "--id") as a top-level "id" field on each metrics line. + default: false + properties: + type: object + description: + Operator-defined key-value pairs emitted under a top-level + "properties" field on each metrics line. At most 10 entries, with + keys up to 64 bytes and values up to 512 bytes. + additionalProperties: + type: string MmdsConfig: type: object diff --git a/src/vmm/src/logger/metrics.rs b/src/vmm/src/logger/metrics.rs index ca96fe8b5b3..71f24f65250 100644 --- a/src/vmm/src/logger/metrics.rs +++ b/src/vmm/src/logger/metrics.rs @@ -31,6 +31,11 @@ //! named `block` which is in turn a serializable child structure collecting metrics for //! the block device such as `activate_fails`, `cfg_fails`, etc. //! +//! Besides the per-component metrics, two top-level fields can be emitted, controlled +//! independently via the metrics API or config file: `id`, the microVM instance id (the jailer +//! `--id`), enabled by `emit_id`; and `properties`, a map of operator-defined key-value pairs. +//! See the `InstanceIdField` and `MetricsProperties` structs. +//! //! # Limitations //! Metrics are only written to buffers. //! @@ -61,16 +66,18 @@ //! If if turns out this approach is not really what we want, it's pretty easy to resort to //! something else, while working behind the same interface. +use std::collections::BTreeMap; use std::fmt::Debug; use std::io::Write; use std::ops::Deref; -use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; use std::sync::{Mutex, OnceLock}; +use serde::ser::SerializeMap; use serde::{Serialize, Serializer}; use utils::time::{ClockType, get_time_ns, get_time_us}; -use super::FcLineWriter; +use super::{DEFAULT_INSTANCE_ID, FcLineWriter, INSTANCE_ID}; use crate::devices::legacy; use crate::devices::virtio::balloon::metrics as balloon_metrics; use crate::devices::virtio::block::virtio::metrics as block_metrics; @@ -332,6 +339,40 @@ impl ProcessTimeReporter { // are interested in. Whenever the name of a field differs from its ideal textual representation // in the serialized form, we can use the #[serde(rename = "name")] attribute to, well, rename it. +/// Operator-supplied key-value pairs emitted alongside the metrics. +#[derive(Debug, Default)] +pub struct MetricsProperties { + /// The properties, set once at metrics configuration time. + inner: OnceLock>, +} + +impl Serialize for MetricsProperties { + /// Serializes as a flattened `properties` field when set, or as nothing when unset. + fn serialize(&self, serializer: S) -> Result { + let mut map = serializer.serialize_map(None)?; + if let Some(props) = self.inner.get() { + map.serialize_entry("properties", props)?; + } + map.end() + } +} + +impl MetricsProperties { + /// Const default construction. + pub const fn new() -> Self { + Self { + inner: OnceLock::new(), + } + } + + /// Sets the properties once. Returns an error if already set. + pub fn set(&self, properties: BTreeMap) -> Result<(), MetricsError> { + self.inner + .set(properties) + .map_err(|_| MetricsError::AlreadyInitialized) + } +} + /// Metrics related to the internal API server. #[derive(Debug, Default, Serialize)] pub struct ApiServerMetrics { @@ -873,6 +914,48 @@ impl Serialize for SerializeToUtcTimestampMs { } } +/// The microVM instance id emitted alongside the metrics. +#[derive(Debug, Default)] +pub struct InstanceIdField { + /// Whether the `id` field should be emitted. Flipped once at metrics configuration time. + enabled: AtomicBool, +} + +impl Serialize for InstanceIdField { + /// Serializes as a flattened `id` field (the jailer `--id`) when enabled, or as nothing when + /// disabled. + fn serialize(&self, serializer: S) -> Result { + let mut map = serializer.serialize_map(None)?; + if self.enabled.load(Ordering::Relaxed) { + map.serialize_entry( + "id", + INSTANCE_ID + .get() + .map(String::as_str) + .unwrap_or(DEFAULT_INSTANCE_ID), + )?; + } + map.end() + } +} + +impl InstanceIdField { + /// Const default construction. + pub const fn new() -> Self { + Self { + enabled: AtomicBool::new(false), + } + } + + /// Enables emission of the `id` field. Returns an error if already enabled. + pub fn enable(&self) -> Result<(), MetricsError> { + if self.enabled.swap(true, Ordering::Relaxed) { + return Err(MetricsError::AlreadyInitialized); + } + Ok(()) + } +} + macro_rules! create_serialize_proxy { // By using the below structure in FirecrackerMetrics it is easy // to serialise Firecracker app_metrics as a single json object which @@ -907,6 +990,12 @@ create_serialize_proxy!(MemoryHotplugSerializeProxy, virtio_mem_metrics); #[derive(Debug, Default, Serialize)] pub struct FirecrackerMetrics { utc_timestamp_ms: SerializeToUtcTimestampMs, + #[serde(flatten)] + /// The microVM instance id (jailer `--id`), emitted when enabled. + pub id: InstanceIdField, + #[serde(flatten)] + /// Operator-supplied custom properties. + pub properties: MetricsProperties, /// API Server related metrics. pub api_server: ApiServerMetrics, #[serde(flatten)] @@ -966,6 +1055,8 @@ impl FirecrackerMetrics { pub const fn new() -> Self { Self { utc_timestamp_ms: SerializeToUtcTimestampMs::new(), + id: InstanceIdField::new(), + properties: MetricsProperties::new(), api_server: ApiServerMetrics::new(), balloon_ser: BalloonMetricsSerializeProxy {}, block_ser: BlockMetricsSerializeProxy {}, @@ -1028,6 +1119,54 @@ mod tests { m.init(LineWriter::new(f.into_file())).unwrap_err(); } + #[test] + fn test_instance_id_serialize_disabled() { + let id = InstanceIdField::new(); + assert_eq!(serde_json::to_string(&id).unwrap(), "{}"); + } + + #[test] + fn test_instance_id_serialize_enabled() { + let id = InstanceIdField::new(); + id.enable().unwrap(); + let out = serde_json::to_string(&id).unwrap(); + assert!(out.contains(r#""id":"#)); + } + + #[test] + fn test_instance_id_enable_once() { + let id = InstanceIdField::new(); + id.enable().unwrap(); + id.enable().unwrap_err(); + } + + #[test] + fn test_metrics_properties_serialize_unset() { + let props = MetricsProperties::new(); + assert_eq!(serde_json::to_string(&props).unwrap(), "{}"); + } + + #[test] + fn test_metrics_properties_serialize_set() { + let props = MetricsProperties::new(); + let mut map = BTreeMap::new(); + map.insert("customer_id".to_string(), "1234".to_string()); + map.insert("bundle_id".to_string(), "fn-abc".to_string()); + props.set(map).unwrap(); + + assert_eq!( + serde_json::to_string(&props).unwrap(), + r#"{"properties":{"bundle_id":"fn-abc","customer_id":"1234"}}"# + ); + } + + #[test] + fn test_metrics_properties_set_once() { + let props = MetricsProperties::new(); + props.set(BTreeMap::new()).unwrap(); + props.set(BTreeMap::new()).unwrap_err(); + } + #[test] fn test_shared_inc_metric() { let metric = Arc::new(SharedIncMetric::default()); diff --git a/src/vmm/src/rpc_interface.rs b/src/vmm/src/rpc_interface.rs index 7693b305f65..aea186995a1 100644 --- a/src/vmm/src/rpc_interface.rs +++ b/src/vmm/src/rpc_interface.rs @@ -1289,6 +1289,8 @@ mod tests { check_unsupported(runtime_request(VmmAction::ConfigureMetrics( MetricsConfig { metrics_path: PathBuf::new(), + emit_id: false, + properties: None, }, ))); check_unsupported(runtime_request(VmmAction::SetVsockDevice( diff --git a/src/vmm/src/vmm_config/metrics.rs b/src/vmm/src/vmm_config/metrics.rs index 38001661fc6..f3422e61c8d 100644 --- a/src/vmm/src/vmm_config/metrics.rs +++ b/src/vmm/src/vmm_config/metrics.rs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 //! Auxiliary module for configuring the metrics system. +use std::collections::BTreeMap; use std::path::PathBuf; use serde::{Deserialize, Serialize}; @@ -9,11 +10,24 @@ use serde::{Deserialize, Serialize}; use crate::logger::{FcLineWriter, METRICS}; use crate::utils::open_file_nonblock; +/// Maximum number of metrics properties that can be configured. +const MAX_PROPERTIES: usize = 10; +/// Maximum length (in bytes) of a metrics property key. +const MAX_KEY_LEN: usize = 64; +/// Maximum length (in bytes) of a metrics property value. +const MAX_VALUE_LEN: usize = 512; + /// Strongly typed structure used to describe the metrics system. #[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)] pub struct MetricsConfig { /// Named pipe or file used as output for metrics. pub metrics_path: PathBuf, + /// Whether to emit the microVM instance id as a top-level `id` field on every metrics line. + #[serde(default)] + pub emit_id: bool, + /// Operator-defined key-value properties emitted on every metrics line. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub properties: Option>, } /// Errors associated with actions on the `MetricsConfig`. @@ -21,17 +35,62 @@ pub struct MetricsConfig { pub enum MetricsConfigError { /// Cannot initialize the metrics system due to bad user input: {0} InitializationFailure(String), + /// Too many metrics properties: {0} (maximum is {1}). + TooManyProperties(usize, usize), + /// A metrics property key exceeds the maximum length of {0} bytes. + KeyTooLong(usize), + /// A metrics property value exceeds the maximum length of {0} bytes. + ValueTooLong(usize), +} + +/// Validates operator-supplied metrics properties against the configured size limits. +fn validate_properties(properties: &BTreeMap) -> Result<(), MetricsConfigError> { + if properties.len() > MAX_PROPERTIES { + return Err(MetricsConfigError::TooManyProperties( + properties.len(), + MAX_PROPERTIES, + )); + } + for (key, value) in properties { + if key.len() > MAX_KEY_LEN { + return Err(MetricsConfigError::KeyTooLong(MAX_KEY_LEN)); + } + if value.len() > MAX_VALUE_LEN { + return Err(MetricsConfigError::ValueTooLong(MAX_VALUE_LEN)); + } + } + Ok(()) } /// Configures the metrics as described in `metrics_cfg`. pub fn init_metrics(metrics_cfg: MetricsConfig) -> Result<(), MetricsConfigError> { + if let Some(properties) = &metrics_cfg.properties { + validate_properties(properties)?; + } + let writer = FcLineWriter::new( open_file_nonblock(&metrics_cfg.metrics_path) .map_err(|err| MetricsConfigError::InitializationFailure(err.to_string()))?, ); METRICS .init(writer) - .map_err(|err| MetricsConfigError::InitializationFailure(err.to_string())) + .map_err(|err| MetricsConfigError::InitializationFailure(err.to_string()))?; + + if metrics_cfg.emit_id { + METRICS + .id + .enable() + .map_err(|err| MetricsConfigError::InitializationFailure(err.to_string()))?; + } + + if let Some(properties) = metrics_cfg.properties { + METRICS + .properties + .set(properties) + .map_err(|err| MetricsConfigError::InitializationFailure(err.to_string()))?; + } + + Ok(()) } #[cfg(test)] @@ -42,13 +101,51 @@ mod tests { #[test] fn test_init_metrics() { - // Initializing metrics with valid pipe is ok. + // Initializing metrics with a valid pipe is ok. Enable both optional + // fields so their configuration paths are exercised. let metrics_file = TempFile::new().unwrap(); + let mut properties = BTreeMap::new(); + properties.insert("customer_id".to_string(), "1234".to_string()); let desc = MetricsConfig { metrics_path: metrics_file.as_path().to_path_buf(), + emit_id: true, + properties: Some(properties), }; init_metrics(desc.clone()).unwrap(); + // Metrics can only be initialized once. init_metrics(desc).unwrap_err(); } + + #[test] + fn test_validate_properties() { + let mut props = BTreeMap::new(); + props.insert("customer_id".to_string(), "1234".to_string()); + props.insert("bundle_id".to_string(), "fn-abc".to_string()); + validate_properties(&props).unwrap(); + + let too_many: BTreeMap = (0..=MAX_PROPERTIES) + .map(|i| (i.to_string(), "v".to_string())) + .collect(); + validate_properties(&too_many).unwrap_err(); + + let mut long_key = BTreeMap::new(); + long_key.insert("k".repeat(MAX_KEY_LEN + 1), "v".to_string()); + validate_properties(&long_key).unwrap_err(); + + let mut long_value = BTreeMap::new(); + long_value.insert("k".to_string(), "v".repeat(MAX_VALUE_LEN + 1)); + validate_properties(&long_value).unwrap_err(); + } + + #[test] + fn test_emit_id_defaults_to_false() { + let cfg: MetricsConfig = serde_json::from_str(r#"{"metrics_path": "metrics"}"#).unwrap(); + assert!(!cfg.emit_id); + assert_eq!(cfg.properties, None); + + let cfg: MetricsConfig = + serde_json::from_str(r#"{"metrics_path": "metrics", "emit_id": true}"#).unwrap(); + assert!(cfg.emit_id); + } } diff --git a/tests/host_tools/fcmetrics.py b/tests/host_tools/fcmetrics.py index dbbc9c4f24a..aefdb85e2eb 100644 --- a/tests/host_tools/fcmetrics.py +++ b/tests/host_tools/fcmetrics.py @@ -376,6 +376,16 @@ def validate_fc_metrics(metrics): firecracker_metrics_schema = create_metrics_schema_objects(firecracker_metrics) + # "id" and "properties" are optional non-metric fields (see + # MetricsConfig.emit_id and MetricsConfig.properties): allow them when + # present, never require them, and keep every real metric strictly + # validated. + firecracker_metrics_schema["properties"]["id"] = {"type": "string"} + firecracker_metrics_schema["properties"]["properties"] = { + "type": "object", + "additionalProperties": {"type": "string"}, + } + jsonschema.validate(instance=metrics, schema=firecracker_metrics_schema) def validate_missing_metrics(metrics): diff --git a/tests/integration_tests/functional/test_metrics.py b/tests/integration_tests/functional/test_metrics.py index 029e63f6df4..9a7989b1a9b 100644 --- a/tests/integration_tests/functional/test_metrics.py +++ b/tests/integration_tests/functional/test_metrics.py @@ -3,6 +3,7 @@ """Tests the metrics system.""" import os +from pathlib import Path import host_tools.drive as drive_tools from framework.artifacts import GUEST_KERNEL_DEFAULT, pin_guest_kernel @@ -23,6 +24,83 @@ def test_flush_metrics(uvm): validate_fc_metrics(metrics) +def _configure_metrics_via_api(microvm, **kwargs): + """Configure metrics through the API rather than the `--metrics-path` CLI + option (the two ways of initializing metrics are mutually exclusive), and + wire up the host-side metrics file so the line can be read back.""" + microvm.spawn(metrics_path=None) + microvm.basic_config() + + metrics_path = Path(microvm.path) / "metrics.ndjson" + metrics_path.touch() + microvm.metrics_file = metrics_path + + microvm.api.metrics.put( + metrics_path=microvm.create_jailed_resource(metrics_path), **kwargs + ) + microvm.start() + + +@pin_guest_kernel(GUEST_KERNEL_DEFAULT) +def test_metrics_default_shape(uvm): + """ + Check that no `id` or `properties` field is emitted by default. + """ + microvm = uvm + microvm.spawn() + microvm.basic_config() + microvm.start() + + metrics = microvm.flush_metrics() + validate_fc_metrics(metrics) + assert "id" not in metrics + assert "properties" not in metrics + + +@pin_guest_kernel(GUEST_KERNEL_DEFAULT) +def test_metrics_emit_id(uvm): + """ + Check that `emit_id` emits the instance id, independently of `properties`. + """ + microvm = uvm + _configure_metrics_via_api(microvm, emit_id=True) + + metrics = microvm.flush_metrics() + validate_fc_metrics(metrics) + assert metrics["id"] == microvm.id + assert "properties" not in metrics + + +@pin_guest_kernel(GUEST_KERNEL_DEFAULT) +def test_metrics_with_properties(uvm): + """ + Check that `properties` is emitted, independently of `emit_id`. + """ + microvm = uvm + properties = {"customer_id": "1234", "bundle_id": "fn-abc"} + _configure_metrics_via_api(microvm, properties=properties) + + metrics = microvm.flush_metrics() + validate_fc_metrics(metrics) + assert metrics["properties"] == properties + assert "id" not in metrics + + +@pin_guest_kernel(GUEST_KERNEL_DEFAULT) +def test_metrics_emit_id_and_properties(uvm): + """ + Check that `emit_id` and `properties` can be enabled together. + """ + microvm = uvm + properties = {"customer_id": "1234", "bundle_id": "fn-abc"} + _configure_metrics_via_api(microvm, emit_id=True, properties=properties) + + metrics = microvm.flush_metrics() + validate_fc_metrics(metrics) + assert metrics["id"] == microvm.id + assert metrics["properties"] == properties + + @pin_guest_kernel(GUEST_KERNEL_DEFAULT) def test_net_metrics(uvm): """