Split attribute catalog from registry (in v2) (#1054)

jsuereth · web-flow · commit 1c6e5265e465 · 2025-12-05T13:51:15.000-05:00
* Split attribute catalog from registry.

Only attribute definitions wind up in the registry now.  Any refinement can be in the catalog but is NOT in the registry.

* Fix tests for new attribute splits.

* Clippy fix.

* Cargo fmt
diff --git a/crates/weaver_forge/src/v2/registry.rs b/crates/weaver_forge/src/v2/registry.rs
@@ -2,7 +2,7 @@
 
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
-use weaver_resolved_schema::attribute::AttributeRef;
+use weaver_resolved_schema::{attribute::AttributeRef, v2::catalog::AttributeCatalog};
 
 use crate::{
     error::Error,
@@ -84,20 +84,23 @@ impl ForgeResolvedRegistry {
     ) -> Result<Self, Error> {
         let mut errors = Vec::new();
 
+        let attribute_lookup = |r: &weaver_resolved_schema::v2::attribute::AttributeRef| {
+            schema.attribute_catalog.attribute(r)
+        };
         // We create an attribute lookup map.
         let mut attributes: Vec<Attribute> = schema
             .registry
             .attributes
             .iter()
+            .filter_map(&attribute_lookup)
             .map(|a| Attribute {
                 key: a.key.clone(),
                 r#type: a.r#type.clone(),
                 examples: a.examples.clone(),
                 common: a.common.clone(),
             })
             .collect();
-        let attribute_lookup =
-            |r: &weaver_resolved_schema::v2::attribute::AttributeRef| attributes.get(r.0 as usize);
+
         let mut metrics = Vec::new();
         for metric in schema.registry.metrics {
             let attributes = metric
@@ -447,14 +450,15 @@ mod tests {
             file_format: "2.0.0".to_owned(),
             schema_url: "https://example.com/schema".to_owned(),
             registry_id: "my-registry".to_owned(),
+            attribute_catalog: vec![attribute::Attribute {
+                key: "test.attr".to_owned(),
+                r#type: AttributeType::PrimitiveOrArray(PrimitiveOrArrayTypeSpec::String),
+                examples: None,
+                common: CommonFields::default(),
+            }],
             registry: v2::registry::Registry {
                 registry_url: "https://example.com/registry".to_owned(),
-                attributes: vec![attribute::Attribute {
-                    key: "test.attr".to_owned(),
-                    r#type: AttributeType::PrimitiveOrArray(PrimitiveOrArrayTypeSpec::String),
-                    examples: None,
-                    common: CommonFields::default(),
-                }],
+                attributes: vec![attribute::AttributeRef(0)],
                 spans: vec![span::Span {
                     r#type: SignalId::from("my-span".to_owned()),
                     kind: SpanKindSpec::Internal,
@@ -610,6 +614,7 @@ mod tests {
             file_format: "2.0.0".to_owned(),
             schema_url: "https://example.com/schema".to_owned(),
             registry_id: "my-registry".to_owned(),
+            attribute_catalog: vec![],
             registry: v2::registry::Registry {
                 registry_url: "https://example.com/registry".to_owned(),
                 attributes: vec![], // No attributes - This is the logic bug.
diff --git a/crates/weaver_resolved_schema/src/v2/catalog.rs b/crates/weaver_resolved_schema/src/v2/catalog.rs
@@ -75,6 +75,31 @@ impl Catalog {
     }
 }
 
+/// Provides methods which can resolve an `AttributeRef` into an `Attribute`.
+pub trait AttributeCatalog {
+    /// Returns the attribute from an attribute ref if it exists.
+    #[must_use]
+    fn attribute(&self, attribute_ref: &AttributeRef) -> Option<&Attribute>;
+    /// Returns the attribute name from an attribute ref if it exists
+    /// in the catalog or None if it does not exist.
+    #[must_use]
+    fn attribute_key(&self, attribute_ref: &AttributeRef) -> Option<&str> {
+        self.attribute(attribute_ref).map(|a| a.key.as_str())
+    }
+}
+
+impl AttributeCatalog for [Attribute] {
+    fn attribute(&self, attribute_ref: &AttributeRef) -> Option<&Attribute> {
+        self.get(attribute_ref.0 as usize)
+    }
+}
+
+impl AttributeCatalog for Vec<Attribute> {
+    fn attribute(&self, attribute_ref: &AttributeRef) -> Option<&Attribute> {
+        self.get(attribute_ref.0 as usize)
+    }
+}
+
 #[cfg(test)]
 mod test {
     use std::collections::BTreeMap;
diff --git a/crates/weaver_resolved_schema/src/v2/mod.rs b/crates/weaver_resolved_schema/src/v2/mod.rs
@@ -13,6 +13,7 @@ use weaver_semconv::{
 };
 
 use crate::v2::{
+    attribute::Attribute,
     attribute_group::AttributeGroup,
     catalog::Catalog,
     entity::Entity,
@@ -46,6 +47,8 @@ pub struct ResolvedTelemetrySchema {
     pub schema_url: String,
     /// The ID of the registry that this schema belongs to.
     pub registry_id: String,
+    /// Catalog of attributes. Note: this will include duplicates for the same key.
+    pub attribute_catalog: Vec<Attribute>,
     /// The registry that this schema belongs to.
     pub registry: Registry,
     /// Refinements for the registry
@@ -57,7 +60,7 @@ impl ResolvedTelemetrySchema {
     /// Statistics about this schema.
     pub fn stats(&self) -> Stats {
         Stats {
-            registry: self.registry.stats(),
+            registry: self.registry.stats(&self.attribute_catalog),
             refinements: self.refinements.stats(),
         }
     }
@@ -67,12 +70,14 @@ impl ResolvedTelemetrySchema {
 impl TryFrom<crate::ResolvedTelemetrySchema> for ResolvedTelemetrySchema {
     type Error = crate::error::Error;
     fn try_from(value: crate::ResolvedTelemetrySchema) -> Result<Self, Self::Error> {
-        let (registry, refinements) = convert_v1_to_v2(value.catalog, value.registry)?;
+        let (attribute_catalog, registry, refinements) =
+            convert_v1_to_v2(value.catalog, value.registry)?;
         Ok(ResolvedTelemetrySchema {
             // TODO - bump file format?
             file_format: value.file_format,
             schema_url: value.schema_url,
             registry_id: value.registry_id,
+            attribute_catalog,
             registry,
             refinements,
         })
@@ -95,15 +100,15 @@ fn fix_span_group_id(group_id: &str) -> SignalId {
 pub fn convert_v1_to_v2(
     c: crate::catalog::Catalog,
     r: crate::registry::Registry,
-) -> Result<(Registry, Refinements), crate::error::Error> {
+) -> Result<(Vec<Attribute>, Registry, Refinements), crate::error::Error> {
     // When pulling attributes, as we collapse things, we need to filter
     // to just unique.
-    let attributes: HashSet<attribute::Attribute> = c
+    let attributes: HashSet<Attribute> = c
         .attributes
         .iter()
         .cloned()
         .map(|a| {
-            attribute::Attribute {
+            Attribute {
                 key: a.name,
                 r#type: a.r#type,
                 examples: a.examples,
@@ -421,9 +426,31 @@ pub fn convert_v1_to_v2(
         }
     }
 
+    // Now we need to hunt for attribute definitions
+    let mut attributes = Vec::new();
+    for g in r.groups.iter() {
+        for a in g.attributes.iter() {
+            if let Some(attr) = c.attribute(a) {
+                // Attribute definitions do not have lineage.
+                let is_def = g
+                    .lineage
+                    .as_ref()
+                    .and_then(|l| l.attribute(&attr.name))
+                    .is_none();
+                if is_def {
+                    if let Some(v2) = v2_catalog.convert_ref(attr) {
+                        attributes.push(v2);
+                    } else {
+                        // TODO logic error!
+                    }
+                }
+            }
+        }
+    }
+
     let v2_registry = Registry {
         registry_url: r.registry_url,
-        attributes: v2_catalog.into(),
+        attributes,
         spans,
         metrics,
         events,
@@ -435,15 +462,19 @@ pub fn convert_v1_to_v2(
         metrics: metric_refinements,
         events: event_refinements,
     };
-    Ok((v2_registry, v2_refinements))
+    Ok((v2_catalog.into(), v2_registry, v2_refinements))
 }
 
 #[cfg(test)]
 mod tests {
 
     use weaver_semconv::{provenance::Provenance, stability::Stability};
 
-    use crate::{attribute::Attribute, lineage::GroupLineage, registry::Group};
+    use crate::{
+        attribute::Attribute,
+        lineage::{AttributeLineage, GroupLineage},
+        registry::Group,
+    };
 
     use super::*;
 
@@ -496,6 +527,8 @@ mod tests {
         ]);
         let mut refinement_span_lineage = GroupLineage::new(Provenance::new("tmp", "tmp"));
         refinement_span_lineage.extends("span.my-span");
+        refinement_span_lineage
+            .add_attribute_lineage("test.key".to_owned(), AttributeLineage::new("span.my-span"));
         let v1_registry = crate::registry::Registry {
             registry_url: "my.schema.url".to_owned(),
             groups: vec![
@@ -548,9 +581,11 @@ mod tests {
             ],
         };
 
-        let (v2_registry, v2_refinements) =
+        let (catalog, v2_registry, v2_refinements) =
             convert_v1_to_v2(v1_catalog, v1_registry).expect("Failed to convert v1 to v2");
         // assert only ONE attribute due to sharing.
+        assert_eq!(catalog.len(), 1);
+        // Assert one attribute shows up, due to lineage.
         assert_eq!(v2_registry.attributes.len(), 1);
         // assert attribute fields not shared show up on ref in span.
         assert_eq!(v2_registry.spans.len(), 1);
@@ -620,6 +655,8 @@ mod tests {
         ]);
         let mut refinement_metric_lineage = GroupLineage::new(Provenance::new("tmp", "tmp"));
         refinement_metric_lineage.extends("metric.http");
+        refinement_metric_lineage
+            .add_attribute_lineage("test.key".to_owned(), AttributeLineage::new("metric.http"));
         let v1_registry = crate::registry::Registry {
             registry_url: "my.schema.url".to_owned(),
             groups: vec![
@@ -672,7 +709,7 @@ mod tests {
             ],
         };
 
-        let (v2_registry, v2_refinements) =
+        let (_, v2_registry, v2_refinements) =
             convert_v1_to_v2(v1_catalog, v1_registry).expect("Failed to convert v1 to v2");
         // assert only ONE attribute due to sharing.
         assert_eq!(v2_registry.attributes.len(), 1);
@@ -746,7 +783,7 @@ mod tests {
             }],
         };
 
-        let (v2_registry, _) =
+        let (_, v2_registry, _) =
             convert_v1_to_v2(v1_catalog, v1_registry).expect("Failed to convert v1 to v2");
         assert_eq!(v2_registry.events.len(), 1);
         if let Some(event) = v2_registry.events.first() {
@@ -805,7 +842,7 @@ mod tests {
             }],
         };
 
-        let (v2_registry, _) =
+        let (_, v2_registry, _) =
             convert_v1_to_v2(v1_catalog, v1_registry).expect("Failed to convert v1 to v2");
         assert_eq!(v2_registry.entities.len(), 1);
         if let Some(entity) = v2_registry.entities.first() {
diff --git a/crates/weaver_resolved_schema/src/v2/registry.rs b/crates/weaver_resolved_schema/src/v2/registry.rs
@@ -7,8 +7,9 @@ use serde::{Deserialize, Serialize};
 use weaver_semconv::attribute::AttributeType;
 
 use crate::v2::{
-    attribute::{Attribute, AttributeRef},
+    attribute::AttributeRef,
     attribute_group::AttributeGroup,
+    catalog::AttributeCatalog,
     entity::Entity,
     event::Event,
     metric::Metric,
@@ -28,8 +29,8 @@ use crate::v2::{
 #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema)]
 #[serde(deny_unknown_fields)]
 pub struct Registry {
-    /// Catalog of attributes used in the schema.
-    pub attributes: Vec<Attribute>,
+    /// Catalog of attributes definitions.
+    pub attributes: Vec<AttributeRef>,
 
     /// Catalog of (public) attribute groups.
     pub attribute_groups: Vec<AttributeGroup>,
@@ -53,28 +54,18 @@ pub struct Registry {
 }
 
 impl Registry {
-    /// Returns the attribute from an attribute ref if it exists.
-    #[must_use]
-    pub fn attribute(&self, attribute_ref: &AttributeRef) -> Option<&Attribute> {
-        self.attributes.get(attribute_ref.0 as usize)
-    }
-    /// Returns the attribute name from an attribute ref if it exists
-    /// in the catalog or None if it does not exist.
-    #[must_use]
-    pub fn attribute_key(&self, attribute_ref: &AttributeRef) -> Option<&str> {
-        self.attributes
-            .get(attribute_ref.0 as usize)
-            .map(|attr| attr.key.as_ref())
-    }
-
     /// Returns the statistics for this registry.
     #[must_use]
-    pub fn stats(&self) -> RegistryStats {
+    pub fn stats<T: AttributeCatalog>(&self, catalog: &T) -> RegistryStats {
         let attributes = {
             let mut attribute_type_breakdown = BTreeMap::new();
             let mut stability_breakdown = HashMap::new();
             let mut deprecated_count = 0;
-            for attribute in &self.attributes {
+            for attribute in self
+                .attributes
+                .iter()
+                .filter_map(|ar| catalog.attribute(ar))
+            {
                 let attribute_type = if let AttributeType::Enum { members, .. } = &attribute.r#type
                 {
                     format!("enum(card:{:03})", members.len())
@@ -254,12 +245,26 @@ mod test {
         v2::{span::SpanName, CommonFields},
     };
 
-    use crate::v2::entity::EntityAttributeRef;
+    use crate::v2::{attribute::Attribute, entity::EntityAttributeRef};
 
     use super::*;
 
     #[test]
     fn test_stats() {
+        let catalog = vec![Attribute {
+            key: "key".to_owned(),
+            r#type: AttributeType::PrimitiveOrArray(
+                weaver_semconv::attribute::PrimitiveOrArrayTypeSpec::String,
+            ),
+            examples: None,
+            common: CommonFields {
+                brief: "test".to_owned(),
+                note: "".to_owned(),
+                stability: Stability::Stable,
+                deprecated: None,
+                annotations: BTreeMap::new(),
+            },
+        }];
         let registry = Registry {
             attribute_groups: vec![],
             registry_url: "https://opentelemetry.io/schemas/1.23.0".to_owned(),
@@ -311,22 +316,9 @@ mod test {
                     annotations: BTreeMap::new(),
                 },
             }],
-            attributes: vec![Attribute {
-                key: "key".to_owned(),
-                r#type: AttributeType::PrimitiveOrArray(
-                    weaver_semconv::attribute::PrimitiveOrArrayTypeSpec::String,
-                ),
-                examples: None,
-                common: CommonFields {
-                    brief: "test".to_owned(),
-                    note: "".to_owned(),
-                    stability: Stability::Stable,
-                    deprecated: None,
-                    annotations: BTreeMap::new(),
-                },
-            }],
+            attributes: vec![AttributeRef(0)],
         };
-        let stats = registry.stats();
+        let stats = registry.stats(&catalog);
         assert_eq!(stats.attributes.attribute_count, 1);
         assert_eq!(
             stats.attributes.attribute_type_breakdown.get("string"),
