Inherit overridden attribute properties (#204)

Co-authored-by: Christian Neumüller <christian+github@neumueller.me>

semantic-conventions/CHANGELOG.md

@@ -4,6 +4,9 @@ Please update the changelog as part of any significant pull request.
 
 ## Unreleased
 
+- When an attribute is referenced using `ref:` from a group that already inherits the attribute with `extends:`, resolve the reference to the closest inherited attribute instead of the primary definition. This makes a difference in case the inherited reference overwrites any properties.
+  ([#204](https://github.com/open-telemetry/build-tools/pull/204))
+
 ## v0.21.0
 
 - Render template-type attributes from yaml files
@@ -27,7 +30,7 @@ Please update the changelog as part of any significant pull request.
 
 - Allow multiple semconv in --only flag
   ([#157](https://github.com/open-telemetry/build-tools/pull/157))
-  
+
 ## v0.17.0
 
 - Rename Optional attribute requirement level to Opt-In
@@ -44,7 +47,7 @@ Please update the changelog as part of any significant pull request.
 
 ## v0.15.0
 
-- Add a semantic convention type for Metrics ("metric" and "metric_group") 
+- Add a semantic convention type for Metrics ("metric" and "metric_group")
   ([#79](https://github.com/open-telemetry/build-tools/pull/79))
 - Add a semantic convention type for generic attribute group ("attribute_group")
   ([#124](https://github.com/open-telemetry/build-tools/pull/124)).

semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py

@@ -502,6 +502,7 @@ def resolve_ref(self, semconv):
         attr: SemanticAttribute
         for attr in semconv.attributes:
             if attr.ref is not None and attr.attr_id is None:
+                attr = self._fill_inherited_attribute(attr, semconv)
                 # There are changes
                 fixpoint_ref = False
                 ref_attr = self._lookup_attribute(attr.ref)
@@ -510,20 +511,34 @@ def resolve_ref(self, semconv):
                         semconv._position,
                         f"Semantic Convention {semconv.semconv_id} reference `{attr.ref}` but it cannot be found!",
                     )
-                attr.attr_type = ref_attr.attr_type
-                if not attr.brief:
-                    attr.brief = ref_attr.brief
-                if not attr.requirement_level:
-                    attr.requirement_level = ref_attr.requirement_level
-                    if not attr.requirement_level_msg:
-                        attr.requirement_level_msg = ref_attr.requirement_level_msg
-                if not attr.note:
-                    attr.note = ref_attr.note
-                if attr.examples is None:
-                    attr.examples = ref_attr.examples
-                attr.attr_id = attr.ref
+                attr = self._merge_attribute(attr, ref_attr)
         return fixpoint_ref
 
+    def _fill_inherited_attribute(self, attr, semconv):
+        if attr.attr_id is not None:
+            return attr
+
+        if attr.ref in semconv.attrs_by_name.keys():
+            attr = self._merge_attribute(attr, semconv.attrs_by_name[attr.ref])
+        if semconv.extends in self.models:
+            attr = self._fill_inherited_attribute(attr, self.models[semconv.extends])
+        return attr
+
+    def _merge_attribute(self, child, parent):
+        child.attr_type = parent.attr_type
+        if not child.brief:
+            child.brief = parent.brief
+        if not child.requirement_level:
+            child.requirement_level = parent.requirement_level
+            if not child.requirement_level_msg:
+                child.requirement_level_msg = parent.requirement_level_msg
+        if not child.note:
+            child.note = parent.note
+        if child.examples is None:
+            child.examples = parent.examples
+        child.attr_id = parent.attr_id
+        return child
+
     def resolve_include(self, semconv):
         fixpoint_inc = True
         for constraint in semconv.constraints:

semantic-conventions/src/tests/data/markdown/ref_extends/expected.md

@@ -0,0 +1,29 @@
+# Spans
+
+<!-- semconv http.client.spans(full) -->
+| Attribute  | Type | Description  | Examples  | Requirement Level |
+|---|---|---|---|---|
+| [`server.address`](input_server.md) | string | Server component of Host header. (overridden brief) [1] | `foo.io` | Required |
+
+**[1]:** Note on the overridden attribute definition.
+
+Following attributes MUST be provided **at span creation time** (when provided at all), so they can be considered for sampling decisions:
+
+* [`server.address`](input_server.md)
+<!-- endsemconv -->
+
+# Metrics
+
+<!-- semconv http.client.request.duration.metric(metric_table) -->
+| Name     | Instrument Type | Unit (UCUM) | Description    |
+| -------- | --------------- | ----------- | -------------- |
+| `http.client.request.duration` | Histogram | `s` | Measures request duration. |
+<!-- endsemconv -->
+
+<!-- semconv http.client.request.duration.metric(full) -->
+| Attribute  | Type | Description  | Examples  | Requirement Level |
+|---|---|---|---|---|
+| [`server.address`](input_server.md) | string | Server component of Host header. (overridden brief) [1] | `foo.io` | Required |
+
+**[1]:** Note on the overridden attribute definition.
+<!-- endsemconv -->

semantic-conventions/src/tests/data/markdown/ref_extends/http.yaml

@@ -0,0 +1,26 @@
+groups:
+  - id: http.client.attributes
+    type: attribute_group
+    brief: 'This document defines semantic conventions for HTTP client attributes.'
+    attributes:
+      - ref: server.address
+        requirement_level: required
+        brief: 'Server component of Host header. (overridden brief)'
+        note: 'Note on the overridden attribute definition.'
+        examples: ['foo.io']
+
+  - id: http.client.spans
+    type: span
+    brief: 'This document defines semantic conventions for HTTP client Spans.'
+    extends: http.client.attributes
+    attributes:
+      - ref: server.address
+        sampling_relevant: true
+
+  - id: http.client.request.duration.metric
+    type: metric
+    metric_name: http.client.request.duration
+    brief: "Measures request duration."
+    instrument: histogram
+    unit: "s"
+    extends: http.client.attributes

semantic-conventions/src/tests/data/markdown/ref_extends/input.md

@@ -0,0 +1,12 @@
+# Spans
+
+<!-- semconv http.client.spans(full) -->
+<!-- endsemconv -->
+
+# Metrics
+
+<!-- semconv http.client.request.duration.metric(metric_table) -->
+<!-- endsemconv -->
+
+<!-- semconv http.client.request.duration.metric(full) -->
+<!-- endsemconv -->

semantic-conventions/src/tests/data/markdown/ref_extends/input_server.md

@@ -0,0 +1,4 @@
+# General
+
+<!-- semconv server -->
+<!-- endsemconv -->

semantic-conventions/src/tests/data/markdown/ref_extends/server.yaml

@@ -0,0 +1,11 @@
+groups:
+  - id: server
+    type: attribute_group
+    prefix: server
+    brief: 'This document defines semantic conventions for common server attributes.'
+    attributes:
+      - id: address
+        type: string
+        brief: 'Domain name. (original brief)'
+        examples: 'foo'
+        note: 'Note on the original attribute definition.'

semantic-conventions/src/tests/semconv/templating/test_markdown.py

@@ -27,6 +27,9 @@ class TestCorrectMarkdown(unittest.TestCase):
     def testRef(self):
         self.check("markdown/ref/")
 
+    def testRefExtends(self):
+        self.check("markdown/ref_extends/")
+
     def testInclude(self):
         self.check("markdown/include/")