diff --git a/nexus/tests/integration_tests/metrics.rs b/nexus/tests/integration_tests/metrics.rs index 9ebb831dab6..8ba283c13e9 100644 --- a/nexus/tests/integration_tests/metrics.rs +++ b/nexus/tests/integration_tests/metrics.rs @@ -22,7 +22,7 @@ use nexus_test_utils::resource_helpers::{ use nexus_test_utils::wait_for_producer; use nexus_test_utils_macros::nexus_test; use nexus_types::external_api::shared::ProjectRole; -use nexus_types::external_api::views::OxqlQueryResult; +use nexus_types::external_api::views; use nexus_types::silo::DEFAULT_SILO_ID; use omicron_uuid_kinds::{GenericUuid, InstanceUuid}; use oximeter::TimeseriesSchema; @@ -270,17 +270,18 @@ async fn test_instance_watcher_metrics( #[track_caller] fn count_state( - table: &oxql_types::TableOutput, + table: &views::OxqlTable, instance_id: InstanceUuid, state: &'static str, ) -> Result { use oxql_types::point::ValueArray; let uuid = FieldValue::Uuid(instance_id.into_untyped_uuid()); let state = FieldValue::String(state.into()); - let mut timeserieses = table.timeseries().filter(|ts| { - ts.fields.get(INSTANCE_ID_FIELD) == Some(&uuid) - && ts.fields.get(STATE_FIELD) == Some(&state) - }); + let mut timeserieses = + table.timeseries.clone().into_iter().filter(|ts| { + ts.fields.get(INSTANCE_ID_FIELD) == Some(&uuid) + && ts.fields.get(STATE_FIELD) == Some(&state) + }); let timeseries = timeserieses.next().ok_or_else(|| { MetricsNotYet::new(format!( "missing timeseries for instance {instance_id}, state {state}\n\ @@ -325,7 +326,7 @@ async fn test_instance_watcher_metrics( .system_timeseries_query_until(OXQL_QUERY, |metrics| { let checks = metrics .iter() - .find(|t| t.name() == "virtual_machine:check") + .find(|t| t.name == "virtual_machine:check") .ok_or_else(|| { MetricsNotYet::new("missing virtual_machine:check") })?; @@ -348,7 +349,7 @@ async fn test_instance_watcher_metrics( .system_timeseries_query_until(OXQL_QUERY, |metrics| { let checks = metrics .iter() - .find(|t| t.name() == "virtual_machine:check") + .find(|t| t.name == "virtual_machine:check") .expect("missing virtual_machine:check"); let ts1 = dbg!(count_state(&checks, instance1_uuid, STATE_STARTING)?); @@ -371,7 +372,7 @@ async fn test_instance_watcher_metrics( .system_timeseries_query_until(OXQL_QUERY, |metrics| { let checks = metrics .iter() - .find(|t| t.name() == "virtual_machine:check") + .find(|t| t.name == "virtual_machine:check") .expect("missing virtual_machine:check"); let ts1_starting = dbg!(count_state(&checks, instance1_uuid, STATE_STARTING)?); @@ -402,7 +403,7 @@ async fn test_instance_watcher_metrics( .system_timeseries_query_until(OXQL_QUERY, |metrics| { let checks = metrics .iter() - .find(|t| t.name() == "virtual_machine:check") + .find(|t| t.name == "virtual_machine:check") .expect("missing virtual_machine:check"); let ts1_starting = @@ -437,7 +438,7 @@ async fn test_instance_watcher_metrics( .system_timeseries_query_until(OXQL_QUERY, |metrics| { let checks = metrics .iter() - .find(|t| t.name() == "virtual_machine:check") + .find(|t| t.name == "virtual_machine:check") .expect("missing virtual_machine:check"); let ts1_starting = dbg!(count_state(&checks, instance1_uuid, STATE_STARTING)?); @@ -497,7 +498,7 @@ async fn test_project_timeseries_query( return Err(MetricsNotYet::new("waiting for table creation")); } assert_eq!(result.len(), 1); - if result[0].timeseries().len() == 0 { + if result[0].timeseries.is_empty() { return Err(MetricsNotYet::new( "waiting for timeseries population", )); @@ -511,11 +512,11 @@ async fn test_project_timeseries_query( .project_timeseries_query(&p1.identity.id.to_string(), q1) .await; assert_eq!(result.len(), 1); - assert!(result[0].timeseries().len() > 0); + assert!(!result[0].timeseries.is_empty()); let result = metrics_querier.project_timeseries_query("project2", q1).await; assert_eq!(result.len(), 1); - assert!(result[0].timeseries().len() > 0); + assert!(!result[0].timeseries.is_empty()); // with project specified let q2 = &format!("{} | filter project_id == \"{}\"", q1, p1.identity.id); @@ -523,11 +524,11 @@ async fn test_project_timeseries_query( let result = metrics_querier.project_timeseries_query("project1", q2).await; assert_eq!(result.len(), 1); // we get 2 timeseries because there are two instances - assert!(result[0].timeseries().len() == 2); + assert!(result[0].timeseries.len() == 2); let result = metrics_querier.project_timeseries_query("project2", q2).await; assert_eq!(result.len(), 1); - assert_eq!(result[0].timeseries().len(), 0); + assert_eq!(result[0].timeseries.len(), 0); // with instance specified let q3 = @@ -536,12 +537,12 @@ async fn test_project_timeseries_query( // project containing instance gives me something let result = metrics_querier.project_timeseries_query("project1", q3).await; assert_eq!(result.len(), 1); - assert_eq!(result[0].timeseries().len(), 1); + assert_eq!(result[0].timeseries.len(), 1); // should be empty or error let result = metrics_querier.project_timeseries_query("project2", q3).await; assert_eq!(result.len(), 1); - assert_eq!(result[0].timeseries().len(), 0); + assert_eq!(result[0].timeseries.len(), 0); // now let's test it with group_by let q4 = &format!( @@ -550,7 +551,7 @@ async fn test_project_timeseries_query( ); let result = metrics_querier.project_timeseries_query("project1", q4).await; assert_eq!(result.len(), 1); - assert_eq!(result[0].timeseries().len(), 2); + assert_eq!(result[0].timeseries.len(), 2); // test with a nested query let q5 = &format!( @@ -565,13 +566,13 @@ async fn test_project_timeseries_query( // we get two results, each contains one timeseries, and the instance ID // on each corresponds to the one we requested assert_eq!(result.len(), 2); - assert_eq!(result[0].timeseries().len(), 1); - let timeseries = result[0].timeseries().next().unwrap(); + assert_eq!(result[0].timeseries.len(), 1); + let timeseries = result[0].timeseries[0].clone(); let instance_id = timeseries.fields.get("instance_id").unwrap().to_string(); assert_eq!(instance_id, i1p1.identity.id.to_string()); - assert_eq!(result[1].timeseries().len(), 1); - let timeseries = result[1].timeseries().next().unwrap(); + assert_eq!(result[1].timeseries.len(), 1); + let timeseries = &result[1].timeseries[0]; let instance_id = timeseries.fields.get("instance_id").unwrap().to_string(); assert_eq!(instance_id, i2p1.identity.id.to_string()); @@ -639,10 +640,10 @@ async fn test_project_timeseries_query( .expect_status(Some(StatusCode::OK)); let result = NexusRequest::new(request) .authn_as(AuthnMode::UnprivilegedUser) - .execute_and_parse_unwrap::() + .execute_and_parse_unwrap::() .await; assert_eq!(result.tables.len(), 1); - assert_eq!(result.tables[0].timeseries().len(), 2); // two instances + assert_eq!(result.tables[0].timeseries.len(), 2); // two instances } #[nexus_test] @@ -780,7 +781,7 @@ async fn test_mgs_metrics( querier.system_timeseries_query_until(&query, |tables| { let table = tables .into_iter() - .find(|t| t.name() == name) + .find(|t| t.name == *name) .ok_or_else(|| { MetricsNotYet::new(format!( "failed to find table for {query}", @@ -791,7 +792,7 @@ async fn test_mgs_metrics( .keys() .map(|serial| (serial.clone(), 0)) .collect::>(); - for timeseries in table.timeseries() { + for timeseries in &table.timeseries { let fields = ×eries.fields; if timeseries.points.is_empty() { return Err(MetricsNotYet::new(format!( diff --git a/nexus/tests/integration_tests/metrics_querier.rs b/nexus/tests/integration_tests/metrics_querier.rs index d898086de71..91c1eb5c7f7 100644 --- a/nexus/tests/integration_tests/metrics_querier.rs +++ b/nexus/tests/integration_tests/metrics_querier.rs @@ -13,7 +13,7 @@ use nexus_test_utils::http_testing::NexusRequest; use nexus_test_utils::http_testing::RequestBuilder; use nexus_test_utils::resource_helpers::objects_list_page_authz; use nexus_types::external_api::params; -use nexus_types::external_api::views::OxqlQueryResult; +use nexus_types::external_api::views; use omicron_test_utils::dev::poll; use omicron_test_utils::dev::poll::CondCheckError; use omicron_test_utils::dev::poll::wait_for_condition; @@ -93,7 +93,7 @@ impl<'a, N> MetricsQuerier<'a, N> { cond: F, ) -> T where - F: Fn(Vec) -> Result, + F: Fn(Vec) -> Result, { self.timeseries_query_until("/v1/system/timeseries/query", query, cond) .await @@ -108,7 +108,7 @@ impl<'a, N> MetricsQuerier<'a, N> { cond: F, ) -> T where - F: Fn(Vec) -> Result, + F: Fn(Vec) -> Result, { self.timeseries_query_until( &format!("/v1/timeseries/query?project={project}"), @@ -128,7 +128,7 @@ impl<'a, N> MetricsQuerier<'a, N> { &self, project: &str, query: &str, - ) -> Vec { + ) -> Vec { self.project_timeseries_query_until(project, query, |tables| Ok(tables)) .await } @@ -270,7 +270,7 @@ impl<'a, N> MetricsQuerier<'a, N> { cond: F, ) -> T where - F: Fn(Vec) -> Result, + F: Fn(Vec) -> Result, { let result = wait_for_condition( || async { @@ -375,7 +375,7 @@ impl<'a, N> MetricsQuerier<'a, N> { // Try to parse the query as usual, which will fail on other kinds of // errors. TimeseriesQueryResult::Ok( - rsp.parsed_body::() + rsp.parsed_body::() .unwrap_or_else(|e| { panic!( "could not parse timeseries query response: {e:?}\n\ @@ -389,5 +389,5 @@ impl<'a, N> MetricsQuerier<'a, N> { enum TimeseriesQueryResult { TimeseriesNotFound, - Ok(Vec), + Ok(Vec), } diff --git a/nexus/types/src/external_api/views.rs b/nexus/types/src/external_api/views.rs index d1a12a031eb..6cc509ebbb5 100644 --- a/nexus/types/src/external_api/views.rs +++ b/nexus/types/src/external_api/views.rs @@ -1113,11 +1113,59 @@ pub struct AllowList { // OxQL QUERIES +/// A table represents one or more timeseries with the same schema. +/// +/// A table is the result of an OxQL query. It contains a name, usually the name +/// of the timeseries schema from which the data is derived, and any number of +/// timeseries, which contain the actual data. +// +// # Motivation +// +// This struct is derived from [`oxql_types::Table`] but presents timeseries data as a `Vec` +// rather than a map keyed by [`TimeseriesKey`]. This provides a cleaner JSON +// representation for external consumers, as these numeric keys are ephemeral +// identifiers that have no meaning to API consumers. Key ordering is retained +// as this is contructed from the already sorted values present in [`Table`]. +// +// When serializing a [`Table`] to JSON, the `BTreeMap` +// structure produces output with numeric keys like: +// ```json +// { +// "timeseries": { +// "2352746367989923131": { ... }, +// "3940108470521992408": { ... } +// } +// } +// ``` +// +// The `Table` view instead serializes timeseries as an array: +// ```json +// { +// "timeseries": [ { ... }, { ... } ] +// } +// ``` +#[derive(Clone, Debug, Deserialize, JsonSchema, PartialEq, Serialize)] +pub struct OxqlTable { + /// The name of the table. + pub name: String, + /// The set of timeseries in the table, ordered by key. + pub timeseries: Vec, +} + +impl From for OxqlTable { + fn from(table: oxql_types::Table) -> Self { + OxqlTable { + name: table.name.clone(), + timeseries: table.into_iter().collect(), + } + } +} + /// The result of a successful OxQL query. #[derive(Clone, Debug, Deserialize, JsonSchema, Serialize)] pub struct OxqlQueryResult { /// Tables resulting from the query, each containing timeseries. - pub tables: Vec, + pub tables: Vec, } // ALERTS diff --git a/openapi/nexus.json b/openapi/nexus.json index b2da6589ab9..3722de21441 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -22275,7 +22275,7 @@ "description": "Tables resulting from the query, each containing timeseries.", "type": "array", "items": { - "$ref": "#/components/schemas/TableOutput" + "$ref": "#/components/schemas/OxqlTable" } } }, @@ -22283,6 +22283,27 @@ "tables" ] }, + "OxqlTable": { + "description": "A table represents one or more timeseries with the same schema.\n\nA table is the result of an OxQL query. It contains a name, usually the name of the timeseries schema from which the data is derived, and any number of timeseries, which contain the actual data.", + "type": "object", + "properties": { + "name": { + "description": "The name of the table.", + "type": "string" + }, + "timeseries": { + "description": "The set of timeseries in the table, ordered by key.", + "type": "array", + "items": { + "$ref": "#/components/schemas/Timeseries" + } + } + }, + "required": [ + "name", + "timeseries" + ] + }, "Password": { "title": "A password used to authenticate a user", "description": "Passwords may be subject to additional constraints.", @@ -25523,25 +25544,6 @@ "vlan_id" ] }, - "TableOutput": { - "description": "A table represents one or more timeseries with the same schema.\n\nA table is the result of an OxQL query. It contains a name, usually the name of the timeseries schema from which the data is derived, and any number of timeseries, which contain the actual data.", - "type": "object", - "properties": { - "name": { - "type": "string" - }, - "timeseries": { - "type": "array", - "items": { - "$ref": "#/components/schemas/Timeseries" - } - } - }, - "required": [ - "name", - "timeseries" - ] - }, "TargetRelease": { "description": "View of a system software target release.", "type": "object", diff --git a/oximeter/oxql-types/src/lib.rs b/oximeter/oxql-types/src/lib.rs index 498fc659d89..00468705a93 100644 --- a/oximeter/oxql-types/src/lib.rs +++ b/oximeter/oxql-types/src/lib.rs @@ -11,7 +11,6 @@ pub mod point; pub mod table; pub use self::table::Table; -pub use self::table::TableOutput; pub use self::table::Timeseries; /// Describes the time alignment for an OxQL query. diff --git a/oximeter/oxql-types/src/table.rs b/oximeter/oxql-types/src/table.rs index 38024009a9f..e6b857daa7e 100644 --- a/oximeter/oxql-types/src/table.rs +++ b/oximeter/oxql-types/src/table.rs @@ -360,61 +360,3 @@ impl Table { } } } - -/// A table represents one or more timeseries with the same schema. -/// -/// A table is the result of an OxQL query. It contains a name, usually the name -/// of the timeseries schema from which the data is derived, and any number of -/// timeseries, which contain the actual data. -// -// # Motivation -// -// This struct is derived from [`Table`] but presents timeseries data as a `Vec` -// rather than a map keyed by [`TimeseriesKey`]. This provides a cleaner JSON -// representation for external consumers, as these numeric keys are ephemeral -// identifiers that have no meaning to API consumers. Key ordering is retained -// as this is contructed from the already sorted values present in [`Table`]. -// -// When serializing a [`Table`] to JSON, the `BTreeMap` -// structure produces output with numeric keys like: -// ```json -// { -// "timeseries": { -// "2352746367989923131": { ... }, -// "3940108470521992408": { ... } -// } -// } -// ``` -// -// `TableOutput` instead serializes timeseries as an array: -// ```json -// { -// "timeseries": [ { ... }, { ... } ] -// } -// ``` -#[derive(Clone, Debug, Deserialize, JsonSchema, PartialEq, Serialize)] -pub struct TableOutput { - // The name of the table. - pub name: String, - // The set of timeseries in the table, ordered by key. - timeseries: Vec, -} - -impl From for TableOutput { - fn from(table: Table) -> Self { - let timeseries: Vec<_> = table.timeseries.into_values().collect(); - TableOutput { name: table.name, timeseries } - } -} - -impl TableOutput { - /// Return the name of the table. - pub fn name(&self) -> &str { - self.name.as_str() - } - - /// Return the list of timeseries in this table, ordered by key. - pub fn timeseries(&self) -> impl ExactSizeIterator { - self.timeseries.iter() - } -}