Skip to content

nexus: Remove TimeseriesKey from timeseries output #8842

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 19, 2025
Merged

nexus: Remove TimeseriesKey from timeseries output #8842

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dfc2cf4
nexus: Remove TimeseriesKey from timeseries output
wfchandler Aug 14, 2025
e40c7c5
`Table` -> `OxqlTable` and move to views.rs (#8847)
david-crespo Aug 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions nexus/src/external_api/http_entrypoints.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6571,7 +6571,11 @@ impl NexusExternalApi for NexusExternalApiImpl {
nexus
.timeseries_query(&opctx, &query)
.await
.map(|tables| HttpResponseOk(views::OxqlQueryResult { tables }))
.map(|tables| {
HttpResponseOk(views::OxqlQueryResult {
tables: tables.into_iter().map(Into::into).collect(),
})
})
.map_err(HttpError::from)
};
apictx
Expand All @@ -6598,7 +6602,11 @@ impl NexusExternalApi for NexusExternalApiImpl {
nexus
.timeseries_query_project(&opctx, &project_lookup, &query)
.await
.map(|tables| HttpResponseOk(views::OxqlQueryResult { tables }))
.map(|tables| {
HttpResponseOk(views::OxqlQueryResult {
tables: tables.into_iter().map(Into::into).collect(),
})
})
.map_err(HttpError::from)
};
apictx
Expand Down
55 changes: 28 additions & 27 deletions nexus/tests/integration_tests/metrics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -270,17 +270,18 @@ async fn test_instance_watcher_metrics(

#[track_caller]
fn count_state(
table: &oxql_types::Table,
table: &views::OxqlTable,
instance_id: InstanceUuid,
state: &'static str,
) -> Result<i64, MetricsNotYet> {
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\
Expand Down Expand Up @@ -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")
})?;
Expand All @@ -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)?);
Expand All @@ -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)?);
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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)?);
Expand Down Expand Up @@ -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",
));
Expand All @@ -511,23 +512,23 @@ 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);

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 =
Expand All @@ -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!(
Expand All @@ -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!(
Expand All @@ -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());

Expand Down Expand Up @@ -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::<OxqlQueryResult>()
.execute_and_parse_unwrap::<views::OxqlQueryResult>()
.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]
Expand Down Expand Up @@ -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}",
Expand All @@ -791,7 +792,7 @@ async fn test_mgs_metrics(
.keys()
.map(|serial| (serial.clone(), 0))
.collect::<HashMap<_, usize>>();
for timeseries in table.timeseries() {
for timeseries in &table.timeseries {
let fields = &timeseries.fields;
if timeseries.points.is_empty() {
return Err(MetricsNotYet::new(format!(
Expand Down
14 changes: 7 additions & 7 deletions nexus/tests/integration_tests/metrics_querier.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -93,7 +93,7 @@ impl<'a, N> MetricsQuerier<'a, N> {
cond: F,
) -> T
where
F: Fn(Vec<oxql_types::Table>) -> Result<T, MetricsNotYet>,
F: Fn(Vec<views::OxqlTable>) -> Result<T, MetricsNotYet>,
{
self.timeseries_query_until("/v1/system/timeseries/query", query, cond)
.await
Expand All @@ -108,7 +108,7 @@ impl<'a, N> MetricsQuerier<'a, N> {
cond: F,
) -> T
where
F: Fn(Vec<oxql_types::Table>) -> Result<T, MetricsNotYet>,
F: Fn(Vec<views::OxqlTable>) -> Result<T, MetricsNotYet>,
{
self.timeseries_query_until(
&format!("/v1/timeseries/query?project={project}"),
Expand All @@ -128,7 +128,7 @@ impl<'a, N> MetricsQuerier<'a, N> {
&self,
project: &str,
query: &str,
) -> Vec<oxql_types::Table> {
) -> Vec<views::OxqlTable> {
self.project_timeseries_query_until(project, query, |tables| Ok(tables))
.await
}
Expand Down Expand Up @@ -270,7 +270,7 @@ impl<'a, N> MetricsQuerier<'a, N> {
cond: F,
) -> T
where
F: Fn(Vec<oxql_types::Table>) -> Result<T, MetricsNotYet>,
F: Fn(Vec<views::OxqlTable>) -> Result<T, MetricsNotYet>,
{
let result = wait_for_condition(
|| async {
Expand Down Expand Up @@ -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::<OxqlQueryResult>()
rsp.parsed_body::<views::OxqlQueryResult>()
.unwrap_or_else(|e| {
panic!(
"could not parse timeseries query response: {e:?}\n\
Expand All @@ -389,5 +389,5 @@ impl<'a, N> MetricsQuerier<'a, N> {

enum TimeseriesQueryResult {
TimeseriesNotFound,
Ok(Vec<oxql_types::Table>),
Ok(Vec<views::OxqlTable>),
}
50 changes: 49 additions & 1 deletion nexus/types/src/external_api/views.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<TimeseriesKey, Timeseries>`
// 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<oxql_types::Timeseries>,
}

impl From<oxql_types::Table> 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<oxql_types::Table>,
pub tables: Vec<OxqlTable>,
}

// ALERTS
Expand Down
42 changes: 22 additions & 20 deletions openapi/nexus.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22275,14 +22275,35 @@
"description": "Tables resulting from the query, each containing timeseries.",
"type": "array",
"items": {
"$ref": "#/components/schemas/Table"
"$ref": "#/components/schemas/OxqlTable"
}
}
},
"required": [
"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.",
Expand Down Expand Up @@ -25523,25 +25544,6 @@
"vlan_id"
]
},
"Table": {
"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": "object",
"additionalProperties": {
"$ref": "#/components/schemas/Timeseries"
}
}
},
"required": [
"name",
"timeseries"
]
},
"TargetRelease": {
"description": "View of a system software target release.",
"type": "object",
Expand Down
Loading