Skip to content

oximeter: clean up query summary logic and docs. #9184

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

oximeter: clean up query summary logic and docs. #9184

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 6 additions & 6 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 @@ -7157,13 +7157,13 @@ impl NexusExternalApi for NexusExternalApiImpl {
.into_iter()
.map(Into::into)
.collect(),
query_summaries: include_summaries.then_some(
query_summaries: include_summaries.then(|| {
result
.query_summaries
.into_iter()
.map(Into::into)
.collect(),
),
.collect()
}),
})
})
.map_err(HttpError::from)
Expand Down Expand Up @@ -7201,13 +7201,13 @@ impl NexusExternalApi for NexusExternalApiImpl {
.into_iter()
.map(Into::into)
.collect(),
query_summaries: include_summaries.then_some(
query_summaries: include_summaries.then(|| {
result
.query_summaries
.into_iter()
.map(Into::into)
.collect(),
),
.collect()
}),
})
})
.map_err(HttpError::from)
Expand Down
4 changes: 3 additions & 1 deletion nexus/types/src/external_api/params.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2443,8 +2443,10 @@ pub struct ProbeListSelector {
pub struct TimeseriesQuery {
/// A timeseries query string, written in the Oximeter query language.
pub query: String,
/// Whether to include ClickHouse query summaries in the response.
/// Whether to include query summaries in the response. Note: we omit this
//field from the generated docs, since it's mainly of interest internally.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
//field from the generated docs, since it's mainly of interest internally.
/// field from the generated docs, since it is not intended for consumption by customers.

#[serde(default)]
#[schemars(skip)]
pub include_summaries: bool,
}

Expand Down
10 changes: 6 additions & 4 deletions nexus/types/src/external_api/views.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1123,14 +1123,14 @@ impl From<oxql_types::Table> for OxqlTable {
}
}

/// Basic metadata about the resource usage of a single ClickHouse SQL query.
/// Basic metadata about the resource usage of a single backend query.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Basic metadata about the resource usage of a single backend query.
/// Basic metadata about the resource usage of a query.

#[derive(Clone, Debug, Deserialize, JsonSchema, Serialize)]
pub struct OxqlQuerySummary {
/// The database-assigned query ID.
pub id: Uuid,
/// The raw ClickHouse SQL query.
/// The raw query.
pub query: String,
/// The total duration of the ClickHouse query (network plus execution).
/// The total duration of the query (network plus execution).
pub elapsed_ms: usize,
/// Summary of the data read and written.
pub io_summary: oxql_types::IoSummary,
Expand All @@ -1152,7 +1152,9 @@ impl From<oxql_types::QuerySummary> for OxqlQuerySummary {
pub struct OxqlQueryResult {
/// Tables resulting from the query, each containing timeseries.
pub tables: Vec<OxqlTable>,
/// Summaries of queries run against ClickHouse.
/// Summaries of queries run against ClickHouse. Note: we omit this field
//from the generated docs, since it's mainly of interest internally.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as https://github.com/oxidecomputer/omicron/pull/9184/files#r2417344822

#[schemars(skip)]
pub query_summaries: Option<Vec<OxqlQuerySummary>>,
}

Expand Down
96 changes: 0 additions & 96 deletions openapi/nexus.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21386,54 +21386,6 @@
"items"
]
},
"IoCount": {
"description": "A count of bytes / rows accessed during a query.",
"type": "object",
"properties": {
"bytes": {
"description": "The number of bytes accessed.",
"type": "integer",
"format": "uint64",
"minimum": 0
},
"rows": {
"description": "The number of rows accessed.",
"type": "integer",
"format": "uint64",
"minimum": 0
}
},
"required": [
"bytes",
"rows"
]
},
"IoSummary": {
"description": "Summary of the I/O resources used by a query.",
"type": "object",
"properties": {
"read": {
"description": "The bytes and rows read by the query.",
"allOf": [
{
"$ref": "#/components/schemas/IoCount"
}
]
},
"written": {
"description": "The bytes and rows written by the query.",
"allOf": [
{
"$ref": "#/components/schemas/IoCount"
}
]
}
},
"required": [
"read",
"written"
]
},
"IpNet": {
"x-rust-type": {
"crate": "oxnet",
Expand Down Expand Up @@ -22551,14 +22503,6 @@
"description": "The result of a successful OxQL query.",
"type": "object",
"properties": {
"query_summaries": {
"nullable": true,
"description": "Summaries of queries run against ClickHouse.",
"type": "array",
"items": {
"$ref": "#/components/schemas/OxqlQuerySummary"
}
},
"tables": {
"description": "Tables resulting from the query, each containing timeseries.",
"type": "array",
Expand All @@ -22571,41 +22515,6 @@
"tables"
]
},
"OxqlQuerySummary": {
"description": "Basic metadata about the resource usage of a single ClickHouse SQL query.",
"type": "object",
"properties": {
"elapsed_ms": {
"description": "The total duration of the ClickHouse query (network plus execution).",
"type": "integer",
"format": "uint",
"minimum": 0
},
"id": {
"description": "The database-assigned query ID.",
"type": "string",
"format": "uuid"
},
"io_summary": {
"description": "Summary of the data read and written.",
"allOf": [
{
"$ref": "#/components/schemas/IoSummary"
}
]
},
"query": {
"description": "The raw ClickHouse SQL query.",
"type": "string"
}
},
"required": [
"elapsed_ms",
"id",
"io_summary",
"query"
]
},
"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",
Expand Down Expand Up @@ -26036,11 +25945,6 @@
"description": "A timeseries query string, written in the Oximeter query language.",
"type": "object",
"properties": {
"include_summaries": {
"description": "Whether to include ClickHouse query summaries in the response.",
"default": false,
"type": "boolean"
},
"query": {
"description": "A timeseries query string, written in the Oximeter query language.",
"type": "string"
Expand Down
Loading