From b982d9567f4fa6f292cfc20a78b196f7d51c4225 Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Wed, 27 Sep 2023 08:41:29 -0700 Subject: [PATCH] Inherit overridden attribute properties (#204) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Christian Neumüller --- semantic-conventions/CHANGELOG.md | 7 +++- .../semconv/model/semantic_convention.py | 39 +++++++++++++------ .../data/markdown/ref_extends/expected.md | 29 ++++++++++++++ .../tests/data/markdown/ref_extends/http.yaml | 26 +++++++++++++ .../tests/data/markdown/ref_extends/input.md | 12 ++++++ .../data/markdown/ref_extends/input_server.md | 4 ++ .../data/markdown/ref_extends/server.yaml | 11 ++++++ .../tests/semconv/templating/test_markdown.py | 3 ++ 8 files changed, 117 insertions(+), 14 deletions(-) create mode 100644 semantic-conventions/src/tests/data/markdown/ref_extends/expected.md create mode 100644 semantic-conventions/src/tests/data/markdown/ref_extends/http.yaml create mode 100644 semantic-conventions/src/tests/data/markdown/ref_extends/input.md create mode 100644 semantic-conventions/src/tests/data/markdown/ref_extends/input_server.md create mode 100644 semantic-conventions/src/tests/data/markdown/ref_extends/server.yaml diff --git a/semantic-conventions/CHANGELOG.md b/semantic-conventions/CHANGELOG.md index 9428e7d5..ef78e9cb 100644 --- a/semantic-conventions/CHANGELOG.md +++ b/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)). diff --git a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py index 4e533831..6f91ae2e 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py +++ b/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: diff --git a/semantic-conventions/src/tests/data/markdown/ref_extends/expected.md b/semantic-conventions/src/tests/data/markdown/ref_extends/expected.md new file mode 100644 index 00000000..ebb17dd4 --- /dev/null +++ b/semantic-conventions/src/tests/data/markdown/ref_extends/expected.md @@ -0,0 +1,29 @@ +# Spans + + +| 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) + + +# Metrics + + +| Name | Instrument Type | Unit (UCUM) | Description | +| -------- | --------------- | ----------- | -------------- | +| `http.client.request.duration` | Histogram | `s` | Measures request duration. | + + + +| 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. + diff --git a/semantic-conventions/src/tests/data/markdown/ref_extends/http.yaml b/semantic-conventions/src/tests/data/markdown/ref_extends/http.yaml new file mode 100644 index 00000000..89cd5a5e --- /dev/null +++ b/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 diff --git a/semantic-conventions/src/tests/data/markdown/ref_extends/input.md b/semantic-conventions/src/tests/data/markdown/ref_extends/input.md new file mode 100644 index 00000000..2e35e568 --- /dev/null +++ b/semantic-conventions/src/tests/data/markdown/ref_extends/input.md @@ -0,0 +1,12 @@ +# Spans + + + + +# Metrics + + + + + + diff --git a/semantic-conventions/src/tests/data/markdown/ref_extends/input_server.md b/semantic-conventions/src/tests/data/markdown/ref_extends/input_server.md new file mode 100644 index 00000000..540f456b --- /dev/null +++ b/semantic-conventions/src/tests/data/markdown/ref_extends/input_server.md @@ -0,0 +1,4 @@ +# General + + + \ No newline at end of file diff --git a/semantic-conventions/src/tests/data/markdown/ref_extends/server.yaml b/semantic-conventions/src/tests/data/markdown/ref_extends/server.yaml new file mode 100644 index 00000000..0b25c86f --- /dev/null +++ b/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.' diff --git a/semantic-conventions/src/tests/semconv/templating/test_markdown.py b/semantic-conventions/src/tests/semconv/templating/test_markdown.py index b34af20e..540243b4 100644 --- a/semantic-conventions/src/tests/semconv/templating/test_markdown.py +++ b/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/")