From 6f4923e63d39e1a530db992ed7f9c26976559846 Mon Sep 17 00:00:00 2001
From: bnaecker <ben@oxide.computer>
Date: Wed, 25 Oct 2023 10:11:39 -0700
Subject: [PATCH] Add new datum types to timeseries schema table (#4349)

- Fixes https://github.com/oxidecomputer/omicron/issues/4336
- Adds regression test making sure a query selecting all datum types
from the schema table succeeds, even if it returns zero results. This
uses an iterator over the datum type enum, so it should catch future
changes to the type.
- Adds new datum types to schema table
- Bumps oximeter database version number
---
 oximeter/db/src/client.rs               | 41 +++++++++++++++++++++++++
 oximeter/db/src/db-replicated-init.sql  | 20 +++++++++++-
 oximeter/db/src/db-single-node-init.sql | 20 +++++++++++-
 oximeter/db/src/model.rs                |  2 +-
 oximeter/oximeter/src/types.rs          |  1 +
 5 files changed, 81 insertions(+), 3 deletions(-)

diff --git a/oximeter/db/src/client.rs b/oximeter/db/src/client.rs
index c2b7c820a8..2d563045f2 100644
--- a/oximeter/db/src/client.rs
+++ b/oximeter/db/src/client.rs
@@ -2634,4 +2634,45 @@ mod tests {
         db.cleanup().await.expect("Failed to cleanup ClickHouse server");
         logctx.cleanup_successful();
     }
+
+    // Regression test for https://github.com/oxidecomputer/omicron/issues/4336.
+    //
+    // This tests that we can successfully query all extant datum types from the
+    // schema table. There may be no such values, but the query itself should
+    // succeed.
+    #[tokio::test]
+    async fn test_select_all_datum_types() {
+        use strum::IntoEnumIterator;
+        usdt::register_probes().unwrap();
+        let logctx = test_setup_log("test_update_schema_cache_on_new_sample");
+        let log = &logctx.log;
+
+        // Let the OS assign a port and discover it after ClickHouse starts
+        let mut db = ClickHouseInstance::new_single_node(0)
+            .await
+            .expect("Failed to start ClickHouse");
+        let address = SocketAddr::new("::1".parse().unwrap(), db.port());
+
+        let client = Client::new(address, &log);
+        client
+            .init_single_node_db()
+            .await
+            .expect("Failed to initialize timeseries database");
+
+        // Attempt to select all schema with each datum type.
+        for ty in oximeter::DatumType::iter() {
+            let sql = format!(
+                "SELECT COUNT() \
+                FROM {}.timeseries_schema WHERE \
+                datum_type = '{:?}'",
+                crate::DATABASE_NAME,
+                crate::model::DbDatumType::from(ty),
+            );
+            let res = client.execute_with_body(sql).await.unwrap();
+            let count = res.trim().parse::<usize>().unwrap();
+            assert_eq!(count, 0);
+        }
+        db.cleanup().await.expect("Failed to cleanup ClickHouse server");
+        logctx.cleanup_successful();
+    }
 }
diff --git a/oximeter/db/src/db-replicated-init.sql b/oximeter/db/src/db-replicated-init.sql
index 7b756f4b0d..3b734c113d 100644
--- a/oximeter/db/src/db-replicated-init.sql
+++ b/oximeter/db/src/db-replicated-init.sql
@@ -652,7 +652,25 @@ CREATE TABLE IF NOT EXISTS oximeter.timeseries_schema ON CLUSTER oximeter_cluste
         'CumulativeI64' = 6,
         'CumulativeF64' = 7,
         'HistogramI64' = 8,
-        'HistogramF64' = 9
+        'HistogramF64' = 9,
+        'I8' = 10,
+        'U8' = 11,
+        'I16' = 12,
+        'U16' = 13,
+        'I32' = 14,
+        'U32' = 15,
+        'U64' = 16,
+        'F32' = 17,
+        'CumulativeU64' = 18,
+        'CumulativeF32' = 19,
+        'HistogramI8' = 20,
+        'HistogramU8' = 21,
+        'HistogramI16' = 23,
+        'HistogramU16' = 23,
+        'HistogramI32' = 24,
+        'HistogramU32' = 25,
+        'HistogramU64' = 26,
+        'HistogramF32' = 27
     ),
     created DateTime64(9, 'UTC')
 )
diff --git a/oximeter/db/src/db-single-node-init.sql b/oximeter/db/src/db-single-node-init.sql
index 1f648fd5d5..2fb5c36397 100644
--- a/oximeter/db/src/db-single-node-init.sql
+++ b/oximeter/db/src/db-single-node-init.sql
@@ -476,7 +476,25 @@ CREATE TABLE IF NOT EXISTS oximeter.timeseries_schema
         'CumulativeI64' = 6,
         'CumulativeF64' = 7,
         'HistogramI64' = 8,
-        'HistogramF64' = 9
+        'HistogramF64' = 9,
+        'I8' = 10,
+        'U8' = 11,
+        'I16' = 12,
+        'U16' = 13,
+        'I32' = 14,
+        'U32' = 15,
+        'U64' = 16,
+        'F32' = 17,
+        'CumulativeU64' = 18,
+        'CumulativeF32' = 19,
+        'HistogramI8' = 20,
+        'HistogramU8' = 21,
+        'HistogramI16' = 22,
+        'HistogramU16' = 23,
+        'HistogramI32' = 24,
+        'HistogramU32' = 25,
+        'HistogramU64' = 26,
+        'HistogramF32' = 27
     ),
     created DateTime64(9, 'UTC')
 )
diff --git a/oximeter/db/src/model.rs b/oximeter/db/src/model.rs
index 7f5b150b46..41c7ab9d24 100644
--- a/oximeter/db/src/model.rs
+++ b/oximeter/db/src/model.rs
@@ -42,7 +42,7 @@ use uuid::Uuid;
 ///
 /// TODO(#4271): The current implementation of versioning will wipe the metrics
 /// database if this number is incremented.
-pub const OXIMETER_VERSION: u64 = 1;
+pub const OXIMETER_VERSION: u64 = 2;
 
 // Wrapper type to represent a boolean in the database.
 //
diff --git a/oximeter/oximeter/src/types.rs b/oximeter/oximeter/src/types.rs
index d3f1b9e746..0cc3299ec4 100644
--- a/oximeter/oximeter/src/types.rs
+++ b/oximeter/oximeter/src/types.rs
@@ -275,6 +275,7 @@ pub struct Field {
     JsonSchema,
     Serialize,
     Deserialize,
+    strum::EnumIter,
 )]
 #[serde(rename_all = "snake_case")]
 pub enum DatumType {