Skip to content

Commit 778d6e6

Browse files
wfchandlerdavid-crespo
wfchandler
and
david-crespo
authored
nexus: Remove TimeseriesKey from timeseries output (#8842)
Our external API timeseries endpoints currently return data as a `BTreeMap<TimeseriesKey, Timeseries>`, which results in JSON output with arbitrary numeric keys that have no meaning to API consumers: ```json { "timeseries": { "2352746367989923131": { ... }, "3940108470521992408": { ... } } } ``` This commit introduces a new `TableOutput` struct that presents timeseries data as an array instead of a map. The `TableOutput` type is converted from the internal `Table` representation at the API boundary, preserving the ordering from the original `BTreeMap` while providing a cleaner JSON structure: ```json { "timeseries": [ { ... }, { ... } ] } ``` Closes #8108 --------- Co-authored-by: David Crespo <[email protected]>
1 parent 5b6adf0 commit 778d6e6

File tree

5 files changed

+116
-57
lines changed

5 files changed

+116
-57
lines changed

‎nexus/src/external_api/http_entrypoints.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6571,7 +6571,11 @@ impl NexusExternalApi for NexusExternalApiImpl {
65716571
nexus
65726572
.timeseries_query(&opctx, &query)
65736573
.await
6574-
.map(|tables| HttpResponseOk(views::OxqlQueryResult { tables }))
6574+
.map(|tables| {
6575+
HttpResponseOk(views::OxqlQueryResult {
6576+
tables: tables.into_iter().map(Into::into).collect(),
6577+
})
6578+
})
65756579
.map_err(HttpError::from)
65766580
};
65776581
apictx
@@ -6598,7 +6602,11 @@ impl NexusExternalApi for NexusExternalApiImpl {
65986602
nexus
65996603
.timeseries_query_project(&opctx, &project_lookup, &query)
66006604
.await
6601-
.map(|tables| HttpResponseOk(views::OxqlQueryResult { tables }))
6605+
.map(|tables| {
6606+
HttpResponseOk(views::OxqlQueryResult {
6607+
tables: tables.into_iter().map(Into::into).collect(),
6608+
})
6609+
})
66026610
.map_err(HttpError::from)
66036611
};
66046612
apictx

‎nexus/tests/integration_tests/metrics.rs

Lines changed: 28 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use nexus_test_utils::resource_helpers::{
2222
use nexus_test_utils::wait_for_producer;
2323
use nexus_test_utils_macros::nexus_test;
2424
use nexus_types::external_api::shared::ProjectRole;
25-
use nexus_types::external_api::views::OxqlQueryResult;
25+
use nexus_types::external_api::views;
2626
use nexus_types::silo::DEFAULT_SILO_ID;
2727
use omicron_uuid_kinds::{GenericUuid, InstanceUuid};
2828
use oximeter::TimeseriesSchema;
@@ -270,17 +270,18 @@ async fn test_instance_watcher_metrics(
270270

271271
#[track_caller]
272272
fn count_state(
273-
table: &oxql_types::Table,
273+
table: &views::OxqlTable,
274274
instance_id: InstanceUuid,
275275
state: &'static str,
276276
) -> Result<i64, MetricsNotYet> {
277277
use oxql_types::point::ValueArray;
278278
let uuid = FieldValue::Uuid(instance_id.into_untyped_uuid());
279279
let state = FieldValue::String(state.into());
280-
let mut timeserieses = table.timeseries().filter(|ts| {
281-
ts.fields.get(INSTANCE_ID_FIELD) == Some(&uuid)
282-
&& ts.fields.get(STATE_FIELD) == Some(&state)
283-
});
280+
let mut timeserieses =
281+
table.timeseries.clone().into_iter().filter(|ts| {
282+
ts.fields.get(INSTANCE_ID_FIELD) == Some(&uuid)
283+
&& ts.fields.get(STATE_FIELD) == Some(&state)
284+
});
284285
let timeseries = timeserieses.next().ok_or_else(|| {
285286
MetricsNotYet::new(format!(
286287
"missing timeseries for instance {instance_id}, state {state}\n\
@@ -325,7 +326,7 @@ async fn test_instance_watcher_metrics(
325326
.system_timeseries_query_until(OXQL_QUERY, |metrics| {
326327
let checks = metrics
327328
.iter()
328-
.find(|t| t.name() == "virtual_machine:check")
329+
.find(|t| t.name == "virtual_machine:check")
329330
.ok_or_else(|| {
330331
MetricsNotYet::new("missing virtual_machine:check")
331332
})?;
@@ -348,7 +349,7 @@ async fn test_instance_watcher_metrics(
348349
.system_timeseries_query_until(OXQL_QUERY, |metrics| {
349350
let checks = metrics
350351
.iter()
351-
.find(|t| t.name() == "virtual_machine:check")
352+
.find(|t| t.name == "virtual_machine:check")
352353
.expect("missing virtual_machine:check");
353354
let ts1 =
354355
dbg!(count_state(&checks, instance1_uuid, STATE_STARTING)?);
@@ -371,7 +372,7 @@ async fn test_instance_watcher_metrics(
371372
.system_timeseries_query_until(OXQL_QUERY, |metrics| {
372373
let checks = metrics
373374
.iter()
374-
.find(|t| t.name() == "virtual_machine:check")
375+
.find(|t| t.name == "virtual_machine:check")
375376
.expect("missing virtual_machine:check");
376377
let ts1_starting =
377378
dbg!(count_state(&checks, instance1_uuid, STATE_STARTING)?);
@@ -402,7 +403,7 @@ async fn test_instance_watcher_metrics(
402403
.system_timeseries_query_until(OXQL_QUERY, |metrics| {
403404
let checks = metrics
404405
.iter()
405-
.find(|t| t.name() == "virtual_machine:check")
406+
.find(|t| t.name == "virtual_machine:check")
406407
.expect("missing virtual_machine:check");
407408

408409
let ts1_starting =
@@ -437,7 +438,7 @@ async fn test_instance_watcher_metrics(
437438
.system_timeseries_query_until(OXQL_QUERY, |metrics| {
438439
let checks = metrics
439440
.iter()
440-
.find(|t| t.name() == "virtual_machine:check")
441+
.find(|t| t.name == "virtual_machine:check")
441442
.expect("missing virtual_machine:check");
442443
let ts1_starting =
443444
dbg!(count_state(&checks, instance1_uuid, STATE_STARTING)?);
@@ -497,7 +498,7 @@ async fn test_project_timeseries_query(
497498
return Err(MetricsNotYet::new("waiting for table creation"));
498499
}
499500
assert_eq!(result.len(), 1);
500-
if result[0].timeseries().len() == 0 {
501+
if result[0].timeseries.is_empty() {
501502
return Err(MetricsNotYet::new(
502503
"waiting for timeseries population",
503504
));
@@ -511,23 +512,23 @@ async fn test_project_timeseries_query(
511512
.project_timeseries_query(&p1.identity.id.to_string(), q1)
512513
.await;
513514
assert_eq!(result.len(), 1);
514-
assert!(result[0].timeseries().len() > 0);
515+
assert!(!result[0].timeseries.is_empty());
515516

516517
let result = metrics_querier.project_timeseries_query("project2", q1).await;
517518
assert_eq!(result.len(), 1);
518-
assert!(result[0].timeseries().len() > 0);
519+
assert!(!result[0].timeseries.is_empty());
519520

520521
// with project specified
521522
let q2 = &format!("{} | filter project_id == \"{}\"", q1, p1.identity.id);
522523

523524
let result = metrics_querier.project_timeseries_query("project1", q2).await;
524525
assert_eq!(result.len(), 1);
525526
// we get 2 timeseries because there are two instances
526-
assert!(result[0].timeseries().len() == 2);
527+
assert!(result[0].timeseries.len() == 2);
527528

528529
let result = metrics_querier.project_timeseries_query("project2", q2).await;
529530
assert_eq!(result.len(), 1);
530-
assert_eq!(result[0].timeseries().len(), 0);
531+
assert_eq!(result[0].timeseries.len(), 0);
531532

532533
// with instance specified
533534
let q3 =
@@ -536,12 +537,12 @@ async fn test_project_timeseries_query(
536537
// project containing instance gives me something
537538
let result = metrics_querier.project_timeseries_query("project1", q3).await;
538539
assert_eq!(result.len(), 1);
539-
assert_eq!(result[0].timeseries().len(), 1);
540+
assert_eq!(result[0].timeseries.len(), 1);
540541

541542
// should be empty or error
542543
let result = metrics_querier.project_timeseries_query("project2", q3).await;
543544
assert_eq!(result.len(), 1);
544-
assert_eq!(result[0].timeseries().len(), 0);
545+
assert_eq!(result[0].timeseries.len(), 0);
545546

546547
// now let's test it with group_by
547548
let q4 = &format!(
@@ -550,7 +551,7 @@ async fn test_project_timeseries_query(
550551
);
551552
let result = metrics_querier.project_timeseries_query("project1", q4).await;
552553
assert_eq!(result.len(), 1);
553-
assert_eq!(result[0].timeseries().len(), 2);
554+
assert_eq!(result[0].timeseries.len(), 2);
554555

555556
// test with a nested query
556557
let q5 = &format!(
@@ -565,13 +566,13 @@ async fn test_project_timeseries_query(
565566
// we get two results, each contains one timeseries, and the instance ID
566567
// on each corresponds to the one we requested
567568
assert_eq!(result.len(), 2);
568-
assert_eq!(result[0].timeseries().len(), 1);
569-
let timeseries = result[0].timeseries().next().unwrap();
569+
assert_eq!(result[0].timeseries.len(), 1);
570+
let timeseries = result[0].timeseries[0].clone();
570571
let instance_id = timeseries.fields.get("instance_id").unwrap().to_string();
571572
assert_eq!(instance_id, i1p1.identity.id.to_string());
572573

573-
assert_eq!(result[1].timeseries().len(), 1);
574-
let timeseries = result[1].timeseries().next().unwrap();
574+
assert_eq!(result[1].timeseries.len(), 1);
575+
let timeseries = &result[1].timeseries[0];
575576
let instance_id = timeseries.fields.get("instance_id").unwrap().to_string();
576577
assert_eq!(instance_id, i2p1.identity.id.to_string());
577578

@@ -639,10 +640,10 @@ async fn test_project_timeseries_query(
639640
.expect_status(Some(StatusCode::OK));
640641
let result = NexusRequest::new(request)
641642
.authn_as(AuthnMode::UnprivilegedUser)
642-
.execute_and_parse_unwrap::<OxqlQueryResult>()
643+
.execute_and_parse_unwrap::<views::OxqlQueryResult>()
643644
.await;
644645
assert_eq!(result.tables.len(), 1);
645-
assert_eq!(result.tables[0].timeseries().len(), 2); // two instances
646+
assert_eq!(result.tables[0].timeseries.len(), 2); // two instances
646647
}
647648

648649
#[nexus_test]
@@ -780,7 +781,7 @@ async fn test_mgs_metrics(
780781
querier.system_timeseries_query_until(&query, |tables| {
781782
let table = tables
782783
.into_iter()
783-
.find(|t| t.name() == name)
784+
.find(|t| t.name == *name)
784785
.ok_or_else(|| {
785786
MetricsNotYet::new(format!(
786787
"failed to find table for {query}",
@@ -791,7 +792,7 @@ async fn test_mgs_metrics(
791792
.keys()
792793
.map(|serial| (serial.clone(), 0))
793794
.collect::<HashMap<_, usize>>();
794-
for timeseries in table.timeseries() {
795+
for timeseries in &table.timeseries {
795796
let fields = &timeseries.fields;
796797
if timeseries.points.is_empty() {
797798
return Err(MetricsNotYet::new(format!(

‎nexus/tests/integration_tests/metrics_querier.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use nexus_test_utils::http_testing::NexusRequest;
1313
use nexus_test_utils::http_testing::RequestBuilder;
1414
use nexus_test_utils::resource_helpers::objects_list_page_authz;
1515
use nexus_types::external_api::params;
16-
use nexus_types::external_api::views::OxqlQueryResult;
16+
use nexus_types::external_api::views;
1717
use omicron_test_utils::dev::poll;
1818
use omicron_test_utils::dev::poll::CondCheckError;
1919
use omicron_test_utils::dev::poll::wait_for_condition;
@@ -93,7 +93,7 @@ impl<'a, N> MetricsQuerier<'a, N> {
9393
cond: F,
9494
) -> T
9595
where
96-
F: Fn(Vec<oxql_types::Table>) -> Result<T, MetricsNotYet>,
96+
F: Fn(Vec<views::OxqlTable>) -> Result<T, MetricsNotYet>,
9797
{
9898
self.timeseries_query_until("/v1/system/timeseries/query", query, cond)
9999
.await
@@ -108,7 +108,7 @@ impl<'a, N> MetricsQuerier<'a, N> {
108108
cond: F,
109109
) -> T
110110
where
111-
F: Fn(Vec<oxql_types::Table>) -> Result<T, MetricsNotYet>,
111+
F: Fn(Vec<views::OxqlTable>) -> Result<T, MetricsNotYet>,
112112
{
113113
self.timeseries_query_until(
114114
&format!("/v1/timeseries/query?project={project}"),
@@ -128,7 +128,7 @@ impl<'a, N> MetricsQuerier<'a, N> {
128128
&self,
129129
project: &str,
130130
query: &str,
131-
) -> Vec<oxql_types::Table> {
131+
) -> Vec<views::OxqlTable> {
132132
self.project_timeseries_query_until(project, query, |tables| Ok(tables))
133133
.await
134134
}
@@ -270,7 +270,7 @@ impl<'a, N> MetricsQuerier<'a, N> {
270270
cond: F,
271271
) -> T
272272
where
273-
F: Fn(Vec<oxql_types::Table>) -> Result<T, MetricsNotYet>,
273+
F: Fn(Vec<views::OxqlTable>) -> Result<T, MetricsNotYet>,
274274
{
275275
let result = wait_for_condition(
276276
|| async {
@@ -375,7 +375,7 @@ impl<'a, N> MetricsQuerier<'a, N> {
375375
// Try to parse the query as usual, which will fail on other kinds of
376376
// errors.
377377
TimeseriesQueryResult::Ok(
378-
rsp.parsed_body::<OxqlQueryResult>()
378+
rsp.parsed_body::<views::OxqlQueryResult>()
379379
.unwrap_or_else(|e| {
380380
panic!(
381381
"could not parse timeseries query response: {e:?}\n\
@@ -389,5 +389,5 @@ impl<'a, N> MetricsQuerier<'a, N> {
389389

390390
enum TimeseriesQueryResult {
391391
TimeseriesNotFound,
392-
Ok(Vec<oxql_types::Table>),
392+
Ok(Vec<views::OxqlTable>),
393393
}

‎nexus/types/src/external_api/views.rs

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1113,11 +1113,59 @@ pub struct AllowList {
11131113

11141114
// OxQL QUERIES
11151115

1116+
/// A table represents one or more timeseries with the same schema.
1117+
///
1118+
/// A table is the result of an OxQL query. It contains a name, usually the name
1119+
/// of the timeseries schema from which the data is derived, and any number of
1120+
/// timeseries, which contain the actual data.
1121+
//
1122+
// # Motivation
1123+
//
1124+
// This struct is derived from [`oxql_types::Table`] but presents timeseries data as a `Vec`
1125+
// rather than a map keyed by [`TimeseriesKey`]. This provides a cleaner JSON
1126+
// representation for external consumers, as these numeric keys are ephemeral
1127+
// identifiers that have no meaning to API consumers. Key ordering is retained
1128+
// as this is contructed from the already sorted values present in [`Table`].
1129+
//
1130+
// When serializing a [`Table`] to JSON, the `BTreeMap<TimeseriesKey, Timeseries>`
1131+
// structure produces output with numeric keys like:
1132+
// ```json
1133+
// {
1134+
// "timeseries": {
1135+
// "2352746367989923131": { ... },
1136+
// "3940108470521992408": { ... }
1137+
// }
1138+
// }
1139+
// ```
1140+
//
1141+
// The `Table` view instead serializes timeseries as an array:
1142+
// ```json
1143+
// {
1144+
// "timeseries": [ { ... }, { ... } ]
1145+
// }
1146+
// ```
1147+
#[derive(Clone, Debug, Deserialize, JsonSchema, PartialEq, Serialize)]
1148+
pub struct OxqlTable {
1149+
/// The name of the table.
1150+
pub name: String,
1151+
/// The set of timeseries in the table, ordered by key.
1152+
pub timeseries: Vec<oxql_types::Timeseries>,
1153+
}
1154+
1155+
impl From<oxql_types::Table> for OxqlTable {
1156+
fn from(table: oxql_types::Table) -> Self {
1157+
OxqlTable {
1158+
name: table.name.clone(),
1159+
timeseries: table.into_iter().collect(),
1160+
}
1161+
}
1162+
}
1163+
11161164
/// The result of a successful OxQL query.
11171165
#[derive(Clone, Debug, Deserialize, JsonSchema, Serialize)]
11181166
pub struct OxqlQueryResult {
11191167
/// Tables resulting from the query, each containing timeseries.
1120-
pub tables: Vec<oxql_types::Table>,
1168+
pub tables: Vec<OxqlTable>,
11211169
}
11221170

11231171
// ALERTS

‎openapi/nexus.json

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -22275,14 +22275,35 @@
2227522275
"description": "Tables resulting from the query, each containing timeseries.",
2227622276
"type": "array",
2227722277
"items": {
22278-
"$ref": "#/components/schemas/Table"
22278+
"$ref": "#/components/schemas/OxqlTable"
2227922279
}
2228022280
}
2228122281
},
2228222282
"required": [
2228322283
"tables"
2228422284
]
2228522285
},
22286+
"OxqlTable": {
22287+
"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.",
22288+
"type": "object",
22289+
"properties": {
22290+
"name": {
22291+
"description": "The name of the table.",
22292+
"type": "string"
22293+
},
22294+
"timeseries": {
22295+
"description": "The set of timeseries in the table, ordered by key.",
22296+
"type": "array",
22297+
"items": {
22298+
"$ref": "#/components/schemas/Timeseries"
22299+
}
22300+
}
22301+
},
22302+
"required": [
22303+
"name",
22304+
"timeseries"
22305+
]
22306+
},
2228622307
"Password": {
2228722308
"title": "A password used to authenticate a user",
2228822309
"description": "Passwords may be subject to additional constraints.",
@@ -25523,25 +25544,6 @@
2552325544
"vlan_id"
2552425545
]
2552525546
},
25526-
"Table": {
25527-
"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.",
25528-
"type": "object",
25529-
"properties": {
25530-
"name": {
25531-
"type": "string"
25532-
},
25533-
"timeseries": {
25534-
"type": "object",
25535-
"additionalProperties": {
25536-
"$ref": "#/components/schemas/Timeseries"
25537-
}
25538-
}
25539-
},
25540-
"required": [
25541-
"name",
25542-
"timeseries"
25543-
]
25544-
},
2554525547
"TargetRelease": {
2554625548
"description": "View of a system software target release.",
2554725549
"type": "object",

0 commit comments

Comments
 (0)