Skip to content

Commit 704f7f8

Browse files
authored
sql-333: Add application_name to peek duration metrics (#36998)
Adding to provide better observability for catalog server incidents. Unsure if a test for the metric is necessary 🤷
1 parent 62a7647 commit 704f7f8

4 files changed

Lines changed: 53 additions & 8 deletions

File tree

src/adapter/src/client.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1583,12 +1583,9 @@ impl RecordFirstRowStream {
15831583
instance_id: Option<ComputeInstanceId>,
15841584
strategy: Option<StatementExecutionStrategy>,
15851585
) -> Histogram {
1586-
let isolation_level = *client
1587-
.session
1588-
.as_ref()
1589-
.expect("session invariant")
1590-
.vars()
1591-
.transaction_isolation();
1586+
let session = client.session.as_ref().expect("session invariant");
1587+
let isolation_level = *session.vars().transaction_isolation();
1588+
let name_hint = ApplicationNameHint::from_str(session.application_name());
15921589
let instance = match instance_id {
15931590
Some(i) => Cow::Owned(i.to_string()),
15941591
None => Cow::Borrowed("none"),
@@ -1606,6 +1603,7 @@ impl RecordFirstRowStream {
16061603
instance.as_ref(),
16071604
isolation_level.as_variant_str(),
16081605
strategy,
1606+
name_hint.as_str(),
16091607
])
16101608
}
16111609

src/adapter/src/metrics.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ impl Metrics {
140140
time_to_first_row_seconds: registry.register(metric! {
141141
name: "mz_time_to_first_row_seconds",
142142
help: "Latency of an execute for a successful query from pgwire's perspective",
143-
var_labels: ["instance_id", "isolation_level", "strategy"],
143+
var_labels: ["instance_id", "isolation_level", "strategy", "application_name"],
144144
buckets: histogram_seconds_buckets(0.000_128, 32.0)
145145
}),
146146
statement_logging_records: registry.register(metric! {

src/environmentd/tests/server.rs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3547,6 +3547,52 @@ async fn test_http_metrics() {
35473547
assert_eq!(failure_metric.get_label()[2].value(), "422");
35483548
}
35493549

3550+
#[mz_ore::test(tokio::test(flavor = "multi_thread", worker_threads = 1))]
3551+
#[cfg_attr(miri, ignore)]
3552+
#[allow(clippy::disallowed_methods)]
3553+
async fn test_time_to_first_row_metric() {
3554+
let server = test_util::TestHarness::default().start().await;
3555+
3556+
let client = server.connect().application_name("dbt").await.unwrap();
3557+
let _ = client.query_one("SELECT 1", &[]).await.unwrap();
3558+
3559+
let metrics = server.metrics_registry.gather();
3560+
let metric = metrics
3561+
.into_iter()
3562+
.find(|m| m.name() == "mz_time_to_first_row_seconds")
3563+
.expect("mz_time_to_first_row_seconds metric should exist");
3564+
3565+
// Find the series produced by our query.
3566+
let series = metric
3567+
.get_metric()
3568+
.iter()
3569+
.find(|m| {
3570+
m.get_label()
3571+
.iter()
3572+
.any(|l| l.name() == "application_name" && l.value() == "dbt")
3573+
})
3574+
.expect("a `mz_time_to_first_row_seconds` series for application_name=dbt should exist");
3575+
3576+
// Validate the labels are correct
3577+
let mut labels: Vec<(&str, &str)> = series
3578+
.get_label()
3579+
.iter()
3580+
.map(|l| (l.name(), l.value()))
3581+
.collect();
3582+
3583+
labels.sort();
3584+
3585+
assert_eq!(
3586+
labels,
3587+
vec![
3588+
("application_name", "dbt"),
3589+
("instance_id", "none"),
3590+
("isolation_level", "strict serializable"),
3591+
("strategy", "constant"),
3592+
],
3593+
);
3594+
}
3595+
35503596
#[mz_ore::test(tokio::test(flavor = "multi_thread", worker_threads = 2))]
35513597
#[cfg_attr(miri, ignore)] // too slow
35523598
#[allow(clippy::disallowed_methods)]

src/sql/src/session/hint.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,6 @@ impl ApplicationNameHint {
9191
pub fn from_str(s: &str) -> Self {
9292
match s.to_lowercase().as_str() {
9393
"psql" => ApplicationNameHint::Psql(Private),
94-
"dbt" => ApplicationNameHint::Dbt(Private),
9594
"web_console" => ApplicationNameHint::WebConsole(Private),
9695
"web_console_shell" => ApplicationNameHint::WebConsoleShell(Private),
9796
"mz_psql" => ApplicationNameHint::MzPsql(Private),
@@ -108,6 +107,8 @@ impl ApplicationNameHint {
108107
x if x.starts_with("terraform-provider-materialize") => {
109108
ApplicationNameHint::TerraformProviderMaterialize(Private)
110109
}
110+
// dbt provides the version as a suffix.
111+
x if x.starts_with("dbt") => ApplicationNameHint::Dbt(Private),
111112
// DataGrip provides the version as a suffix.
112113
x if x.starts_with("datagrip") => ApplicationNameHint::DataGrip(Private),
113114
// DBeaver provides the version as a suffix.

0 commit comments

Comments
 (0)