Skip to content

Commit ef0a966

Browse files
jmcarpjmcarp
authored
Include oximeter query summaries in api response. (#9017)
[Resolves #9014] [Resolves #9015]
1 parent 200ce2d commit ef0a966

File tree

23 files changed

+270
-171
lines changed

23 files changed

+270
-171
lines changed

‎Cargo.lock‎

Lines changed: 3 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎nexus/src/app/metrics.rs‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use nexus_external_api::TimeseriesSchemaPaginationParams;
1313
use nexus_types::external_api::params::SystemMetricName;
1414
use omicron_common::api::external::{Error, InternalContext};
1515
use oximeter_db::{
16-
Measurement, TimeseriesSchema, oxql::query::QueryAuthzScope,
16+
Measurement, OxqlResult, TimeseriesSchema, oxql::query::QueryAuthzScope,
1717
};
1818
use std::num::NonZeroU32;
1919

@@ -130,7 +130,7 @@ impl super::Nexus {
130130
&self,
131131
opctx: &OpContext,
132132
query: impl AsRef<str>,
133-
) -> Result<Vec<oxql_types::Table>, Error> {
133+
) -> Result<OxqlResult, Error> {
134134
// Must be a fleet user to list timeseries schema.
135135
//
136136
// TODO-security: We need to figure out how to implement proper security
@@ -140,14 +140,6 @@ impl super::Nexus {
140140
self.timeseries_client
141141
.oxql_query(query, QueryAuthzScope::Fleet)
142142
.await
143-
// TODO-observability: The query method returns information
144-
// about the duration of the OxQL query and the database
145-
// resource usage for each contained SQL query. We should
146-
// publish this as a timeseries itself, so that we can track
147-
// improvements to query processing.
148-
//
149-
// For now, simply return the tables alone.
150-
.map(|result| result.tables)
151143
.map_err(map_timeseries_err)
152144
}
153145

@@ -157,7 +149,7 @@ impl super::Nexus {
157149
_opctx: &OpContext,
158150
project_lookup: &lookup::Project<'_>,
159151
query: impl AsRef<str>,
160-
) -> Result<Vec<oxql_types::Table>, Error> {
152+
) -> Result<OxqlResult, Error> {
161153
// Ensure the user has read access to the project
162154
let (authz_silo, authz_project) =
163155
project_lookup.lookup_for(authz::Action::Read).await?;
@@ -170,7 +162,6 @@ impl super::Nexus {
170162
},
171163
)
172164
.await
173-
.map(|result| result.tables)
174165
.map_err(map_timeseries_err)
175166
}
176167
}

‎nexus/src/external_api/http_entrypoints.rs‎

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6562,13 +6562,26 @@ impl NexusExternalApi for NexusExternalApiImpl {
65626562
let nexus = &apictx.context.nexus;
65636563
let opctx =
65646564
crate::context::op_context_for_external_api(&rqctx).await?;
6565-
let query = body.into_inner().query;
6565+
let body_params = body.into_inner();
6566+
let query = body_params.query;
6567+
let include_summaries = body_params.include_summaries;
65666568
nexus
65676569
.timeseries_query(&opctx, &query)
65686570
.await
6569-
.map(|tables| {
6571+
.map(|result| {
65706572
HttpResponseOk(views::OxqlQueryResult {
6571-
tables: tables.into_iter().map(Into::into).collect(),
6573+
tables: result
6574+
.tables
6575+
.into_iter()
6576+
.map(Into::into)
6577+
.collect(),
6578+
query_summaries: include_summaries.then_some(
6579+
result
6580+
.query_summaries
6581+
.into_iter()
6582+
.map(Into::into)
6583+
.collect(),
6584+
),
65726585
})
65736586
})
65746587
.map_err(HttpError::from)
@@ -6591,15 +6604,28 @@ impl NexusExternalApi for NexusExternalApiImpl {
65916604
let opctx =
65926605
crate::context::op_context_for_external_api(&rqctx).await?;
65936606
let project_selector = query_params.into_inner();
6594-
let query = body.into_inner().query;
6607+
let body_params = body.into_inner();
6608+
let query = body_params.query;
6609+
let include_summaries = body_params.include_summaries;
65956610
let project_lookup =
65966611
nexus.project_lookup(&opctx, project_selector)?;
65976612
nexus
65986613
.timeseries_query_project(&opctx, &project_lookup, &query)
65996614
.await
6600-
.map(|tables| {
6615+
.map(|result| {
66016616
HttpResponseOk(views::OxqlQueryResult {
6602-
tables: tables.into_iter().map(Into::into).collect(),
6617+
tables: result
6618+
.tables
6619+
.into_iter()
6620+
.map(Into::into)
6621+
.collect(),
6622+
query_summaries: include_summaries.then_some(
6623+
result
6624+
.query_summaries
6625+
.into_iter()
6626+
.map(Into::into)
6627+
.collect(),
6628+
),
66036629
})
66046630
})
66056631
.map_err(HttpError::from)

‎nexus/tests/integration_tests/endpoints.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1147,6 +1147,7 @@ pub static SYSTEM_TIMESERIES_QUERY_URL: LazyLock<String> =
11471147
pub static DEMO_TIMESERIES_QUERY: LazyLock<params::TimeseriesQuery> =
11481148
LazyLock::new(|| params::TimeseriesQuery {
11491149
query: String::from("get http_service:request_latency_histogram"),
1150+
include_summaries: false,
11501151
});
11511152

11521153
// Users

‎nexus/tests/integration_tests/metrics.rs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -582,6 +582,7 @@ async fn test_project_timeseries_query(
582582
let url = "/v1/timeseries/query?project=project1";
583583
let body = nexus_types::external_api::params::TimeseriesQuery {
584584
query: q6.to_string(),
585+
include_summaries: false,
585586
};
586587
let result =
587588
object_create_error(client, url, &body, StatusCode::BAD_REQUEST).await;
@@ -602,6 +603,7 @@ async fn test_project_timeseries_query(
602603
let url = "/v1/timeseries/query?project=nonexistent";
603604
let body = nexus_types::external_api::params::TimeseriesQuery {
604605
query: q6.to_string(),
606+
include_summaries: false,
605607
};
606608
let result =
607609
object_create_error(client, url, &body, StatusCode::NOT_FOUND).await;
@@ -611,6 +613,7 @@ async fn test_project_timeseries_query(
611613
let url = "/v1/timeseries/query?project=project1";
612614
let body = nexus_types::external_api::params::TimeseriesQuery {
613615
query: q1.to_string(),
616+
include_summaries: false,
614617
};
615618

616619
let request = RequestBuilder::new(client, Method::POST, url)
@@ -645,6 +648,24 @@ async fn test_project_timeseries_query(
645648
.await;
646649
assert_eq!(result.tables.len(), 1);
647650
assert_eq!(result.tables[0].timeseries.len(), 2); // two instances
651+
assert!(result.query_summaries.is_none());
652+
653+
// Request metrics again, this time with query summaries.
654+
let body = nexus_types::external_api::params::TimeseriesQuery {
655+
query: q1.to_string(),
656+
include_summaries: true,
657+
};
658+
let request = RequestBuilder::new(client, Method::POST, url)
659+
.body(Some(&body))
660+
.expect_status(Some(StatusCode::OK));
661+
let result = NexusRequest::new(request)
662+
.authn_as(AuthnMode::UnprivilegedUser)
663+
.execute_and_parse_unwrap::<views::OxqlQueryResult>()
664+
.await;
665+
assert_eq!(result.tables.len(), 1);
666+
assert_eq!(result.tables[0].timeseries.len(), 2); // two instances
667+
assert!(result.query_summaries.is_some());
668+
assert_eq!(result.query_summaries.unwrap().len(), 2);
648669
}
649670

650671
#[nexus_test]

‎nexus/tests/integration_tests/metrics_querier.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -339,7 +339,7 @@ impl<'a, N> MetricsQuerier<'a, N> {
339339
query: String,
340340
) -> TimeseriesQueryResult {
341341
// Issue the query.
342-
let body = params::TimeseriesQuery { query };
342+
let body = params::TimeseriesQuery { query, include_summaries: false };
343343
let query = &body.query;
344344
let rsp = NexusRequest::new(
345345
RequestBuilder::new(

‎nexus/types/Cargo.toml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,9 @@ itertools.workspace = true
3232
newtype_derive.workspace = true
3333
omicron-uuid-kinds.workspace = true
3434
openssl.workspace = true
35-
oxql-types.workspace = true
35+
oximeter-db.workspace = true
3636
oxnet.workspace = true
37+
oxql-types.workspace = true
3738
parse-display.workspace = true
3839
regex.workspace = true
3940
schemars = { workspace = true, features = ["chrono", "uuid1", "url"] }

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2443,6 +2443,9 @@ pub struct ProbeListSelector {
24432443
pub struct TimeseriesQuery {
24442444
/// A timeseries query string, written in the Oximeter query language.
24452445
pub query: String,
2446+
/// Whether to include ClickHouse query summaries in the response.
2447+
#[serde(default)]
2448+
pub include_summaries: bool,
24462449
}
24472450

24482451
// Allowed source IPs

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1123,11 +1123,37 @@ impl From<oxql_types::Table> for OxqlTable {
11231123
}
11241124
}
11251125

1126+
/// Basic metadata about the resource usage of a single ClickHouse SQL query.
1127+
#[derive(Clone, Debug, Deserialize, JsonSchema, Serialize)]
1128+
pub struct OxqlQuerySummary {
1129+
/// The database-assigned query ID.
1130+
pub id: Uuid,
1131+
/// The raw ClickHouse SQL query.
1132+
pub query: String,
1133+
/// The total duration of the ClickHouse query (network plus execution).
1134+
pub elapsed_ms: usize,
1135+
/// Summary of the data read and written.
1136+
pub io_summary: oxql_types::IoSummary,
1137+
}
1138+
1139+
impl From<oxql_types::QuerySummary> for OxqlQuerySummary {
1140+
fn from(query_summary: oxql_types::QuerySummary) -> Self {
1141+
OxqlQuerySummary {
1142+
id: query_summary.id,
1143+
query: query_summary.query,
1144+
elapsed_ms: query_summary.elapsed.as_millis() as usize,
1145+
io_summary: query_summary.io_summary,
1146+
}
1147+
}
1148+
}
1149+
11261150
/// The result of a successful OxQL query.
11271151
#[derive(Clone, Debug, Deserialize, JsonSchema, Serialize)]
11281152
pub struct OxqlQueryResult {
11291153
/// Tables resulting from the query, each containing timeseries.
11301154
pub tables: Vec<OxqlTable>,
1155+
/// Summaries of queries run against ClickHouse.
1156+
pub query_summaries: Option<Vec<OxqlQuerySummary>>,
11311157
}
11321158

11331159
// ALERTS

‎openapi/nexus.json‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21199,6 +21199,54 @@
2119921199
"items"
2120021200
]
2120121201
},
21202+
"IoCount": {
21203+
"description": "A count of bytes / rows accessed during a query.",
21204+
"type": "object",
21205+
"properties": {
21206+
"bytes": {
21207+
"description": "The number of bytes accessed.",
21208+
"type": "integer",
21209+
"format": "uint64",
21210+
"minimum": 0
21211+
},
21212+
"rows": {
21213+
"description": "The number of rows accessed.",
21214+
"type": "integer",
21215+
"format": "uint64",
21216+
"minimum": 0
21217+
}
21218+
},
21219+
"required": [
21220+
"bytes",
21221+
"rows"
21222+
]
21223+
},
21224+
"IoSummary": {
21225+
"description": "Summary of the I/O resources used by a query.",
21226+
"type": "object",
21227+
"properties": {
21228+
"read": {
21229+
"description": "The bytes and rows read by the query.",
21230+
"allOf": [
21231+
{
21232+
"$ref": "#/components/schemas/IoCount"
21233+
}
21234+
]
21235+
},
21236+
"written": {
21237+
"description": "The bytes and rows written by the query.",
21238+
"allOf": [
21239+
{
21240+
"$ref": "#/components/schemas/IoCount"
21241+
}
21242+
]
21243+
}
21244+
},
21245+
"required": [
21246+
"read",
21247+
"written"
21248+
]
21249+
},
2120221250
"IpNet": {
2120321251
"x-rust-type": {
2120421252
"crate": "oxnet",
@@ -22316,6 +22364,14 @@
2231622364
"description": "The result of a successful OxQL query.",
2231722365
"type": "object",
2231822366
"properties": {
22367+
"query_summaries": {
22368+
"nullable": true,
22369+
"description": "Summaries of queries run against ClickHouse.",
22370+
"type": "array",
22371+
"items": {
22372+
"$ref": "#/components/schemas/OxqlQuerySummary"
22373+
}
22374+
},
2231922375
"tables": {
2232022376
"description": "Tables resulting from the query, each containing timeseries.",
2232122377
"type": "array",
@@ -22328,6 +22384,41 @@
2232822384
"tables"
2232922385
]
2233022386
},
22387+
"OxqlQuerySummary": {
22388+
"description": "Basic metadata about the resource usage of a single ClickHouse SQL query.",
22389+
"type": "object",
22390+
"properties": {
22391+
"elapsed_ms": {
22392+
"description": "The total duration of the ClickHouse query (network plus execution).",
22393+
"type": "integer",
22394+
"format": "uint",
22395+
"minimum": 0
22396+
},
22397+
"id": {
22398+
"description": "The database-assigned query ID.",
22399+
"type": "string",
22400+
"format": "uuid"
22401+
},
22402+
"io_summary": {
22403+
"description": "Summary of the data read and written.",
22404+
"allOf": [
22405+
{
22406+
"$ref": "#/components/schemas/IoSummary"
22407+
}
22408+
]
22409+
},
22410+
"query": {
22411+
"description": "The raw ClickHouse SQL query.",
22412+
"type": "string"
22413+
}
22414+
},
22415+
"required": [
22416+
"elapsed_ms",
22417+
"id",
22418+
"io_summary",
22419+
"query"
22420+
]
22421+
},
2233122422
"OxqlTable": {
2233222423
"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.",
2233322424
"type": "object",
@@ -25709,6 +25800,11 @@
2570925800
"description": "A timeseries query string, written in the Oximeter query language.",
2571025801
"type": "object",
2571125802
"properties": {
25803+
"include_summaries": {
25804+
"description": "Whether to include ClickHouse query summaries in the response.",
25805+
"default": false,
25806+
"type": "boolean"
25807+
},
2571225808
"query": {
2571325809
"description": "A timeseries query string, written in the Oximeter query language.",
2571425810
"type": "string"

0 commit comments

Comments
 (0)