First round review feedback

- Remove serde defaults. These are new fields, and we generate defaults
  on conversion from the DB model type, so they're not needed. Also
  remove methods needed for serde defaults.
- Add methods for target/metric name of a timeseries schema, rather than
  tuple indexing.

oximeter/db/src/lib.rs

@@ -163,8 +163,8 @@ impl From<model::DbTimeseriesSchema> for TimeseriesSchema {
             // TODO(ben): Convert from real values once in the DB.
             description: Default::default(),
             version: oximeter::schema::default_schema_version(),
-            authz_scope: oximeter::schema::AuthzScope::fleet(),
-            units: oximeter::schema::Units::count(),
+            authz_scope: oximeter::schema::AuthzScope::Fleet,
+            units: oximeter::schema::Units::Count,
             field_schema: schema.field_schema.into(),
             datum_type: schema.datum_type.into(),
             created: schema.created,
@@ -414,8 +414,8 @@ mod tests {
             timeseries_name: timeseries_name.clone(),
             description: Default::default(),
             version: oximeter::schema::default_schema_version(),
-            authz_scope: oximeter::schema::AuthzScope::fleet(),
-            units: oximeter::schema::Units::count(),
+            authz_scope: oximeter::schema::AuthzScope::Fleet,
+            units: oximeter::schema::Units::Count,
             field_schema,
             datum_type,
             created: Utc::now(),

oximeter/db/src/query.rs

@@ -779,8 +779,8 @@ mod tests {
             timeseries_name: TimeseriesName::try_from("foo:bar").unwrap(),
             description: Default::default(),
             version: oximeter::schema::default_schema_version(),
-            authz_scope: oximeter::schema::AuthzScope::fleet(),
-            units: oximeter::schema::Units::count(),
+            authz_scope: oximeter::schema::AuthzScope::Fleet,
+            units: oximeter::schema::Units::Count,
             field_schema: [
                 FieldSchema {
                     name: "f0".to_string(),
@@ -918,8 +918,8 @@ mod tests {
             timeseries_name: TimeseriesName::try_from("foo:bar").unwrap(),
             description: Default::default(),
             version: oximeter::schema::default_schema_version(),
-            authz_scope: oximeter::schema::AuthzScope::fleet(),
-            units: oximeter::schema::Units::count(),
+            authz_scope: oximeter::schema::AuthzScope::Fleet,
+            units: oximeter::schema::Units::Count,
             field_schema: BTreeSet::new(),
             datum_type: DatumType::I64,
             created: Utc::now(),
@@ -944,8 +944,8 @@ mod tests {
             timeseries_name: TimeseriesName::try_from("foo:bar").unwrap(),
             description: Default::default(),
             version: oximeter::schema::default_schema_version(),
-            authz_scope: oximeter::schema::AuthzScope::fleet(),
-            units: oximeter::schema::Units::count(),
+            authz_scope: oximeter::schema::AuthzScope::Fleet,
+            units: oximeter::schema::Units::Count,
             field_schema: BTreeSet::new(),
             datum_type: DatumType::I64,
             created: Utc::now(),
@@ -1018,8 +1018,8 @@ mod tests {
             timeseries_name: TimeseriesName::try_from("foo:bar").unwrap(),
             description: Default::default(),
             version: oximeter::schema::default_schema_version(),
-            authz_scope: oximeter::schema::AuthzScope::fleet(),
-            units: oximeter::schema::Units::count(),
+            authz_scope: oximeter::schema::AuthzScope::Fleet,
+            units: oximeter::schema::Units::Count,
             field_schema: [
                 FieldSchema {
                     name: "f0".to_string(),
@@ -1087,8 +1087,8 @@ mod tests {
             timeseries_name: TimeseriesName::try_from("foo:bar").unwrap(),
             description: Default::default(),
             version: oximeter::schema::default_schema_version(),
-            authz_scope: oximeter::schema::AuthzScope::fleet(),
-            units: oximeter::schema::Units::count(),
+            authz_scope: oximeter::schema::AuthzScope::Fleet,
+            units: oximeter::schema::Units::Count,
             field_schema: [
                 FieldSchema {
                     name: "f0".to_string(),
@@ -1144,8 +1144,8 @@ mod tests {
             timeseries_name: TimeseriesName::try_from("foo:bar").unwrap(),
             description: Default::default(),
             version: oximeter::schema::default_schema_version(),
-            authz_scope: oximeter::schema::AuthzScope::fleet(),
-            units: oximeter::schema::Units::count(),
+            authz_scope: oximeter::schema::AuthzScope::Fleet,
+            units: oximeter::schema::Units::Count,
             field_schema: [
                 FieldSchema {
                     name: "f0".to_string(),

oximeter/impl/src/schema/codegen.rs

@@ -32,7 +32,7 @@ use quote::quote;
 pub fn use_timeseries(contents: &str) -> Result<TokenStream, MetricsError> {
     let schema = load_schema(contents)?;
     let latest = find_schema_version(schema.iter().cloned(), None);
-    let mod_name = quote::format_ident!("{}", latest[0].component_names().0);
+    let mod_name = quote::format_ident!("{}", latest[0].target_name());
     let types = emit_schema_types(latest);
     let func = emit_schema_function(schema.into_iter());
     Ok(quote! {
@@ -75,10 +75,9 @@ fn emit_target(schema: &TimeseriesSchema) -> TokenStream {
 }
 
 fn emit_one(source: FieldSource, schema: &TimeseriesSchema) -> TokenStream {
-    let names = schema.component_names();
     let name = match source {
-        FieldSource::Target => names.0,
-        FieldSource::Metric => names.1,
+        FieldSource::Target => schema.target_name(),
+        FieldSource::Metric => schema.metric_name(),
     };
     let struct_name =
         quote::format_ident!("{}", format!("{}", heck::AsPascalCase(name)));

oximeter/impl/src/schema/ir.rs

@@ -417,7 +417,7 @@ pub(super) fn find_schema_version(
         None => {
             let mut last_version = BTreeMap::new();
             for schema in list {
-                let metric_name = schema.component_names().1.to_string();
+                let metric_name = schema.metric_name().to_string();
                 match last_version.entry(metric_name) {
                     Entry::Vacant(entry) => {
                         entry.insert((schema.version, schema.clone()));

oximeter/impl/src/schema/mod.rs

@@ -43,7 +43,6 @@ pub struct FieldSchema {
     pub name: String,
     pub field_type: FieldType,
     pub source: FieldSource,
-    #[serde(default)]
     pub description: String,
 }
 
@@ -177,31 +176,18 @@ pub enum Units {
     Bytes,
 }
 
-impl Units {
-    /// Construct a `Units::Count`.
-    pub const fn count() -> Self {
-        Units::Count
-    }
-}
-
 /// The schema for a timeseries.
 ///
 /// This includes the name of the timeseries, as well as the datum type of its metric and the
 /// schema for each field.
 #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
 pub struct TimeseriesSchema {
     pub timeseries_name: TimeseriesName,
-    // TODO-cleanup: This default should be removed once schema are tracked in
-    // CockroachDB. Same for the version and scope fields.
-    #[serde(default)]
     pub description: TimeseriesDescription,
     pub field_schema: BTreeSet<FieldSchema>,
     pub datum_type: DatumType,
-    #[serde(default = "default_schema_version")]
     pub version: NonZeroU8,
-    #[serde(default = "AuthzScope::fleet")]
     pub authz_scope: AuthzScope,
-    #[serde(default = "Units::count")]
     pub units: Units,
     pub created: DateTime<Utc>,
 }
@@ -240,7 +226,7 @@ impl From<&Sample> for TimeseriesSchema {
             field_schema,
             datum_type,
             version: default_schema_version(),
-            authz_scope: AuthzScope::fleet(),
+            authz_scope: AuthzScope::Fleet,
             units: Units::Count,
             created: Utc::now(),
         }
@@ -306,6 +292,16 @@ impl TimeseriesSchema {
             .split_once(':')
             .expect("Incorrectly formatted timseries name")
     }
+
+    /// Return the name of the target for this timeseries.
+    pub fn target_name(&self) -> &str {
+        self.component_names().0
+    }
+
+    /// Return the name of the metric for this timeseries.
+    pub fn metric_name(&self) -> &str {
+        self.component_names().1
+    }
 }
 
 impl PartialEq for TimeseriesSchema {