From 34326d6a1f996ca869b1584bc84949ffa7a4b371 Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Tue, 20 Feb 2024 08:46:31 -0800 Subject: [PATCH 01/11] add backward compatibility check mode --- semantic-conventions/dev-requirements.txt | 1 + .../src/opentelemetry/semconv/main.py | 85 +++++++-- .../semconv/model/semantic_attribute.py | 8 +- .../semconv/templating/compat.py | 99 +++++++++++ .../semconv/templating/markdown/options.py | 1 + .../vnext.yaml | 19 ++ .../vprev.yaml | 18 ++ .../compat/attribute_type_changed/vnext.yaml | 19 ++ .../compat/attribute_type_changed/vprev.yaml | 18 ++ .../data/compat/enum_type_changed/vnext.yaml | 17 ++ .../data/compat/enum_type_changed/vprev.yaml | 17 ++ .../data/compat/enum_value_removed/vnext.yaml | 17 ++ .../data/compat/enum_value_removed/vprev.yaml | 17 ++ .../compat/metric_attribute_added/vnext.yaml | 30 ++++ .../compat/metric_attribute_added/vprev.yaml | 29 +++ .../metric_instrument_changed/vnext.yaml | 23 +++ .../metric_instrument_changed/vprev.yaml | 23 +++ .../metric_stable_to_experimental/vnext.yaml | 23 +++ .../metric_stable_to_experimental/vprev.yaml | 23 +++ .../compat/metric_unit_changed/vnext.yaml | 23 +++ .../compat/metric_unit_changed/vprev.yaml | 23 +++ .../data/compat/removed_attribute/vnext.yaml | 6 + .../data/compat/removed_attribute/vprev.yaml | 18 ++ .../src/tests/data/compat/success/vnext.yaml | 53 ++++++ .../src/tests/data/compat/success/vprev.yaml | 44 +++++ .../src/tests/data/jinja/metrics/semconv.yml | 30 ++++ .../markdown/enum_extend_list/enum_attr.yaml | 27 +++ .../markdown/enum_extend_list/expected.md | 24 +++ .../data/markdown/enum_extend_list/input.md | 8 + .../tests/semconv/templating/test_compat.py | 167 ++++++++++++++++++ 30 files changed, 890 insertions(+), 20 deletions(-) create mode 100644 semantic-conventions/src/opentelemetry/semconv/templating/compat.py create mode 100644 semantic-conventions/src/tests/data/compat/attribute_stable_to_experimental/vnext.yaml create mode 100644 semantic-conventions/src/tests/data/compat/attribute_stable_to_experimental/vprev.yaml create mode 100644 semantic-conventions/src/tests/data/compat/attribute_type_changed/vnext.yaml create mode 100644 semantic-conventions/src/tests/data/compat/attribute_type_changed/vprev.yaml create mode 100644 semantic-conventions/src/tests/data/compat/enum_type_changed/vnext.yaml create mode 100644 semantic-conventions/src/tests/data/compat/enum_type_changed/vprev.yaml create mode 100644 semantic-conventions/src/tests/data/compat/enum_value_removed/vnext.yaml create mode 100644 semantic-conventions/src/tests/data/compat/enum_value_removed/vprev.yaml create mode 100644 semantic-conventions/src/tests/data/compat/metric_attribute_added/vnext.yaml create mode 100644 semantic-conventions/src/tests/data/compat/metric_attribute_added/vprev.yaml create mode 100644 semantic-conventions/src/tests/data/compat/metric_instrument_changed/vnext.yaml create mode 100644 semantic-conventions/src/tests/data/compat/metric_instrument_changed/vprev.yaml create mode 100644 semantic-conventions/src/tests/data/compat/metric_stable_to_experimental/vnext.yaml create mode 100644 semantic-conventions/src/tests/data/compat/metric_stable_to_experimental/vprev.yaml create mode 100644 semantic-conventions/src/tests/data/compat/metric_unit_changed/vnext.yaml create mode 100644 semantic-conventions/src/tests/data/compat/metric_unit_changed/vprev.yaml create mode 100644 semantic-conventions/src/tests/data/compat/removed_attribute/vnext.yaml create mode 100644 semantic-conventions/src/tests/data/compat/removed_attribute/vprev.yaml create mode 100644 semantic-conventions/src/tests/data/compat/success/vnext.yaml create mode 100644 semantic-conventions/src/tests/data/compat/success/vprev.yaml create mode 100644 semantic-conventions/src/tests/data/jinja/metrics/semconv.yml create mode 100644 semantic-conventions/src/tests/data/markdown/enum_extend_list/enum_attr.yaml create mode 100644 semantic-conventions/src/tests/data/markdown/enum_extend_list/expected.md create mode 100644 semantic-conventions/src/tests/data/markdown/enum_extend_list/input.md create mode 100644 semantic-conventions/src/tests/semconv/templating/test_compat.py diff --git a/semantic-conventions/dev-requirements.txt b/semantic-conventions/dev-requirements.txt index 4b496dc3..743b662f 100644 --- a/semantic-conventions/dev-requirements.txt +++ b/semantic-conventions/dev-requirements.txt @@ -4,3 +4,4 @@ pytest==8.0.1 flake8==7.0.0 pylint==3.0.3 isort==5.13.2 +requests=2.31.0 \ No newline at end of file diff --git a/semantic-conventions/src/opentelemetry/semconv/main.py b/semantic-conventions/src/opentelemetry/semconv/main.py index ce144d94..4a07022b 100644 --- a/semantic-conventions/src/opentelemetry/semconv/main.py +++ b/semantic-conventions/src/opentelemetry/semconv/main.py @@ -16,7 +16,12 @@ import argparse import glob +import os +import requests import sys +import tempfile +import zipfile + from typing import List from opentelemetry.semconv.model.semantic_convention import ( @@ -24,14 +29,15 @@ SemanticConventionSet, ) from opentelemetry.semconv.templating.code import CodeRenderer +from opentelemetry.semconv.templating.compat import CompatibilityChecker from opentelemetry.semconv.templating.markdown import MarkdownRenderer from opentelemetry.semconv.templating.markdown.options import MarkdownOptions -def parse_semconv(args, parser) -> SemanticConventionSet: - semconv = SemanticConventionSet(args.debug) - find_yaml(args) - for file in sorted(args.files): +def parse_semconv(yaml_root:str, exclude:str, debug:bool, parser) -> SemanticConventionSet: + semconv = SemanticConventionSet(debug) + files = find_yaml(yaml_root, exclude) + for file in sorted(files): if not file.endswith(".yaml") and not file.endswith(".yml"): parser.error(f"{file} is not a yaml file.") semconv.parse(file) @@ -64,8 +70,8 @@ def main(): parser = setup_parser() args = parser.parse_args() check_args(args, parser) - semconv = parse_semconv(args, parser) - semconv_filter = parse_only_filter(args, parser) + semconv = parse_semconv(args.yaml_root, args.exclude, args.debug, parser) + semconv_filter = parse_only_filter(args.only, parser) filter_semconv(semconv, semconv_filter) if len(semconv.models) == 0: parser.error("No semantic convention model found!") @@ -76,7 +82,8 @@ def main(): renderer.render(semconv, args.template, args.output, args.pattern) elif args.flavor == "markdown": process_markdown(semconv, args) - + elif args.flavor == "compat": + check_compatibility(semconv, args, parser) def process_markdown(semconv, args): options = MarkdownOptions( @@ -91,17 +98,30 @@ def process_markdown(semconv, args): md_renderer = MarkdownRenderer(args.markdown_root, semconv, options) md_renderer.render_md() +def check_compatibility(semconv, args, parser): + prev_semconv_path = download_previous_version(args.previous_version) + prev_semconv = parse_semconv(prev_semconv_path, args.exclude, args.debug, parser) + compat_checker = CompatibilityChecker(semconv, prev_semconv) + errors = compat_checker.check() + + if errors: + print(f"Found {len(errors)} compatibility issues:") + for error in errors: + print(f"\t {error}") + + sys.exit(1) -def find_yaml(args): - if args.yaml_root is not None: + +def find_yaml(yaml_root:str, exclude:str) -> List[str]: + + if yaml_root is not None: exclude = set( - exclude_file_list(args.yaml_root if args.yaml_root else "", args.exclude) + exclude_file_list(yaml_root if yaml_root else "", exclude) ) yaml_files = set( - glob.glob(f"{args.yaml_root}/**/*.yaml", recursive=True) - ).union(set(glob.glob(f"{args.yaml_root}/**/*.yml", recursive=True))) - file_names = yaml_files - exclude - args.files.extend(file_names) + glob.glob(f"{yaml_root}/**/*.yaml", recursive=True) + ).union(set(glob.glob(f"{yaml_root}/**/*.yml", recursive=True))) + return yaml_files - exclude def check_args(arguments, parser): @@ -110,11 +130,11 @@ def check_args(arguments, parser): parser.error("Either --yaml-root or YAML_FILE must be present") -def parse_only_filter(arguments, parser): - if not arguments.only: +def parse_only_filter(only:str, parser) -> List[str]: + if not only: return None - types = [t.strip() for t in arguments.only.split(",")] + types = [t.strip() for t in only.split(",")] unknown_types = [t for t in types if t not in CONVENTION_CLS_BY_GROUP_TYPE.keys()] if unknown_types: parser.error( @@ -163,7 +183,6 @@ def add_code_parser(subparsers): action="store_true", ) - def add_md_parser(subparsers): parser = subparsers.add_parser("markdown") parser.add_argument( @@ -188,6 +207,12 @@ def add_md_parser(subparsers): required=False, action="store_true", ) + parser.add_argument( + "--check-compat", + help="Don't write the files, but check backward compatibility with specified older version of semantic conventions.", + type=str, + required=False + ) parser.add_argument( "--md-use-badges", help="Use stability badges instead of labels for attributes.", @@ -215,6 +240,14 @@ def add_md_parser(subparsers): action="store_false", ) +def add_compat_check_parser(subparsers): + parser = subparsers.add_parser("compat") + parser.add_argument( + "--previous-version", + help="Check backward compatibility with specified older version of semantic conventions.", + type=str, + required=True + ) def setup_parser(): parser = argparse.ArgumentParser( @@ -258,9 +291,25 @@ def setup_parser(): subparsers = parser.add_subparsers(dest="flavor") add_code_parser(subparsers) add_md_parser(subparsers) + add_compat_check_parser(subparsers) return parser +def download_previous_version(version: str) -> str: + filename = f"v{version}.zip" + tmppath = tempfile.mkdtemp() + path_to_zip = os.path.join(tmppath, filename) + path_to_semconv = os.path.join(tmppath, f"v{version}") + + semconv_vprev = f"https://github.com/open-telemetry/semantic-conventions/archive/{filename}" + + request = requests.get(semconv_vprev, allow_redirects=True) + open(path_to_zip, 'wb').write(request.content) + + with zipfile.ZipFile(path_to_zip, 'r') as zip_ref: + zip_ref.extractall(path_to_semconv) + + return os.path.join(path_to_semconv, f"semantic-conventions-{version}", "model") if __name__ == "__main__": main() diff --git a/semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py b/semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py index e27f654a..f1bdac62 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py @@ -291,8 +291,12 @@ def parse_stability(stability, position_data): val = stability_value_map.get(stability) if val is not None: return val - msg = f"Value '{stability}' is not allowed as a stability marker" - raise ValidationError.from_yaml_pos(position_data["stability"], msg) + msg = f"Value '{stability}' is not allowed as a stability marker, setting to experimental!" + + print(msg) + return StabilityLevel.EXPERIMENTAL + + #raise ValidationError.from_yaml_pos(position_data["stability"], msg) @staticmethod def parse_deprecated(deprecated, position_data): diff --git a/semantic-conventions/src/opentelemetry/semconv/templating/compat.py b/semantic-conventions/src/opentelemetry/semconv/templating/compat.py new file mode 100644 index 00000000..48569f26 --- /dev/null +++ b/semantic-conventions/src/opentelemetry/semconv/templating/compat.py @@ -0,0 +1,99 @@ +from opentelemetry.semconv.model.semantic_attribute import ( + EnumMember, + RequirementLevel, + SemanticAttribute, + StabilityLevel, +) +from opentelemetry.semconv.model.semantic_convention import ( + MetricSemanticConvention, + SemanticConventionSet, +) + +class Error: + signal: str + name: str + message: str + + def __init__(self, signal: str, name: str, message: str): + self.signal = signal + self.name = name + self.message = message + + def __str__(self): + return f"\t {self.signal} '{self.name}': {self.message}" + +class CompatibilityChecker: + previous_version: str + + def __init__(self, current_semconv: SemanticConventionSet, + previous_semconv: SemanticConventionSet): + self.current_semconv = current_semconv + self.previous_semconv = previous_semconv + + # TODO: attribute templates + # stability + # docs + def check( + self + ) -> list[Error]: + errors = [] + for semconv in self.previous_semconv.models.values(): + for prev_attr in semconv.attributes: + if prev_attr.is_local and prev_attr.attr_id is not None and prev_attr.ref is None: + self._check_attribute(prev_attr, errors) + if isinstance(semconv, MetricSemanticConvention): + self._check_metric(semconv, errors) + return errors + + + def _check_attribute(self, prev: SemanticAttribute, errors: list[str]): + cur = self.current_semconv._lookup_attribute(prev.fqn) + if (cur is None): + errors.append(Error("attribute", prev.fqn, "was removed")) + return + + if prev.stability == StabilityLevel.STABLE: + if cur.stability != prev.stability: + errors.append(Error("attribute", prev.fqn, f"stability changed from '{prev.stability}' to '{cur.stability}'")) + + if prev.is_enum: + if not cur.is_enum: + errors.append(Error("attribute", prev.fqn, f"type changed from '{prev.attr_type}' to '{cur.attr_type}'")) + else: + if cur.attr_type.enum_type != prev.attr_type.enum_type: + errors.append(Error("attribute", prev.fqn, f"enum type changed from '{prev.attr_type.enum_type}' to '{cur.attr_type.enum_type}'")) + for member in prev.attr_type.members: + self._check_member(prev.fqn, member, cur.attr_type.members, errors) + elif cur.attr_type != prev.attr_type: + errors.append(Error("attribute", prev.fqn, f"type changed from '{prev.attr_type}' to '{cur.attr_type}'")) + + + def _check_member(self, fqn: str, prev: EnumMember, members: list[EnumMember], errors: list[str]): + for member in members: + if prev.value == member.value: + return + + errors.append(Error("attribute", fqn, f"enum member with value '{prev.value}' was removed")) + + + def _check_metric(self, prev: MetricSemanticConvention, errors: list[str]): + for cur in self.current_semconv.models.values(): + if isinstance(cur, MetricSemanticConvention) and cur.metric_name == prev.metric_name: + if prev.stability == StabilityLevel.STABLE: + if cur.stability != prev.stability: + errors.append(Error("metric", prev.metric_name, f"stability changed from '{prev.stability}' to '{cur.stability}'")) + if cur.unit != prev.unit: + errors.append(Error("metric", prev.metric_name, f"unit changed from '{prev.unit}' to '{cur.unit}'")) + if cur.instrument != prev.instrument: + errors.append(Error("metric", prev.metric_name, f"instrument changed from '{prev.instrument}' to '{cur.instrument}'")) + self._check_metric_attributes(prev, cur, errors) + return + + errors.append(Error("metric", prev.metric_name, "was removed")) + + def _check_metric_attributes(self, prev: MetricSemanticConvention, cur: MetricSemanticConvention, errors: list[str]): + if prev.stability == StabilityLevel.STABLE: + prev_default_attributes = [attr.fqn for attr in prev.attributes if attr.requirement_level != RequirementLevel.OPT_IN] + cur_default_attributes = [attr.fqn for attr in cur.attributes if attr.requirement_level != RequirementLevel.OPT_IN] + if prev_default_attributes != cur_default_attributes: + errors.append(Error("metric", prev.metric_name, f"attributes changed from '{prev_default_attributes}' to '{cur_default_attributes}'")) diff --git a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/options.py b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/options.py index 419ef8f1..528f4f4d 100644 --- a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/options.py +++ b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/options.py @@ -19,6 +19,7 @@ @dataclass() class MarkdownOptions: check_only: bool = False + check_compatibility_with: str = None enable_stable: bool = False enable_experimental: bool = False enable_deprecated: bool = True diff --git a/semantic-conventions/src/tests/data/compat/attribute_stable_to_experimental/vnext.yaml b/semantic-conventions/src/tests/data/compat/attribute_stable_to_experimental/vnext.yaml new file mode 100644 index 00000000..78882a26 --- /dev/null +++ b/semantic-conventions/src/tests/data/compat/attribute_stable_to_experimental/vnext.yaml @@ -0,0 +1,19 @@ +groups: + - id: first + type: attribute_group + brief: "First group." + note: "First group note." + prefix: "first" + attributes: + - id: first_attr + type: string + brief: "First attribute." + note: "First attribute note." + examples: ["first"] + stability: stable + - id: second_attr + type: int + brief: "Second attribute." + note: "Second attribute note." + stability: experimental + examples: [2] \ No newline at end of file diff --git a/semantic-conventions/src/tests/data/compat/attribute_stable_to_experimental/vprev.yaml b/semantic-conventions/src/tests/data/compat/attribute_stable_to_experimental/vprev.yaml new file mode 100644 index 00000000..9f3f8c73 --- /dev/null +++ b/semantic-conventions/src/tests/data/compat/attribute_stable_to_experimental/vprev.yaml @@ -0,0 +1,18 @@ +groups: + - id: first + type: attribute_group + brief: "first group" + note: "first group note" + prefix: "first" + attributes: + - id: first_attr + type: string + brief: "first attribute" + note: "first attribute note" + examples: "first example" + - id: second_attr + type: int + brief: "second attribute" + note: "second attribute note" + stability: stable + examples: 2 \ No newline at end of file diff --git a/semantic-conventions/src/tests/data/compat/attribute_type_changed/vnext.yaml b/semantic-conventions/src/tests/data/compat/attribute_type_changed/vnext.yaml new file mode 100644 index 00000000..b47d95bf --- /dev/null +++ b/semantic-conventions/src/tests/data/compat/attribute_type_changed/vnext.yaml @@ -0,0 +1,19 @@ +groups: + - id: first + type: attribute_group + brief: "First group." + note: "First group note." + prefix: "first" + attributes: + - id: first_attr + type: int + brief: "First attribute." + note: "First attribute note." + examples: [1] + stability: stable + - id: second_attr + type: string + brief: "Second attribute." + note: "Second attribute note." + stability: stable + examples: ["two"] \ No newline at end of file diff --git a/semantic-conventions/src/tests/data/compat/attribute_type_changed/vprev.yaml b/semantic-conventions/src/tests/data/compat/attribute_type_changed/vprev.yaml new file mode 100644 index 00000000..9f3f8c73 --- /dev/null +++ b/semantic-conventions/src/tests/data/compat/attribute_type_changed/vprev.yaml @@ -0,0 +1,18 @@ +groups: + - id: first + type: attribute_group + brief: "first group" + note: "first group note" + prefix: "first" + attributes: + - id: first_attr + type: string + brief: "first attribute" + note: "first attribute note" + examples: "first example" + - id: second_attr + type: int + brief: "second attribute" + note: "second attribute note" + stability: stable + examples: 2 \ No newline at end of file diff --git a/semantic-conventions/src/tests/data/compat/enum_type_changed/vnext.yaml b/semantic-conventions/src/tests/data/compat/enum_type_changed/vnext.yaml new file mode 100644 index 00000000..6875fb50 --- /dev/null +++ b/semantic-conventions/src/tests/data/compat/enum_type_changed/vnext.yaml @@ -0,0 +1,17 @@ +groups: + - id: first + type: attribute_group + brief: "First group." + note: "First group note." + prefix: "first" + attributes: + - id: third_attr + type: + members: + - id: enum_one + brief: "enum one" + value: 1 + brief: "third attribute" + note: "third attribute note" + examples: [1] + stability: stable \ No newline at end of file diff --git a/semantic-conventions/src/tests/data/compat/enum_type_changed/vprev.yaml b/semantic-conventions/src/tests/data/compat/enum_type_changed/vprev.yaml new file mode 100644 index 00000000..ab72dfa0 --- /dev/null +++ b/semantic-conventions/src/tests/data/compat/enum_type_changed/vprev.yaml @@ -0,0 +1,17 @@ +groups: + - id: first + type: attribute_group + brief: "first group" + note: "first group note" + prefix: "first" + attributes: + - id: third_attr + type: + members: + - id: enum_one + brief: "enum one" + value: "one" + brief: "third attribute" + note: "third attribute note" + examples: ["one"] + stability: stable \ No newline at end of file diff --git a/semantic-conventions/src/tests/data/compat/enum_value_removed/vnext.yaml b/semantic-conventions/src/tests/data/compat/enum_value_removed/vnext.yaml new file mode 100644 index 00000000..88f07223 --- /dev/null +++ b/semantic-conventions/src/tests/data/compat/enum_value_removed/vnext.yaml @@ -0,0 +1,17 @@ +groups: + - id: first + type: attribute_group + brief: "First group." + note: "First group note." + prefix: "first" + attributes: + - id: third_attr + type: + members: + - id: enum_one + brief: "enum one" + value: "1" + brief: "third attribute" + note: "third attribute note" + examples: ["one"] + stability: stable \ No newline at end of file diff --git a/semantic-conventions/src/tests/data/compat/enum_value_removed/vprev.yaml b/semantic-conventions/src/tests/data/compat/enum_value_removed/vprev.yaml new file mode 100644 index 00000000..ab72dfa0 --- /dev/null +++ b/semantic-conventions/src/tests/data/compat/enum_value_removed/vprev.yaml @@ -0,0 +1,17 @@ +groups: + - id: first + type: attribute_group + brief: "first group" + note: "first group note" + prefix: "first" + attributes: + - id: third_attr + type: + members: + - id: enum_one + brief: "enum one" + value: "one" + brief: "third attribute" + note: "third attribute note" + examples: ["one"] + stability: stable \ No newline at end of file diff --git a/semantic-conventions/src/tests/data/compat/metric_attribute_added/vnext.yaml b/semantic-conventions/src/tests/data/compat/metric_attribute_added/vnext.yaml new file mode 100644 index 00000000..7e2fc969 --- /dev/null +++ b/semantic-conventions/src/tests/data/compat/metric_attribute_added/vnext.yaml @@ -0,0 +1,30 @@ +groups: + - id: first + type: attribute_group + brief: "first group" + note: "first group note" + prefix: "first" + attributes: + - id: first_attr + type: string + brief: "first attribute" + note: "first attribute note" + examples: "first example" + stability: stable + - id: second_attr + type: int + brief: "second attribute" + note: "second attribute note" + stability: stable + examples: 2 + - id: metric_one + type: metric + metric_name: "metric_one" + brief: "Metric one." + note: "Metric one note." + stability: stable + unit: "s" + instrument: histogram + attributes: + - ref: first.first_attr + - ref: first.second_attr \ No newline at end of file diff --git a/semantic-conventions/src/tests/data/compat/metric_attribute_added/vprev.yaml b/semantic-conventions/src/tests/data/compat/metric_attribute_added/vprev.yaml new file mode 100644 index 00000000..b7819edf --- /dev/null +++ b/semantic-conventions/src/tests/data/compat/metric_attribute_added/vprev.yaml @@ -0,0 +1,29 @@ +groups: + - id: first + type: attribute_group + brief: "first group" + note: "first group note" + prefix: "first" + attributes: + - id: first_attr + type: string + brief: "first attribute" + note: "first attribute note" + examples: "first example" + stability: stable + - id: second_attr + type: int + brief: "second attribute" + note: "second attribute note" + stability: stable + examples: 2 + - id: metric_one + type: metric + metric_name: "metric_one" + brief: "metric one" + note: "metric one note" + stability: stable + unit: "s" + instrument: histogram + attributes: + - ref: first.first_attr diff --git a/semantic-conventions/src/tests/data/compat/metric_instrument_changed/vnext.yaml b/semantic-conventions/src/tests/data/compat/metric_instrument_changed/vnext.yaml new file mode 100644 index 00000000..ae373320 --- /dev/null +++ b/semantic-conventions/src/tests/data/compat/metric_instrument_changed/vnext.yaml @@ -0,0 +1,23 @@ +groups: + - id: first + type: attribute_group + brief: "first group" + note: "first group note" + prefix: "first" + attributes: + - id: first_attr + type: string + brief: "first attribute" + note: "first attribute note" + examples: "first example" + stability: stable + - id: metric_one + type: metric + metric_name: "metric_one" + brief: "Metric one." + note: "Metric one note." + stability: stable + unit: "s" + instrument: histogram + attributes: + - ref: first.first_attr diff --git a/semantic-conventions/src/tests/data/compat/metric_instrument_changed/vprev.yaml b/semantic-conventions/src/tests/data/compat/metric_instrument_changed/vprev.yaml new file mode 100644 index 00000000..deeb61b5 --- /dev/null +++ b/semantic-conventions/src/tests/data/compat/metric_instrument_changed/vprev.yaml @@ -0,0 +1,23 @@ +groups: + - id: first + type: attribute_group + brief: "first group" + note: "first group note" + prefix: "first" + attributes: + - id: first_attr + type: string + brief: "first attribute" + note: "first attribute note" + examples: "first example" + stability: stable + - id: metric_one + type: metric + metric_name: "metric_one" + brief: "metric one" + note: "metric one note" + stability: stable + unit: "s" + instrument: counter + attributes: + - ref: first.first_attr diff --git a/semantic-conventions/src/tests/data/compat/metric_stable_to_experimental/vnext.yaml b/semantic-conventions/src/tests/data/compat/metric_stable_to_experimental/vnext.yaml new file mode 100644 index 00000000..22f8d3f3 --- /dev/null +++ b/semantic-conventions/src/tests/data/compat/metric_stable_to_experimental/vnext.yaml @@ -0,0 +1,23 @@ +groups: + - id: first + type: attribute_group + brief: "first group" + note: "first group note" + prefix: "first" + attributes: + - id: first_attr + type: string + brief: "first attribute" + note: "first attribute note" + examples: "first example" + stability: stable + - id: metric_one + type: metric + metric_name: "metric_one" + brief: "Metric one." + note: "Metric one note." + stability: experimental + unit: "s" + instrument: histogram + attributes: + - ref: first.first_attr diff --git a/semantic-conventions/src/tests/data/compat/metric_stable_to_experimental/vprev.yaml b/semantic-conventions/src/tests/data/compat/metric_stable_to_experimental/vprev.yaml new file mode 100644 index 00000000..5bfd15b0 --- /dev/null +++ b/semantic-conventions/src/tests/data/compat/metric_stable_to_experimental/vprev.yaml @@ -0,0 +1,23 @@ +groups: + - id: first + type: attribute_group + brief: "first group" + note: "first group note" + prefix: "first" + attributes: + - id: first_attr + type: string + brief: "first attribute" + note: "first attribute note" + examples: "first example" + stability: stable + - id: metric_one + type: metric + metric_name: "metric_one" + brief: "metric one" + note: "metric one note" + stability: stable + unit: "s" + instrument: histogram + attributes: + - ref: first.first_attr diff --git a/semantic-conventions/src/tests/data/compat/metric_unit_changed/vnext.yaml b/semantic-conventions/src/tests/data/compat/metric_unit_changed/vnext.yaml new file mode 100644 index 00000000..ae373320 --- /dev/null +++ b/semantic-conventions/src/tests/data/compat/metric_unit_changed/vnext.yaml @@ -0,0 +1,23 @@ +groups: + - id: first + type: attribute_group + brief: "first group" + note: "first group note" + prefix: "first" + attributes: + - id: first_attr + type: string + brief: "first attribute" + note: "first attribute note" + examples: "first example" + stability: stable + - id: metric_one + type: metric + metric_name: "metric_one" + brief: "Metric one." + note: "Metric one note." + stability: stable + unit: "s" + instrument: histogram + attributes: + - ref: first.first_attr diff --git a/semantic-conventions/src/tests/data/compat/metric_unit_changed/vprev.yaml b/semantic-conventions/src/tests/data/compat/metric_unit_changed/vprev.yaml new file mode 100644 index 00000000..acaed738 --- /dev/null +++ b/semantic-conventions/src/tests/data/compat/metric_unit_changed/vprev.yaml @@ -0,0 +1,23 @@ +groups: + - id: first + type: attribute_group + brief: "first group" + note: "first group note" + prefix: "first" + attributes: + - id: first_attr + type: string + brief: "first attribute" + note: "first attribute note" + examples: "first example" + stability: stable + - id: metric_one + type: metric + metric_name: "metric_one" + brief: "metric one" + note: "metric one note" + stability: stable + unit: "ms" + instrument: histogram + attributes: + - ref: first.first_attr diff --git a/semantic-conventions/src/tests/data/compat/removed_attribute/vnext.yaml b/semantic-conventions/src/tests/data/compat/removed_attribute/vnext.yaml new file mode 100644 index 00000000..d6f58e86 --- /dev/null +++ b/semantic-conventions/src/tests/data/compat/removed_attribute/vnext.yaml @@ -0,0 +1,6 @@ +groups: + - id: first + type: attribute_group + brief: "First group." + note: "First group note." + prefix: "first" diff --git a/semantic-conventions/src/tests/data/compat/removed_attribute/vprev.yaml b/semantic-conventions/src/tests/data/compat/removed_attribute/vprev.yaml new file mode 100644 index 00000000..9f3f8c73 --- /dev/null +++ b/semantic-conventions/src/tests/data/compat/removed_attribute/vprev.yaml @@ -0,0 +1,18 @@ +groups: + - id: first + type: attribute_group + brief: "first group" + note: "first group note" + prefix: "first" + attributes: + - id: first_attr + type: string + brief: "first attribute" + note: "first attribute note" + examples: "first example" + - id: second_attr + type: int + brief: "second attribute" + note: "second attribute note" + stability: stable + examples: 2 \ No newline at end of file diff --git a/semantic-conventions/src/tests/data/compat/success/vnext.yaml b/semantic-conventions/src/tests/data/compat/success/vnext.yaml new file mode 100644 index 00000000..9ee6f5fe --- /dev/null +++ b/semantic-conventions/src/tests/data/compat/success/vnext.yaml @@ -0,0 +1,53 @@ +groups: + - id: first + type: attribute_group + brief: "First group." + note: "First group note." + prefix: "first" + attributes: + - id: first_attr + type: int # was an experimental attribute, type can change + brief: "First attribute." + note: "First attribute note." + stability: stable + examples: [1] + - id: second_attr + type: int + brief: "Second attribute." + note: "Second attribute note." + stability: stable + examples: [2] + - id: third_attr + type: + members: + - id: one + brief: "Enum one." + value: "one" + - id: two + brief: "Enum two." + value: "two" + brief: "Third attribute." + note: "Third attribute note." + examples: ["two"] + stability: stable + - id: forth_attr + type: boolean + brief: "forth attribute" + note: "forth attribute note" + examples: [True] + stability: stable + - id: metric_one + type: metric + metric_name: "metric_one" + brief: "Metric one." + note: "Metric one note." + stability: stable + unit: "s" + instrument: histogram + attributes: + - ref: first.second_attr + requirement_level: required + - ref: first.third_attr + requirement_level: recommended + - ref: first.forth_attr + requirement_level: opt_in \ No newline at end of file diff --git a/semantic-conventions/src/tests/data/compat/success/vprev.yaml b/semantic-conventions/src/tests/data/compat/success/vprev.yaml new file mode 100644 index 00000000..023eb387 --- /dev/null +++ b/semantic-conventions/src/tests/data/compat/success/vprev.yaml @@ -0,0 +1,44 @@ +groups: + - id: first + type: attribute_group + brief: "first group" + note: "first group note" + prefix: "first" + attributes: + - id: first_attr + type: string + brief: "first attribute" + note: "first attribute note" + examples: "first example" + - id: second_attr + type: int + brief: "second attribute" + note: "second attribute note" + stability: stable + examples: 2 + - id: third_attr + type: + members: + - id: enum_one + brief: "enum one" + value: "one" + - id: enum_two + brief: "enum two" + value: "two" + brief: "third attribute" + note: "third attribute note" + examples: ["one"] + stability: stable + - id: metric_one + type: metric + metric_name: "metric_one" + brief: "metric one" + note: "metric one note" + stability: stable + unit: "s" + instrument: histogram + attributes: + - ref: first.second_attr + requirement_level: required + - ref: first.third_attr + requirement_level: recommended diff --git a/semantic-conventions/src/tests/data/jinja/metrics/semconv.yml b/semantic-conventions/src/tests/data/jinja/metrics/semconv.yml new file mode 100644 index 00000000..3d2c670b --- /dev/null +++ b/semantic-conventions/src/tests/data/jinja/metrics/semconv.yml @@ -0,0 +1,30 @@ +groups: + - id: first_group_id + type: attribute_group + brief: first description + prefix: first + attributes: + - id: attr_one + type: boolean + brief: short description of attr_one + + - id: first_metric_id + brief: first metric description + metric_name: first.metric + instrument: counter + type: metric + unit: "{one}" + stability: stable + extends: first_group_id + + - id: second_group_id + brief: second metric description + metric_name: second_group.metric + type: metric + instrument: histogram + unit: "s" + prefix: second_group + attributes: + - id: attr_two + type: int + brief: short description of attr_two diff --git a/semantic-conventions/src/tests/data/markdown/enum_extend_list/enum_attr.yaml b/semantic-conventions/src/tests/data/markdown/enum_extend_list/enum_attr.yaml new file mode 100644 index 00000000..1714c7aa --- /dev/null +++ b/semantic-conventions/src/tests/data/markdown/enum_extend_list/enum_attr.yaml @@ -0,0 +1,27 @@ +groups: + - id: db + prefix: db + brief: "" + attributes: + - id: system + type: + allow_custom_values: false + members: + - id: mysql + value: "mysql" + - id: postgresql + value: "postgresql" + - id: mssql + value: "mssql" + brief: "" + examples: ["mysql"] + - id: db.redis + brief: "" + prefix: db.redis + attributes: + - ref: system + type: + members: + - id: redis + value: "redis" + examples: ["redis"] diff --git a/semantic-conventions/src/tests/data/markdown/enum_extend_list/expected.md b/semantic-conventions/src/tests/data/markdown/enum_extend_list/expected.md new file mode 100644 index 00000000..ac5222cd --- /dev/null +++ b/semantic-conventions/src/tests/data/markdown/enum_extend_list/expected.md @@ -0,0 +1,24 @@ + +| Attribute | Type | Description | Examples | Requirement Level | +|---|---|---|---|---| +| `db.system` | string | | `"one"` | Recommended | + +`db.system` MUST be one of the following: + +| Value | Description | +|---|---| +| `one` | one | + + + +| Attribute | Type | Description | Examples | Requirement Level | +|---|---|---|---|---| +| `first.enum_attr` | string | | `"one"` | Recommended | + +`first.enum_attr` MUST be one of the following: + +| Value | Description | +|---|---| +| `one` | one | +| `two` | two | + diff --git a/semantic-conventions/src/tests/data/markdown/enum_extend_list/input.md b/semantic-conventions/src/tests/data/markdown/enum_extend_list/input.md new file mode 100644 index 00000000..8bc34d41 --- /dev/null +++ b/semantic-conventions/src/tests/data/markdown/enum_extend_list/input.md @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/semantic-conventions/src/tests/semconv/templating/test_compat.py b/semantic-conventions/src/tests/semconv/templating/test_compat.py new file mode 100644 index 00000000..3a2c8d37 --- /dev/null +++ b/semantic-conventions/src/tests/semconv/templating/test_compat.py @@ -0,0 +1,167 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import io +import os +import unittest +from pathlib import Path + +from opentelemetry.semconv.model.semantic_convention import SemanticConventionSet +from opentelemetry.semconv.templating.compat import CompatibilityChecker, Error + +class TestCompatibility(unittest.TestCase): + def testSuccess(self): + cur = self.parse_semconv("compat/success/vnext.yaml") + prev = self.parse_semconv("compat/success/vprev.yaml") + checker = CompatibilityChecker(cur, prev) + errors = checker.check() + self.print_errors(errors) + assert len(errors) == 0 + + def testRemovedAttribute(self): + cur = self.parse_semconv("compat/removed_attribute/vnext.yaml") + prev = self.parse_semconv("compat/removed_attribute/vprev.yaml") + checker = CompatibilityChecker(cur, prev) + errors = checker.check() + self.print_errors(errors) + assert len(errors) == 2 + assert errors[0].signal == "attribute" + assert errors[0].name == "first.first_attr" + assert errors[0].message == "was removed" + + assert errors[1].signal == "attribute" + assert errors[1].name == "first.second_attr" + assert errors[1].message == "was removed" + + def testAttributeStableToExperimental(self): + cur = self.parse_semconv("compat/attribute_stable_to_experimental/vnext.yaml") + prev = self.parse_semconv("compat/attribute_stable_to_experimental/vprev.yaml") + checker = CompatibilityChecker(cur, prev) + errors = checker.check() + self.print_errors(errors) + assert len(errors) == 1 + assert errors[0].signal == "attribute" + assert errors[0].name == "first.second_attr" + assert errors[0].message == "stability changed from 'StabilityLevel.STABLE' to 'StabilityLevel.EXPERIMENTAL'" + + def testMetricStableToExperimental(self): + cur = self.parse_semconv("compat/metric_stable_to_experimental/vnext.yaml") + prev = self.parse_semconv("compat/metric_stable_to_experimental/vprev.yaml") + checker = CompatibilityChecker(cur, prev) + errors = checker.check() + self.print_errors(errors) + assert len(errors) == 1 + assert errors[0].signal == "metric" + assert errors[0].name == "metric_one" + assert errors[0].message == "stability changed from 'StabilityLevel.STABLE' to 'StabilityLevel.EXPERIMENTAL'" + + def testMetricInstrumentChanged(self): + cur = self.parse_semconv("compat/metric_instrument_changed/vnext.yaml") + prev = self.parse_semconv("compat/metric_instrument_changed/vprev.yaml") + checker = CompatibilityChecker(cur, prev) + errors = checker.check() + self.print_errors(errors) + assert len(errors) == 1 + assert errors[0].signal == "metric" + assert errors[0].name == "metric_one" + assert errors[0].message == "instrument changed from 'counter' to 'histogram'" + + def testMetricUnitChanged(self): + cur = self.parse_semconv("compat/metric_unit_changed/vnext.yaml") + prev = self.parse_semconv("compat/metric_unit_changed/vprev.yaml") + checker = CompatibilityChecker(cur, prev) + errors = checker.check() + self.print_errors(errors) + assert len(errors) == 1 + assert errors[0].signal == "metric" + assert errors[0].name == "metric_one" + assert errors[0].message == "unit changed from 'ms' to 's'" + + def testMetricAttributeAdded(self): + cur = self.parse_semconv("compat/metric_attribute_added/vnext.yaml") + prev = self.parse_semconv("compat/metric_attribute_added/vprev.yaml") + checker = CompatibilityChecker(cur, prev) + errors = checker.check() + self.print_errors(errors) + assert len(errors) == 1 + assert errors[0].signal == "metric" + assert errors[0].name == "metric_one" + assert errors[0].message == "attributes changed from '['first.first_attr']' to '['first.first_attr', 'first.second_attr']'" + + def testTypeChanged(self): + cur = self.parse_semconv("compat/attribute_type_changed/vnext.yaml") + prev = self.parse_semconv("compat/attribute_type_changed/vprev.yaml") + checker = CompatibilityChecker(cur, prev) + errors = checker.check() + self.print_errors(errors) + assert len(errors) == 1 + assert errors[0].signal == "attribute" + assert errors[0].name == "first.second_attr" + assert errors[0].message == "type changed from 'int' to 'string'" + + def testEnumTypeChanged(self): + cur = self.parse_semconv("compat/enum_type_changed/vnext.yaml") + prev = self.parse_semconv("compat/enum_type_changed/vprev.yaml") + checker = CompatibilityChecker(cur, prev) + errors = checker.check() + self.print_errors(errors) + assert len(errors) == 2 + assert errors[0].signal == "attribute" + assert errors[0].name == "first.third_attr" + assert errors[0].message == "enum type changed from 'string' to 'int'" + + assert errors[1].signal == "attribute" + assert errors[1].name == "first.third_attr" + assert errors[1].message == "enum member with value 'one' was removed" + + def testEnumValueRemoved(self): + cur = self.parse_semconv("compat/enum_value_removed/vnext.yaml") + prev = self.parse_semconv("compat/enum_value_removed/vprev.yaml") + checker = CompatibilityChecker(cur, prev) + errors = checker.check() + self.print_errors(errors) + assert len(errors) == 1 + assert errors[0].signal == "attribute" + assert errors[0].name == "first.third_attr" + assert errors[0].message == "enum member with value 'one' was removed" + + def parse_semconv( + self, + input_dir: str, + ) -> SemanticConventionSet: + + semconv = SemanticConventionSet(debug=True) + + dirpath = Path(self.get_file_path(input_dir)) + if (dirpath.is_dir()): + for fname in dirpath.glob("*.yaml"): + print("Parsing", fname) + semconv.parse(fname) + else: + semconv.parse(dirpath) + + assert not semconv.has_error() + semconv.finish() + return semconv + + _TEST_DIR = os.path.dirname(__file__) + + def get_file_path(self, filename): + return os.path.join(self._TEST_DIR, "..", "..", "data", filename) + + def print_errors(self, errors: list[Error]): + if errors: + for error in errors: + print(error) + From 46215ca17c1e2453a5de760b002c2dda067e4a98 Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Tue, 20 Feb 2024 14:24:27 -0800 Subject: [PATCH 02/11] cleanup and docs --- semantic-conventions/README.md | 33 +++- .../src/opentelemetry/semconv/main.py | 67 ++++--- .../semconv/model/semantic_attribute.py | 17 +- .../semconv/model/semantic_convention.py | 27 +-- .../semconv/templating/compat.py | 99 ----------- .../vnext.yaml | 7 +- .../vprev.yaml | 7 +- .../compat/attribute_type_changed/vnext.yaml | 8 +- .../compat/attribute_type_changed/vprev.yaml | 7 +- .../data/compat/removed_attribute/vprev.yaml | 6 +- .../src/tests/data/compat/success/vnext.yaml | 7 + .../src/tests/data/compat/success/vprev.yaml | 6 + .../tests/semconv/templating/test_compat.py | 167 ------------------ 13 files changed, 141 insertions(+), 317 deletions(-) delete mode 100644 semantic-conventions/src/opentelemetry/semconv/templating/compat.py delete mode 100644 semantic-conventions/src/tests/semconv/templating/test_compat.py diff --git a/semantic-conventions/README.md b/semantic-conventions/README.md index b62da331..9826abcb 100644 --- a/semantic-conventions/README.md +++ b/semantic-conventions/README.md @@ -85,8 +85,8 @@ convention that have the tag `network`. `` will print the constraints and attributes of both `http` and `http.server` semantic conventions that have the tag `network`. -`` will print a table describing a single metric -`http.server.active_requests`. +`` will print a table describing a single metric +`http.server.active_requests`. ## Code Generator @@ -116,3 +116,32 @@ comma using the `--parameters [{key=value},]+` or `-D` flag. The image also supports customising [Whitespace Control in Jinja templates](https://jinja.palletsprojects.com/en/3.1.x/templates/#whitespace-control) via the additional flag `--trim-whitespace`. Providing the flag will enable both `lstrip_blocks` and `trim_blocks`. + +## Version compatibility check + +You can check compatibility between the local one specified with `--yaml-root` and sepcific OpenTelemetry semantic convention version using the following command: + +```bash +docker run --rm otel/semconvgen --yaml-root {yaml_folder} compatibility --previous-version {semconv version} +``` + +The `{semconv version}` (e.g. `1.24.0`) is the previously released version of semantic conventions. + +Following checks are performed + +- On all attributes and metrics (experimental and stable): + - attributes and metrics must not be removed. + +- On stable attributes and attribute templates: + - stability must not be changed + - the type of attribute must not be changed + - enum attribute: type of value must not be changed + - enum attribute: members must not be removed (changing `id` field is allowed, as long as `value` does not change) +- On stable metrics: + - stability must not be changed + - instrument and unit must not be changed + - new attributes should not be added. + This check does not take into account opt-in attributes. Adding new attributes to metric is not always breaking, + so it's considered non-critical and it's possible to suppress it with `--ignore-warnings` + + diff --git a/semantic-conventions/src/opentelemetry/semconv/main.py b/semantic-conventions/src/opentelemetry/semconv/main.py index 4a07022b..1e745d93 100644 --- a/semantic-conventions/src/opentelemetry/semconv/main.py +++ b/semantic-conventions/src/opentelemetry/semconv/main.py @@ -29,18 +29,20 @@ SemanticConventionSet, ) from opentelemetry.semconv.templating.code import CodeRenderer -from opentelemetry.semconv.templating.compat import CompatibilityChecker +from opentelemetry.semconv.templating.compatibility import CompatibilityChecker from opentelemetry.semconv.templating.markdown import MarkdownRenderer from opentelemetry.semconv.templating.markdown.options import MarkdownOptions -def parse_semconv(yaml_root:str, exclude:str, debug:bool, parser) -> SemanticConventionSet: +def parse_semconv( + yaml_root: str, exclude: str, debug: bool, parser +) -> SemanticConventionSet: semconv = SemanticConventionSet(debug) files = find_yaml(yaml_root, exclude) for file in sorted(files): if not file.endswith(".yaml") and not file.endswith(".yml"): parser.error(f"{file} is not a yaml file.") - semconv.parse(file) + semconv.parse(file, False) semconv.finish() if semconv.has_error(): sys.exit(1) @@ -82,9 +84,10 @@ def main(): renderer.render(semconv, args.template, args.output, args.pattern) elif args.flavor == "markdown": process_markdown(semconv, args) - elif args.flavor == "compat": + elif args.flavor == "compatibility": check_compatibility(semconv, args, parser) + def process_markdown(semconv, args): options = MarkdownOptions( check_only=args.md_check, @@ -98,29 +101,31 @@ def process_markdown(semconv, args): md_renderer = MarkdownRenderer(args.markdown_root, semconv, options) md_renderer.render_md() + def check_compatibility(semconv, args, parser): prev_semconv_path = download_previous_version(args.previous_version) prev_semconv = parse_semconv(prev_semconv_path, args.exclude, args.debug, parser) - compat_checker = CompatibilityChecker(semconv, prev_semconv) - errors = compat_checker.check() + compatibility_checker = CompatibilityChecker(semconv, prev_semconv) + problems = compatibility_checker.check() - if errors: - print(f"Found {len(errors)} compatibility issues:") - for error in errors: - print(f"\t {error}") + if problems: + print(f"Found {len(problems)} compatibility issues:") + for problem in problems: + print(f"\t {problem}") - sys.exit(1) + if not args.ignore_warnings or ( + args.ignore_warnings and any(problem.critical for problem in problems) + ): + sys.exit(1) -def find_yaml(yaml_root:str, exclude:str) -> List[str]: +def find_yaml(yaml_root: str, exclude: str) -> List[str]: if yaml_root is not None: - exclude = set( - exclude_file_list(yaml_root if yaml_root else "", exclude) + exclude = set(exclude_file_list(yaml_root if yaml_root else "", exclude)) + yaml_files = set(glob.glob(f"{yaml_root}/**/*.yaml", recursive=True)).union( + set(glob.glob(f"{yaml_root}/**/*.yml", recursive=True)) ) - yaml_files = set( - glob.glob(f"{yaml_root}/**/*.yaml", recursive=True) - ).union(set(glob.glob(f"{yaml_root}/**/*.yml", recursive=True))) return yaml_files - exclude @@ -130,7 +135,7 @@ def check_args(arguments, parser): parser.error("Either --yaml-root or YAML_FILE must be present") -def parse_only_filter(only:str, parser) -> List[str]: +def parse_only_filter(only: str, parser) -> List[str]: if not only: return None @@ -183,6 +188,7 @@ def add_code_parser(subparsers): action="store_true", ) + def add_md_parser(subparsers): parser = subparsers.add_parser("markdown") parser.add_argument( @@ -211,7 +217,7 @@ def add_md_parser(subparsers): "--check-compat", help="Don't write the files, but check backward compatibility with specified older version of semantic conventions.", type=str, - required=False + required=False, ) parser.add_argument( "--md-use-badges", @@ -240,14 +246,23 @@ def add_md_parser(subparsers): action="store_false", ) + def add_compat_check_parser(subparsers): - parser = subparsers.add_parser("compat") + parser = subparsers.add_parser("compatibility") parser.add_argument( "--previous-version", help="Check backward compatibility with specified older version of semantic conventions.", type=str, - required=True + required=True, ) + parser.add_argument( + "--ignore-warnings", + help="Ignore non-critical compatibility problems.", + required=False, + default=False, + action="store_true", + ) + def setup_parser(): parser = argparse.ArgumentParser( @@ -295,21 +310,25 @@ def setup_parser(): return parser + def download_previous_version(version: str) -> str: filename = f"v{version}.zip" tmppath = tempfile.mkdtemp() path_to_zip = os.path.join(tmppath, filename) path_to_semconv = os.path.join(tmppath, f"v{version}") - semconv_vprev = f"https://github.com/open-telemetry/semantic-conventions/archive/{filename}" + semconv_vprev = ( + f"https://github.com/open-telemetry/semantic-conventions/archive/{filename}" + ) request = requests.get(semconv_vprev, allow_redirects=True) - open(path_to_zip, 'wb').write(request.content) + open(path_to_zip, "wb").write(request.content) - with zipfile.ZipFile(path_to_zip, 'r') as zip_ref: + with zipfile.ZipFile(path_to_zip, "r") as zip_ref: zip_ref.extractall(path_to_semconv) return os.path.join(path_to_semconv, f"semantic-conventions-{version}", "model") + if __name__ == "__main__": main() diff --git a/semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py b/semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py index f1bdac62..40ce91b2 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py @@ -81,7 +81,7 @@ def is_enum(self): return isinstance(self.attr_type, EnumAttributeType) @staticmethod - def parse(prefix, yaml_attributes) -> "Dict[str, SemanticAttribute]": + def parse(prefix, yaml_attributes, strict_validation=True) -> "Dict[str, SemanticAttribute]": """This method parses the yaml representation for semantic attributes creating the respective SemanticAttribute objects. """ @@ -177,7 +177,7 @@ def parse(prefix, yaml_attributes) -> "Dict[str, SemanticAttribute]": tag = attribute.get("tag", "").strip() stability = SemanticAttribute.parse_stability( - attribute.get("stability"), position_data + attribute.get("stability"), position_data, not strict_validation ) deprecated = SemanticAttribute.parse_deprecated( attribute.get("deprecated"), position_data @@ -280,7 +280,7 @@ def parse_attribute(attribute): return attr_type, str(brief), examples @staticmethod - def parse_stability(stability, position_data): + def parse_stability(stability, position_data, ignore_deprecated=False): if stability is None: return StabilityLevel.EXPERIMENTAL @@ -291,12 +291,15 @@ def parse_stability(stability, position_data): val = stability_value_map.get(stability) if val is not None: return val - msg = f"Value '{stability}' is not allowed as a stability marker, setting to experimental!" - print(msg) - return StabilityLevel.EXPERIMENTAL + # TODO: remove this branch - it's necessary for now to allow back-compat checks against old spec versions + # where we used 'deprecated' as stability level + if ignore_deprecated and stability == "deprecated": + print(f'WARNING: Using "deprecated" as stability level is no longer supported. Use "experimental" instead.') + return StabilityLevel.EXPERIMENTAL - #raise ValidationError.from_yaml_pos(position_data["stability"], msg) + msg = f"Value '{stability}' is not allowed as a stability marker" + raise ValidationError.from_yaml_pos(position_data["stability"], msg) @staticmethod def parse_deprecated(deprecated, position_data): diff --git a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py index fa6977a9..65ae32ab 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py @@ -59,15 +59,15 @@ def parse_semantic_convention_type(type_value): return CONVENTION_CLS_BY_GROUP_TYPE.get(type_value) -def parse_semantic_convention_groups(yaml_file): +def parse_semantic_convention_groups(yaml_file, strict_validation=True): yaml = YAML().load(yaml_file) models = [] for group in yaml["groups"]: - models.append(SemanticConvention(group)) + models.append(SemanticConvention(group, strict_validation)) return models -def SemanticConvention(group): +def SemanticConvention(group, strict_validation=True): type_value = group.get("type") if type_value is None: line = group.lc.data["id"][0] + 1 @@ -85,7 +85,7 @@ def SemanticConvention(group): # First, validate that the correct fields are available in the yaml convention_type.validate_keys(group) - model = convention_type(group) + model = convention_type(group, strict_validation) # Also, validate that the value of the fields is acceptable model.validate_values() return model @@ -134,7 +134,7 @@ def _get_attributes(self, templates: Optional[bool]): key=lambda attr: attr.fqn, ) - def __init__(self, group): + def __init__(self, group, strict_validation = True): super().__init__(group) self.semconv_id = self.id @@ -142,7 +142,7 @@ def __init__(self, group): self.prefix = group.get("prefix", "").strip() position_data = group.lc.data self.stability = SemanticAttribute.parse_stability( - group.get("stability"), position_data + group.get("stability"), position_data, not strict_validation ) self.deprecated = SemanticAttribute.parse_deprecated( group.get("deprecated"), position_data @@ -151,7 +151,8 @@ def __init__(self, group): self.events = group.get("events", ()) self.constraints = parse_constraints(group.get("constraints", ())) self.attrs_by_name = SemanticAttribute.parse( - self.prefix, group.get("attributes") + self.prefix, group.get("attributes"), + strict_validation ) def contains_attribute(self, attr: "SemanticAttribute"): @@ -198,7 +199,7 @@ class SpanSemanticConvention(BaseSemanticConvention): "span_kind", ) - def __init__(self, group): + def __init__(self, group, strict_validation=True): super().__init__(group) self.span_kind = SpanKind.parse(group.get("span_kind")) if self.span_kind is None: @@ -212,7 +213,7 @@ class EventSemanticConvention(BaseSemanticConvention): allowed_keys = BaseSemanticConvention.allowed_keys + ("name",) - def __init__(self, group): + def __init__(self, group, strict_validation=True): super().__init__(group) self.name = group.get("name", self.prefix) if not self.name: @@ -231,7 +232,7 @@ class UnitSemanticConvention(BaseSemanticConvention): "members", ) - def __init__(self, group): + def __init__(self, group, strict_validation=True): super().__init__(group) self.members = UnitMember.parse(group.get("members")) @@ -260,7 +261,7 @@ class MetricSemanticConvention(MetricGroupSemanticConvention): canonical_instrument_name_by_yaml_name.keys() ) - def __init__(self, group): + def __init__(self, group, strict_validation=True): super().__init__(group) self.metric_name = group.get("metric_name") self.unit = group.get("unit") @@ -292,10 +293,10 @@ class SemanticConventionSet: models: typing.Dict[str, BaseSemanticConvention] = field(default_factory=dict) errors: bool = False - def parse(self, file): + def parse(self, file, strict_validation=True): with open(file, "r", encoding="utf-8") as yaml_file: try: - semconv_models = parse_semantic_convention_groups(yaml_file) + semconv_models = parse_semantic_convention_groups(yaml_file, strict_validation) for model in semconv_models: if model.semconv_id in self.models: self.errors = True diff --git a/semantic-conventions/src/opentelemetry/semconv/templating/compat.py b/semantic-conventions/src/opentelemetry/semconv/templating/compat.py deleted file mode 100644 index 48569f26..00000000 --- a/semantic-conventions/src/opentelemetry/semconv/templating/compat.py +++ /dev/null @@ -1,99 +0,0 @@ -from opentelemetry.semconv.model.semantic_attribute import ( - EnumMember, - RequirementLevel, - SemanticAttribute, - StabilityLevel, -) -from opentelemetry.semconv.model.semantic_convention import ( - MetricSemanticConvention, - SemanticConventionSet, -) - -class Error: - signal: str - name: str - message: str - - def __init__(self, signal: str, name: str, message: str): - self.signal = signal - self.name = name - self.message = message - - def __str__(self): - return f"\t {self.signal} '{self.name}': {self.message}" - -class CompatibilityChecker: - previous_version: str - - def __init__(self, current_semconv: SemanticConventionSet, - previous_semconv: SemanticConventionSet): - self.current_semconv = current_semconv - self.previous_semconv = previous_semconv - - # TODO: attribute templates - # stability - # docs - def check( - self - ) -> list[Error]: - errors = [] - for semconv in self.previous_semconv.models.values(): - for prev_attr in semconv.attributes: - if prev_attr.is_local and prev_attr.attr_id is not None and prev_attr.ref is None: - self._check_attribute(prev_attr, errors) - if isinstance(semconv, MetricSemanticConvention): - self._check_metric(semconv, errors) - return errors - - - def _check_attribute(self, prev: SemanticAttribute, errors: list[str]): - cur = self.current_semconv._lookup_attribute(prev.fqn) - if (cur is None): - errors.append(Error("attribute", prev.fqn, "was removed")) - return - - if prev.stability == StabilityLevel.STABLE: - if cur.stability != prev.stability: - errors.append(Error("attribute", prev.fqn, f"stability changed from '{prev.stability}' to '{cur.stability}'")) - - if prev.is_enum: - if not cur.is_enum: - errors.append(Error("attribute", prev.fqn, f"type changed from '{prev.attr_type}' to '{cur.attr_type}'")) - else: - if cur.attr_type.enum_type != prev.attr_type.enum_type: - errors.append(Error("attribute", prev.fqn, f"enum type changed from '{prev.attr_type.enum_type}' to '{cur.attr_type.enum_type}'")) - for member in prev.attr_type.members: - self._check_member(prev.fqn, member, cur.attr_type.members, errors) - elif cur.attr_type != prev.attr_type: - errors.append(Error("attribute", prev.fqn, f"type changed from '{prev.attr_type}' to '{cur.attr_type}'")) - - - def _check_member(self, fqn: str, prev: EnumMember, members: list[EnumMember], errors: list[str]): - for member in members: - if prev.value == member.value: - return - - errors.append(Error("attribute", fqn, f"enum member with value '{prev.value}' was removed")) - - - def _check_metric(self, prev: MetricSemanticConvention, errors: list[str]): - for cur in self.current_semconv.models.values(): - if isinstance(cur, MetricSemanticConvention) and cur.metric_name == prev.metric_name: - if prev.stability == StabilityLevel.STABLE: - if cur.stability != prev.stability: - errors.append(Error("metric", prev.metric_name, f"stability changed from '{prev.stability}' to '{cur.stability}'")) - if cur.unit != prev.unit: - errors.append(Error("metric", prev.metric_name, f"unit changed from '{prev.unit}' to '{cur.unit}'")) - if cur.instrument != prev.instrument: - errors.append(Error("metric", prev.metric_name, f"instrument changed from '{prev.instrument}' to '{cur.instrument}'")) - self._check_metric_attributes(prev, cur, errors) - return - - errors.append(Error("metric", prev.metric_name, "was removed")) - - def _check_metric_attributes(self, prev: MetricSemanticConvention, cur: MetricSemanticConvention, errors: list[str]): - if prev.stability == StabilityLevel.STABLE: - prev_default_attributes = [attr.fqn for attr in prev.attributes if attr.requirement_level != RequirementLevel.OPT_IN] - cur_default_attributes = [attr.fqn for attr in cur.attributes if attr.requirement_level != RequirementLevel.OPT_IN] - if prev_default_attributes != cur_default_attributes: - errors.append(Error("metric", prev.metric_name, f"attributes changed from '{prev_default_attributes}' to '{cur_default_attributes}'")) diff --git a/semantic-conventions/src/tests/data/compat/attribute_stable_to_experimental/vnext.yaml b/semantic-conventions/src/tests/data/compat/attribute_stable_to_experimental/vnext.yaml index 78882a26..a26c943f 100644 --- a/semantic-conventions/src/tests/data/compat/attribute_stable_to_experimental/vnext.yaml +++ b/semantic-conventions/src/tests/data/compat/attribute_stable_to_experimental/vnext.yaml @@ -16,4 +16,9 @@ groups: brief: "Second attribute." note: "Second attribute note." stability: experimental - examples: [2] \ No newline at end of file + examples: [2] + - id: fifth_attr_template + type: template[string[]] + brief: "Request headers." + note: "Request headers note." + examples: '`first.fifth_attr.bar=["foo"]`' diff --git a/semantic-conventions/src/tests/data/compat/attribute_stable_to_experimental/vprev.yaml b/semantic-conventions/src/tests/data/compat/attribute_stable_to_experimental/vprev.yaml index 9f3f8c73..74f0d577 100644 --- a/semantic-conventions/src/tests/data/compat/attribute_stable_to_experimental/vprev.yaml +++ b/semantic-conventions/src/tests/data/compat/attribute_stable_to_experimental/vprev.yaml @@ -15,4 +15,9 @@ groups: brief: "second attribute" note: "second attribute note" stability: stable - examples: 2 \ No newline at end of file + examples: 2 + - id: fifth_attr_template + type: template[string[]] + brief: "request headers" + examples: '`first.fifth_attr.foo=["bar"]`' + stability: stable \ No newline at end of file diff --git a/semantic-conventions/src/tests/data/compat/attribute_type_changed/vnext.yaml b/semantic-conventions/src/tests/data/compat/attribute_type_changed/vnext.yaml index b47d95bf..bbfd801a 100644 --- a/semantic-conventions/src/tests/data/compat/attribute_type_changed/vnext.yaml +++ b/semantic-conventions/src/tests/data/compat/attribute_type_changed/vnext.yaml @@ -16,4 +16,10 @@ groups: brief: "Second attribute." note: "Second attribute note." stability: stable - examples: ["two"] \ No newline at end of file + examples: ["two"] + - id: fifth_attr_template + type: template[int[]] + brief: "Request headers." + note: "Request headers note." + examples: '`first.fifth_attr.bar=[42]`' + stability: stable \ No newline at end of file diff --git a/semantic-conventions/src/tests/data/compat/attribute_type_changed/vprev.yaml b/semantic-conventions/src/tests/data/compat/attribute_type_changed/vprev.yaml index 9f3f8c73..74f0d577 100644 --- a/semantic-conventions/src/tests/data/compat/attribute_type_changed/vprev.yaml +++ b/semantic-conventions/src/tests/data/compat/attribute_type_changed/vprev.yaml @@ -15,4 +15,9 @@ groups: brief: "second attribute" note: "second attribute note" stability: stable - examples: 2 \ No newline at end of file + examples: 2 + - id: fifth_attr_template + type: template[string[]] + brief: "request headers" + examples: '`first.fifth_attr.foo=["bar"]`' + stability: stable \ No newline at end of file diff --git a/semantic-conventions/src/tests/data/compat/removed_attribute/vprev.yaml b/semantic-conventions/src/tests/data/compat/removed_attribute/vprev.yaml index 9f3f8c73..5d674714 100644 --- a/semantic-conventions/src/tests/data/compat/removed_attribute/vprev.yaml +++ b/semantic-conventions/src/tests/data/compat/removed_attribute/vprev.yaml @@ -15,4 +15,8 @@ groups: brief: "second attribute" note: "second attribute note" stability: stable - examples: 2 \ No newline at end of file + examples: 2 + - id: fifth_attr_template + type: template[string[]] + brief: "request headers" + examples: '`first.fifth_attr.foo=["bar"]`' diff --git a/semantic-conventions/src/tests/data/compat/success/vnext.yaml b/semantic-conventions/src/tests/data/compat/success/vnext.yaml index 9ee6f5fe..e3d89d07 100644 --- a/semantic-conventions/src/tests/data/compat/success/vnext.yaml +++ b/semantic-conventions/src/tests/data/compat/success/vnext.yaml @@ -36,6 +36,13 @@ groups: note: "forth attribute note" examples: [True] stability: stable + - id: fifth_attr_template + type: template[string[]] + brief: "Request headers." + note: "Request headers note." + examples: '`first.fifth_attr.bar=["foo"]`' + stability: stable + - id: metric_one type: metric metric_name: "metric_one" diff --git a/semantic-conventions/src/tests/data/compat/success/vprev.yaml b/semantic-conventions/src/tests/data/compat/success/vprev.yaml index 023eb387..8fc72dc2 100644 --- a/semantic-conventions/src/tests/data/compat/success/vprev.yaml +++ b/semantic-conventions/src/tests/data/compat/success/vprev.yaml @@ -29,6 +29,12 @@ groups: note: "third attribute note" examples: ["one"] stability: stable + - id: fifth_attr_template + type: template[string[]] + brief: "request headers" + examples: '`first.fifth_attr.foo=["bar"]`' + stability: stable + - id: metric_one type: metric metric_name: "metric_one" diff --git a/semantic-conventions/src/tests/semconv/templating/test_compat.py b/semantic-conventions/src/tests/semconv/templating/test_compat.py deleted file mode 100644 index 3a2c8d37..00000000 --- a/semantic-conventions/src/tests/semconv/templating/test_compat.py +++ /dev/null @@ -1,167 +0,0 @@ -# Copyright The OpenTelemetry Authors -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -import io -import os -import unittest -from pathlib import Path - -from opentelemetry.semconv.model.semantic_convention import SemanticConventionSet -from opentelemetry.semconv.templating.compat import CompatibilityChecker, Error - -class TestCompatibility(unittest.TestCase): - def testSuccess(self): - cur = self.parse_semconv("compat/success/vnext.yaml") - prev = self.parse_semconv("compat/success/vprev.yaml") - checker = CompatibilityChecker(cur, prev) - errors = checker.check() - self.print_errors(errors) - assert len(errors) == 0 - - def testRemovedAttribute(self): - cur = self.parse_semconv("compat/removed_attribute/vnext.yaml") - prev = self.parse_semconv("compat/removed_attribute/vprev.yaml") - checker = CompatibilityChecker(cur, prev) - errors = checker.check() - self.print_errors(errors) - assert len(errors) == 2 - assert errors[0].signal == "attribute" - assert errors[0].name == "first.first_attr" - assert errors[0].message == "was removed" - - assert errors[1].signal == "attribute" - assert errors[1].name == "first.second_attr" - assert errors[1].message == "was removed" - - def testAttributeStableToExperimental(self): - cur = self.parse_semconv("compat/attribute_stable_to_experimental/vnext.yaml") - prev = self.parse_semconv("compat/attribute_stable_to_experimental/vprev.yaml") - checker = CompatibilityChecker(cur, prev) - errors = checker.check() - self.print_errors(errors) - assert len(errors) == 1 - assert errors[0].signal == "attribute" - assert errors[0].name == "first.second_attr" - assert errors[0].message == "stability changed from 'StabilityLevel.STABLE' to 'StabilityLevel.EXPERIMENTAL'" - - def testMetricStableToExperimental(self): - cur = self.parse_semconv("compat/metric_stable_to_experimental/vnext.yaml") - prev = self.parse_semconv("compat/metric_stable_to_experimental/vprev.yaml") - checker = CompatibilityChecker(cur, prev) - errors = checker.check() - self.print_errors(errors) - assert len(errors) == 1 - assert errors[0].signal == "metric" - assert errors[0].name == "metric_one" - assert errors[0].message == "stability changed from 'StabilityLevel.STABLE' to 'StabilityLevel.EXPERIMENTAL'" - - def testMetricInstrumentChanged(self): - cur = self.parse_semconv("compat/metric_instrument_changed/vnext.yaml") - prev = self.parse_semconv("compat/metric_instrument_changed/vprev.yaml") - checker = CompatibilityChecker(cur, prev) - errors = checker.check() - self.print_errors(errors) - assert len(errors) == 1 - assert errors[0].signal == "metric" - assert errors[0].name == "metric_one" - assert errors[0].message == "instrument changed from 'counter' to 'histogram'" - - def testMetricUnitChanged(self): - cur = self.parse_semconv("compat/metric_unit_changed/vnext.yaml") - prev = self.parse_semconv("compat/metric_unit_changed/vprev.yaml") - checker = CompatibilityChecker(cur, prev) - errors = checker.check() - self.print_errors(errors) - assert len(errors) == 1 - assert errors[0].signal == "metric" - assert errors[0].name == "metric_one" - assert errors[0].message == "unit changed from 'ms' to 's'" - - def testMetricAttributeAdded(self): - cur = self.parse_semconv("compat/metric_attribute_added/vnext.yaml") - prev = self.parse_semconv("compat/metric_attribute_added/vprev.yaml") - checker = CompatibilityChecker(cur, prev) - errors = checker.check() - self.print_errors(errors) - assert len(errors) == 1 - assert errors[0].signal == "metric" - assert errors[0].name == "metric_one" - assert errors[0].message == "attributes changed from '['first.first_attr']' to '['first.first_attr', 'first.second_attr']'" - - def testTypeChanged(self): - cur = self.parse_semconv("compat/attribute_type_changed/vnext.yaml") - prev = self.parse_semconv("compat/attribute_type_changed/vprev.yaml") - checker = CompatibilityChecker(cur, prev) - errors = checker.check() - self.print_errors(errors) - assert len(errors) == 1 - assert errors[0].signal == "attribute" - assert errors[0].name == "first.second_attr" - assert errors[0].message == "type changed from 'int' to 'string'" - - def testEnumTypeChanged(self): - cur = self.parse_semconv("compat/enum_type_changed/vnext.yaml") - prev = self.parse_semconv("compat/enum_type_changed/vprev.yaml") - checker = CompatibilityChecker(cur, prev) - errors = checker.check() - self.print_errors(errors) - assert len(errors) == 2 - assert errors[0].signal == "attribute" - assert errors[0].name == "first.third_attr" - assert errors[0].message == "enum type changed from 'string' to 'int'" - - assert errors[1].signal == "attribute" - assert errors[1].name == "first.third_attr" - assert errors[1].message == "enum member with value 'one' was removed" - - def testEnumValueRemoved(self): - cur = self.parse_semconv("compat/enum_value_removed/vnext.yaml") - prev = self.parse_semconv("compat/enum_value_removed/vprev.yaml") - checker = CompatibilityChecker(cur, prev) - errors = checker.check() - self.print_errors(errors) - assert len(errors) == 1 - assert errors[0].signal == "attribute" - assert errors[0].name == "first.third_attr" - assert errors[0].message == "enum member with value 'one' was removed" - - def parse_semconv( - self, - input_dir: str, - ) -> SemanticConventionSet: - - semconv = SemanticConventionSet(debug=True) - - dirpath = Path(self.get_file_path(input_dir)) - if (dirpath.is_dir()): - for fname in dirpath.glob("*.yaml"): - print("Parsing", fname) - semconv.parse(fname) - else: - semconv.parse(dirpath) - - assert not semconv.has_error() - semconv.finish() - return semconv - - _TEST_DIR = os.path.dirname(__file__) - - def get_file_path(self, filename): - return os.path.join(self._TEST_DIR, "..", "..", "data", filename) - - def print_errors(self, errors: list[Error]): - if errors: - for error in errors: - print(error) - From 29276b3214193a3a4ef8358df255a02d098ee2c8 Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Tue, 20 Feb 2024 14:28:55 -0800 Subject: [PATCH 03/11] lint --- .../src/opentelemetry/semconv/main.py | 2 +- .../semconv/model/semantic_attribute.py | 8 +- .../semconv/model/semantic_convention.py | 9 +- .../semconv/templating/compatibility.py | 185 +++++++++++++++++ .../semconv/templating/test_compatibility.py | 193 ++++++++++++++++++ 5 files changed, 390 insertions(+), 7 deletions(-) create mode 100644 semantic-conventions/src/opentelemetry/semconv/templating/compatibility.py create mode 100644 semantic-conventions/src/tests/semconv/templating/test_compatibility.py diff --git a/semantic-conventions/src/opentelemetry/semconv/main.py b/semantic-conventions/src/opentelemetry/semconv/main.py index 1e745d93..5da43019 100644 --- a/semantic-conventions/src/opentelemetry/semconv/main.py +++ b/semantic-conventions/src/opentelemetry/semconv/main.py @@ -215,7 +215,7 @@ def add_md_parser(subparsers): ) parser.add_argument( "--check-compat", - help="Don't write the files, but check backward compatibility with specified older version of semantic conventions.", + help="Check backward compatibility with previous version of semantic conventions.", type=str, required=False, ) diff --git a/semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py b/semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py index 40ce91b2..dffaeed0 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py @@ -81,7 +81,9 @@ def is_enum(self): return isinstance(self.attr_type, EnumAttributeType) @staticmethod - def parse(prefix, yaml_attributes, strict_validation=True) -> "Dict[str, SemanticAttribute]": + def parse( + prefix, yaml_attributes, strict_validation=True + ) -> "Dict[str, SemanticAttribute]": """This method parses the yaml representation for semantic attributes creating the respective SemanticAttribute objects. """ @@ -295,7 +297,9 @@ def parse_stability(stability, position_data, ignore_deprecated=False): # TODO: remove this branch - it's necessary for now to allow back-compat checks against old spec versions # where we used 'deprecated' as stability level if ignore_deprecated and stability == "deprecated": - print(f'WARNING: Using "deprecated" as stability level is no longer supported. Use "experimental" instead.') + print( + 'WARNING: Using "deprecated" as stability level is no longer supported. Use "experimental" instead.' + ) return StabilityLevel.EXPERIMENTAL msg = f"Value '{stability}' is not allowed as a stability marker" diff --git a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py index 65ae32ab..7a82e255 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py @@ -134,7 +134,7 @@ def _get_attributes(self, templates: Optional[bool]): key=lambda attr: attr.fqn, ) - def __init__(self, group, strict_validation = True): + def __init__(self, group, strict_validation=True): super().__init__(group) self.semconv_id = self.id @@ -151,8 +151,7 @@ def __init__(self, group, strict_validation = True): self.events = group.get("events", ()) self.constraints = parse_constraints(group.get("constraints", ())) self.attrs_by_name = SemanticAttribute.parse( - self.prefix, group.get("attributes"), - strict_validation + self.prefix, group.get("attributes"), strict_validation ) def contains_attribute(self, attr: "SemanticAttribute"): @@ -296,7 +295,9 @@ class SemanticConventionSet: def parse(self, file, strict_validation=True): with open(file, "r", encoding="utf-8") as yaml_file: try: - semconv_models = parse_semantic_convention_groups(yaml_file, strict_validation) + semconv_models = parse_semantic_convention_groups( + yaml_file, strict_validation + ) for model in semconv_models: if model.semconv_id in self.models: self.errors = True diff --git a/semantic-conventions/src/opentelemetry/semconv/templating/compatibility.py b/semantic-conventions/src/opentelemetry/semconv/templating/compatibility.py new file mode 100644 index 00000000..a2ab6a61 --- /dev/null +++ b/semantic-conventions/src/opentelemetry/semconv/templating/compatibility.py @@ -0,0 +1,185 @@ +from opentelemetry.semconv.model.semantic_attribute import ( + EnumMember, + RequirementLevel, + SemanticAttribute, + StabilityLevel, +) +from opentelemetry.semconv.model.semantic_convention import ( + MetricSemanticConvention, + SemanticConventionSet, +) + + +class Problem: + signal: str + name: str + message: str + critical: bool + + def __init__(self, signal: str, name: str, message: str, critical: bool = True): + self.signal = signal + self.name = name + self.message = message + + def __str__(self): + return f"\t {self.signal} '{self.name}': {self.message}" + + def __eq__(self, other): + if isinstance(other, self.__class__): + return self.__dict__ == other.__dict__ + else: + return False + + def __ne__(self, other): + return not self.__eq__(other) + + +class CompatibilityChecker: + previous_version: str + + def __init__( + self, + current_semconv: SemanticConventionSet, + previous_semconv: SemanticConventionSet, + ): + self.current_semconv = current_semconv + self.previous_semconv = previous_semconv + + def check(self) -> list[Problem]: + problems = [] + for semconv in self.previous_semconv.models.values(): + for prev_attr in semconv.attributes_and_templates: + if ( + prev_attr.is_local + and prev_attr.attr_id is not None + and prev_attr.ref is None + ): + self._check_attribute(prev_attr, problems) + if isinstance(semconv, MetricSemanticConvention): + self._check_metric(semconv, problems) + return problems + + def _check_attribute(self, prev: SemanticAttribute, problems: list[str]): + cur = self.current_semconv._lookup_attribute(prev.fqn) + if cur is None: + problems.append(Problem("attribute", prev.fqn, "was removed")) + return + + if prev.stability == StabilityLevel.STABLE: + if cur.stability != prev.stability: + problems.append( + Problem( + "attribute", + prev.fqn, + f"stability changed from '{prev.stability}' to '{cur.stability}'", + ) + ) + + if prev.is_enum: + if not cur.is_enum: + problems.append( + Problem( + "attribute", + prev.fqn, + f"type changed from '{prev.attr_type}' to '{cur.attr_type}'", + ) + ) + else: + if cur.attr_type.enum_type != prev.attr_type.enum_type: + problems.append( + Problem( + "attribute", + prev.fqn, + f"enum type changed from '{prev.attr_type.enum_type}' to '{cur.attr_type.enum_type}'", + ) + ) + for member in prev.attr_type.members: + self._check_member( + prev.fqn, member, cur.attr_type.members, problems + ) + elif cur.attr_type != prev.attr_type: + problems.append( + Problem( + "attribute", + prev.fqn, + f"type changed from '{prev.attr_type}' to '{cur.attr_type}'", + ) + ) + + def _check_member( + self, fqn: str, prev: EnumMember, members: list[EnumMember], problems: list[str] + ): + for member in members: + if prev.value == member.value: + return + + problems.append( + Problem( + "attribute", fqn, f"enum member with value '{prev.value}' was removed" + ) + ) + + def _check_metric(self, prev: MetricSemanticConvention, problems: list[str]): + for cur in self.current_semconv.models.values(): + if ( + isinstance(cur, MetricSemanticConvention) + and cur.metric_name == prev.metric_name + ): + if prev.stability == StabilityLevel.STABLE: + if cur.stability != prev.stability: + problems.append( + Problem( + "metric", + prev.metric_name, + f"stability changed from '{prev.stability}' to '{cur.stability}'", + ) + ) + if cur.unit != prev.unit: + problems.append( + Problem( + "metric", + prev.metric_name, + f"unit changed from '{prev.unit}' to '{cur.unit}'", + ) + ) + if cur.instrument != prev.instrument: + problems.append( + Problem( + "metric", + prev.metric_name, + f"instrument changed from '{prev.instrument}' to '{cur.instrument}'", + ) + ) + self._check_metric_attributes(prev, cur, problems) + return + + problems.append(Problem("metric", prev.metric_name, "was removed")) + + def _check_metric_attributes( + self, + prev: MetricSemanticConvention, + cur: MetricSemanticConvention, + problems: list[str], + ): + if prev.stability == StabilityLevel.STABLE: + prev_default_attributes = [ + attr.fqn + for attr in prev.attributes + if attr.requirement_level != RequirementLevel.OPT_IN + ] + cur_default_attributes = [ + attr.fqn + for attr in cur.attributes + if attr.requirement_level != RequirementLevel.OPT_IN + ] + if prev_default_attributes != cur_default_attributes: + # Adding attributes to metrics could be fine if it does not increase number of time series, + # so we do not consider it as critical problem. + problems.append( + Problem( + "metric", + prev.metric_name, + f"attributes changed from '{prev_default_attributes}' to '{cur_default_attributes}'", + False, + ) + ) diff --git a/semantic-conventions/src/tests/semconv/templating/test_compatibility.py b/semantic-conventions/src/tests/semconv/templating/test_compatibility.py new file mode 100644 index 00000000..11cd72d0 --- /dev/null +++ b/semantic-conventions/src/tests/semconv/templating/test_compatibility.py @@ -0,0 +1,193 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os +import unittest +from pathlib import Path + +from opentelemetry.semconv.model.semantic_convention import SemanticConventionSet +from opentelemetry.semconv.templating.compatibility import CompatibilityChecker, Problem + + +class TestCompatibility(unittest.TestCase): + def testSuccess(self): + cur = self.parse_semconv("compat/success/vnext.yaml") + prev = self.parse_semconv("compat/success/vprev.yaml") + checker = CompatibilityChecker(cur, prev) + problems = checker.check() + self.assert_errors([], problems) + + def testRemovedAttribute(self): + cur = self.parse_semconv("compat/removed_attribute/vnext.yaml") + prev = self.parse_semconv("compat/removed_attribute/vprev.yaml") + checker = CompatibilityChecker(cur, prev) + problems = checker.check() + + expected_errors = [ + Problem("attribute", "first.first_attr", "was removed"), + Problem("attribute", "first.second_attr", "was removed"), + Problem("attribute", "first.fifth_attr_template", "was removed"), + ] + self.assert_errors(expected_errors, problems) + + def testAttributeStableToExperimental(self): + cur = self.parse_semconv("compat/attribute_stable_to_experimental/vnext.yaml") + prev = self.parse_semconv("compat/attribute_stable_to_experimental/vprev.yaml") + checker = CompatibilityChecker(cur, prev) + problems = checker.check() + expected_errors = [ + Problem( + "attribute", + "first.second_attr", + "stability changed from 'StabilityLevel.STABLE' to 'StabilityLevel.EXPERIMENTAL'", + ), + Problem( + "attribute", + "first.fifth_attr_template", + "stability changed from 'StabilityLevel.STABLE' to 'StabilityLevel.EXPERIMENTAL'", + ), + ] + self.assert_errors(expected_errors, problems) + + def testMetricStableToExperimental(self): + cur = self.parse_semconv("compat/metric_stable_to_experimental/vnext.yaml") + prev = self.parse_semconv("compat/metric_stable_to_experimental/vprev.yaml") + checker = CompatibilityChecker(cur, prev) + problems = checker.check() + expected_errors = [ + Problem( + "metric", + "metric_one", + "stability changed from 'StabilityLevel.STABLE' to 'StabilityLevel.EXPERIMENTAL'", + ) + ] + self.assert_errors(expected_errors, problems) + + def testMetricInstrumentChanged(self): + cur = self.parse_semconv("compat/metric_instrument_changed/vnext.yaml") + prev = self.parse_semconv("compat/metric_instrument_changed/vprev.yaml") + checker = CompatibilityChecker(cur, prev) + problems = checker.check() + expected_errors = [ + Problem( + "metric", + "metric_one", + "instrument changed from 'counter' to 'histogram'", + ) + ] + self.assert_errors(expected_errors, problems) + + def testMetricUnitChanged(self): + cur = self.parse_semconv("compat/metric_unit_changed/vnext.yaml") + prev = self.parse_semconv("compat/metric_unit_changed/vprev.yaml") + checker = CompatibilityChecker(cur, prev) + problems = checker.check() + expected_errors = [ + Problem("metric", "metric_one", "unit changed from 'ms' to 's'") + ] + self.assert_errors(expected_errors, problems) + + def testMetricAttributeAdded(self): + cur = self.parse_semconv("compat/metric_attribute_added/vnext.yaml") + prev = self.parse_semconv("compat/metric_attribute_added/vprev.yaml") + checker = CompatibilityChecker(cur, prev) + problems = checker.check() + expected_errors = [ + Problem( + "metric", + "metric_one", + "attributes changed from '['first.first_attr']' to '['first.first_attr', 'first.second_attr']'", + ) + ] + self.assert_errors(expected_errors, problems) + + def testTypeChanged(self): + cur = self.parse_semconv("compat/attribute_type_changed/vnext.yaml") + prev = self.parse_semconv("compat/attribute_type_changed/vprev.yaml") + checker = CompatibilityChecker(cur, prev) + problems = checker.check() + expected_errors = [ + Problem( + "attribute", "first.second_attr", "type changed from 'int' to 'string'" + ), + Problem( + "attribute", + "first.fifth_attr_template", + "type changed from 'template[string[]]' to 'template[int[]]'", + ), + ] + self.assert_errors(expected_errors, problems) + + def testEnumTypeChanged(self): + cur = self.parse_semconv("compat/enum_type_changed/vnext.yaml") + prev = self.parse_semconv("compat/enum_type_changed/vprev.yaml") + checker = CompatibilityChecker(cur, prev) + problems = checker.check() + expected_errors = [ + Problem( + "attribute", + "first.third_attr", + "enum type changed from 'string' to 'int'", + ), + Problem( + "attribute", + "first.third_attr", + "enum member with value 'one' was removed", + ), + ] + self.assert_errors(expected_errors, problems) + + def testEnumValueRemoved(self): + cur = self.parse_semconv("compat/enum_value_removed/vnext.yaml") + prev = self.parse_semconv("compat/enum_value_removed/vprev.yaml") + checker = CompatibilityChecker(cur, prev) + problems = checker.check() + expected_errors = [ + Problem( + "attribute", + "first.third_attr", + "enum member with value 'one' was removed", + ) + ] + self.assert_errors(expected_errors, problems) + + def parse_semconv( + self, + input_dir: str, + ) -> SemanticConventionSet: + + semconv = SemanticConventionSet(debug=True) + + dirpath = Path(self.get_file_path(input_dir)) + if dirpath.is_dir(): + for fname in dirpath.glob("*.yaml"): + print("Parsing", fname) + semconv.parse(fname) + else: + semconv.parse(dirpath) + + assert not semconv.has_error() + semconv.finish() + return semconv + + _TEST_DIR = os.path.dirname(__file__) + + def get_file_path(self, filename): + return os.path.join(self._TEST_DIR, "..", "..", "data", filename) + + def assert_errors(self, expected: list[Problem], actual: list[Problem]): + assert len(expected) == len(actual) + for a in actual: + print(a) + assert a in expected From 4375b6a958662d725ee2f1525de802ed2ab5f74d Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Tue, 20 Feb 2024 14:36:54 -0800 Subject: [PATCH 04/11] lint --- semantic-conventions/dev-requirements.txt | 2 +- .../src/opentelemetry/semconv/model/semantic_attribute.py | 6 +++--- .../src/opentelemetry/semconv/model/semantic_convention.py | 2 +- .../opentelemetry/semconv/templating/markdown/options.py | 1 - 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/semantic-conventions/dev-requirements.txt b/semantic-conventions/dev-requirements.txt index 743b662f..d3eb9ab1 100644 --- a/semantic-conventions/dev-requirements.txt +++ b/semantic-conventions/dev-requirements.txt @@ -4,4 +4,4 @@ pytest==8.0.1 flake8==7.0.0 pylint==3.0.3 isort==5.13.2 -requests=2.31.0 \ No newline at end of file +requests==2.31.0 \ No newline at end of file diff --git a/semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py b/semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py index dffaeed0..edde7808 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py @@ -179,7 +179,7 @@ def parse( tag = attribute.get("tag", "").strip() stability = SemanticAttribute.parse_stability( - attribute.get("stability"), position_data, not strict_validation + attribute.get("stability"), position_data, strict_validation ) deprecated = SemanticAttribute.parse_deprecated( attribute.get("deprecated"), position_data @@ -282,7 +282,7 @@ def parse_attribute(attribute): return attr_type, str(brief), examples @staticmethod - def parse_stability(stability, position_data, ignore_deprecated=False): + def parse_stability(stability, position_data, strict_validation=True): if stability is None: return StabilityLevel.EXPERIMENTAL @@ -296,7 +296,7 @@ def parse_stability(stability, position_data, ignore_deprecated=False): # TODO: remove this branch - it's necessary for now to allow back-compat checks against old spec versions # where we used 'deprecated' as stability level - if ignore_deprecated and stability == "deprecated": + if not strict_validation and stability == "deprecated": print( 'WARNING: Using "deprecated" as stability level is no longer supported. Use "experimental" instead.' ) diff --git a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py index 7a82e255..af51a6e0 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py @@ -142,7 +142,7 @@ def __init__(self, group, strict_validation=True): self.prefix = group.get("prefix", "").strip() position_data = group.lc.data self.stability = SemanticAttribute.parse_stability( - group.get("stability"), position_data, not strict_validation + group.get("stability"), position_data, strict_validation ) self.deprecated = SemanticAttribute.parse_deprecated( group.get("deprecated"), position_data diff --git a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/options.py b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/options.py index 528f4f4d..419ef8f1 100644 --- a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/options.py +++ b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/options.py @@ -19,7 +19,6 @@ @dataclass() class MarkdownOptions: check_only: bool = False - check_compatibility_with: str = None enable_stable: bool = False enable_experimental: bool = False enable_deprecated: bool = True From ba8fb09bfa52e17deed15fd8d8d9dadf4583ab3c Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Tue, 20 Feb 2024 14:39:16 -0800 Subject: [PATCH 05/11] changelog --- semantic-conventions/CHANGELOG.md | 2 ++ semantic-conventions/src/opentelemetry/semconv/main.py | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/semantic-conventions/CHANGELOG.md b/semantic-conventions/CHANGELOG.md index 8b74cb31..cbd1ff8f 100644 --- a/semantic-conventions/CHANGELOG.md +++ b/semantic-conventions/CHANGELOG.md @@ -6,6 +6,8 @@ Please update the changelog as part of any significant pull request. - BREAKING: Make stability and deprecation independent properties. ([#244](https://github.com/open-telemetry/build-tools/pull/244)) +- Add backward-compatibility check mode. + ([#271](https://github.com/open-telemetry/build-tools/pull/271)) ## v0.23.0 diff --git a/semantic-conventions/src/opentelemetry/semconv/main.py b/semantic-conventions/src/opentelemetry/semconv/main.py index 5da43019..c1ed9f03 100644 --- a/semantic-conventions/src/opentelemetry/semconv/main.py +++ b/semantic-conventions/src/opentelemetry/semconv/main.py @@ -17,13 +17,13 @@ import argparse import glob import os -import requests import sys import tempfile import zipfile - from typing import List +import requests + from opentelemetry.semconv.model.semantic_convention import ( CONVENTION_CLS_BY_GROUP_TYPE, SemanticConventionSet, From 761f193e557274fabc46a6fd1952d0d8c60b275b Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Tue, 20 Feb 2024 14:50:36 -0800 Subject: [PATCH 06/11] mypy --- semantic-conventions/dev-requirements.txt | 3 +-- semantic-conventions/mypy.ini | 4 ++++ semantic-conventions/setup.cfg | 1 + .../src/opentelemetry/semconv/main.py | 14 ++++++----- .../semconv/templating/compatibility.py | 24 ++++++++++++------- .../semconv/templating/test_compatibility.py | 1 + 6 files changed, 30 insertions(+), 17 deletions(-) diff --git a/semantic-conventions/dev-requirements.txt b/semantic-conventions/dev-requirements.txt index d3eb9ab1..6ec71734 100644 --- a/semantic-conventions/dev-requirements.txt +++ b/semantic-conventions/dev-requirements.txt @@ -3,5 +3,4 @@ mypy==0.910 pytest==8.0.1 flake8==7.0.0 pylint==3.0.3 -isort==5.13.2 -requests==2.31.0 \ No newline at end of file +isort==5.13.2 \ No newline at end of file diff --git a/semantic-conventions/mypy.ini b/semantic-conventions/mypy.ini index 9ecdc1d5..a8dff7d7 100644 --- a/semantic-conventions/mypy.ini +++ b/semantic-conventions/mypy.ini @@ -5,3 +5,7 @@ ignore_missing_imports = True [mypy-mistune.*] ignore_missing_imports = True + +[mypy-requests.*] +ignore_missing_imports = True + diff --git a/semantic-conventions/setup.cfg b/semantic-conventions/setup.cfg index ce15e71c..0d1cc511 100644 --- a/semantic-conventions/setup.cfg +++ b/semantic-conventions/setup.cfg @@ -40,6 +40,7 @@ install_requires = ruamel.yaml~=0.16 Jinja2~=3.0 mistune==2.0.0a6 + requests==2.31.0 [options.packages.find] where = src diff --git a/semantic-conventions/src/opentelemetry/semconv/main.py b/semantic-conventions/src/opentelemetry/semconv/main.py index c1ed9f03..e9bdf7d8 100644 --- a/semantic-conventions/src/opentelemetry/semconv/main.py +++ b/semantic-conventions/src/opentelemetry/semconv/main.py @@ -120,13 +120,14 @@ def check_compatibility(semconv, args, parser): def find_yaml(yaml_root: str, exclude: str) -> List[str]: - if yaml_root is not None: - exclude = set(exclude_file_list(yaml_root if yaml_root else "", exclude)) + excluded_files = set(exclude_file_list(yaml_root if yaml_root else "", exclude)) yaml_files = set(glob.glob(f"{yaml_root}/**/*.yaml", recursive=True)).union( set(glob.glob(f"{yaml_root}/**/*.yml", recursive=True)) ) - return yaml_files - exclude + return list(yaml_files - excluded_files) + + return [] def check_args(arguments, parser): @@ -137,7 +138,7 @@ def check_args(arguments, parser): def parse_only_filter(only: str, parser) -> List[str]: if not only: - return None + return [] types = [t.strip() for t in only.split(",")] unknown_types = [t for t in types if t not in CONVENTION_CLS_BY_GROUP_TYPE.keys()] @@ -321,8 +322,9 @@ def download_previous_version(version: str) -> str: f"https://github.com/open-telemetry/semantic-conventions/archive/{filename}" ) - request = requests.get(semconv_vprev, allow_redirects=True) - open(path_to_zip, "wb").write(request.content) + request = requests.get(semconv_vprev, allow_redirects=True, timeout=30) + with open(path_to_zip, "wb") as zip_file: + zip_file.write(request.content) with zipfile.ZipFile(path_to_zip, "r") as zip_ref: zip_ref.extractall(path_to_semconv) diff --git a/semantic-conventions/src/opentelemetry/semconv/templating/compatibility.py b/semantic-conventions/src/opentelemetry/semconv/templating/compatibility.py index a2ab6a61..f5ea0bb3 100644 --- a/semantic-conventions/src/opentelemetry/semconv/templating/compatibility.py +++ b/semantic-conventions/src/opentelemetry/semconv/templating/compatibility.py @@ -1,4 +1,5 @@ from opentelemetry.semconv.model.semantic_attribute import ( + EnumAttributeType, EnumMember, RequirementLevel, SemanticAttribute, @@ -20,6 +21,7 @@ def __init__(self, signal: str, name: str, message: str, critical: bool = True): self.signal = signal self.name = name self.message = message + self.critical = critical def __str__(self): return f"\t {self.signal} '{self.name}': {self.message}" @@ -27,8 +29,8 @@ def __str__(self): def __eq__(self, other): if isinstance(other, self.__class__): return self.__dict__ == other.__dict__ - else: - return False + + return False def __ne__(self, other): return not self.__eq__(other) @@ -46,7 +48,7 @@ def __init__( self.previous_semconv = previous_semconv def check(self) -> list[Problem]: - problems = [] + problems = [] # type: list[Problem] for semconv in self.previous_semconv.models.values(): for prev_attr in semconv.attributes_and_templates: if ( @@ -59,7 +61,7 @@ def check(self) -> list[Problem]: self._check_metric(semconv, problems) return problems - def _check_attribute(self, prev: SemanticAttribute, problems: list[str]): + def _check_attribute(self, prev: SemanticAttribute, problems: list[Problem]): cur = self.current_semconv._lookup_attribute(prev.fqn) if cur is None: problems.append(Problem("attribute", prev.fqn, "was removed")) @@ -75,8 +77,8 @@ def _check_attribute(self, prev: SemanticAttribute, problems: list[str]): ) ) - if prev.is_enum: - if not cur.is_enum: + if isinstance(prev.attr_type, EnumAttributeType): + if not isinstance(cur.attr_type, EnumAttributeType): problems.append( Problem( "attribute", @@ -107,7 +109,11 @@ def _check_attribute(self, prev: SemanticAttribute, problems: list[str]): ) def _check_member( - self, fqn: str, prev: EnumMember, members: list[EnumMember], problems: list[str] + self, + fqn: str, + prev: EnumMember, + members: list[EnumMember], + problems: list[Problem], ): for member in members: if prev.value == member.value: @@ -119,7 +125,7 @@ def _check_member( ) ) - def _check_metric(self, prev: MetricSemanticConvention, problems: list[str]): + def _check_metric(self, prev: MetricSemanticConvention, problems: list[Problem]): for cur in self.current_semconv.models.values(): if ( isinstance(cur, MetricSemanticConvention) @@ -159,7 +165,7 @@ def _check_metric_attributes( self, prev: MetricSemanticConvention, cur: MetricSemanticConvention, - problems: list[str], + problems: list[Problem], ): if prev.stability == StabilityLevel.STABLE: prev_default_attributes = [ diff --git a/semantic-conventions/src/tests/semconv/templating/test_compatibility.py b/semantic-conventions/src/tests/semconv/templating/test_compatibility.py index 11cd72d0..85f88fe1 100644 --- a/semantic-conventions/src/tests/semconv/templating/test_compatibility.py +++ b/semantic-conventions/src/tests/semconv/templating/test_compatibility.py @@ -108,6 +108,7 @@ def testMetricAttributeAdded(self): "metric", "metric_one", "attributes changed from '['first.first_attr']' to '['first.first_attr', 'first.second_attr']'", + critical=False, ) ] self.assert_errors(expected_errors, problems) From 6ed0e2c106b1bc11d5520767e8dc1eaf0831e719 Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Wed, 21 Feb 2024 09:15:09 -0800 Subject: [PATCH 07/11] check download status --- semantic-conventions/src/opentelemetry/semconv/main.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/semantic-conventions/src/opentelemetry/semconv/main.py b/semantic-conventions/src/opentelemetry/semconv/main.py index e9bdf7d8..d3b84ee1 100644 --- a/semantic-conventions/src/opentelemetry/semconv/main.py +++ b/semantic-conventions/src/opentelemetry/semconv/main.py @@ -323,6 +323,10 @@ def download_previous_version(version: str) -> str: ) request = requests.get(semconv_vprev, allow_redirects=True, timeout=30) + if request.status_code != 200: + raise Exception( + f"Failed to download semantic conventions version {version}. Status code: {request.status_code}" + ) with open(path_to_zip, "wb") as zip_file: zip_file.write(request.content) From 01bf80de1326f2953e5a8d263b5d134d5e9b52cf Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Wed, 21 Feb 2024 09:25:08 -0800 Subject: [PATCH 08/11] cosmetic output fixes --- semantic-conventions/src/opentelemetry/semconv/main.py | 7 ++++--- .../src/opentelemetry/semconv/templating/compatibility.py | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/semantic-conventions/src/opentelemetry/semconv/main.py b/semantic-conventions/src/opentelemetry/semconv/main.py index d3b84ee1..6d792e0e 100644 --- a/semantic-conventions/src/opentelemetry/semconv/main.py +++ b/semantic-conventions/src/opentelemetry/semconv/main.py @@ -108,10 +108,11 @@ def check_compatibility(semconv, args, parser): compatibility_checker = CompatibilityChecker(semconv, prev_semconv) problems = compatibility_checker.check() - if problems: + if any(problems): print(f"Found {len(problems)} compatibility issues:") - for problem in problems: - print(f"\t {problem}") + problems_str = [str(p) for p in problems] + for problem in sorted(problems_str): + print(f"\t{problem}") if not args.ignore_warnings or ( args.ignore_warnings and any(problem.critical for problem in problems) diff --git a/semantic-conventions/src/opentelemetry/semconv/templating/compatibility.py b/semantic-conventions/src/opentelemetry/semconv/templating/compatibility.py index f5ea0bb3..d1d4ee45 100644 --- a/semantic-conventions/src/opentelemetry/semconv/templating/compatibility.py +++ b/semantic-conventions/src/opentelemetry/semconv/templating/compatibility.py @@ -24,7 +24,7 @@ def __init__(self, signal: str, name: str, message: str, critical: bool = True): self.critical = critical def __str__(self): - return f"\t {self.signal} '{self.name}': {self.message}" + return f"{self.signal} '{self.name}' {self.message}" def __eq__(self, other): if isinstance(other, self.__class__): From 4ceb53eb0a4a86efdf3bf1f05a3ed50f61e36411 Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Wed, 21 Feb 2024 11:22:21 -0800 Subject: [PATCH 09/11] better way to raise error --- .../src/opentelemetry/semconv/main.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/semantic-conventions/src/opentelemetry/semconv/main.py b/semantic-conventions/src/opentelemetry/semconv/main.py index 6d792e0e..461d0c28 100644 --- a/semantic-conventions/src/opentelemetry/semconv/main.py +++ b/semantic-conventions/src/opentelemetry/semconv/main.py @@ -110,8 +110,7 @@ def check_compatibility(semconv, args, parser): if any(problems): print(f"Found {len(problems)} compatibility issues:") - problems_str = [str(p) for p in problems] - for problem in sorted(problems_str): + for problem in sorted(str(p) for p in problems): print(f"\t{problem}") if not args.ignore_warnings or ( @@ -323,13 +322,11 @@ def download_previous_version(version: str) -> str: f"https://github.com/open-telemetry/semantic-conventions/archive/{filename}" ) - request = requests.get(semconv_vprev, allow_redirects=True, timeout=30) - if request.status_code != 200: - raise Exception( - f"Failed to download semantic conventions version {version}. Status code: {request.status_code}" - ) + response = requests.get(semconv_vprev, allow_redirects=True, timeout=30) + response.raise_for_status() + with open(path_to_zip, "wb") as zip_file: - zip_file.write(request.content) + zip_file.write(response.content) with zipfile.ZipFile(path_to_zip, "r") as zip_ref: zip_ref.extractall(path_to_semconv) From 9f48b3600e407b333b0b9bc29a74a80a348f58b8 Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Thu, 22 Feb 2024 12:14:46 -0800 Subject: [PATCH 10/11] prohibit stable member id change, improve tests and error messages --- .../semconv/templating/compatibility.py | 23 ++++++++++--- .../compat/enum_member_removed/vnext.yaml | 17 ++++++++++ .../vprev.yaml | 0 .../vnext.yaml | 0 .../data/compat/enum_value_changed/vprev.yaml | 17 ++++++++++ .../src/tests/data/compat/success/vnext.yaml | 4 +-- .../semconv/templating/test_compatibility.py | 32 +++++++++++++------ 7 files changed, 77 insertions(+), 16 deletions(-) create mode 100644 semantic-conventions/src/tests/data/compat/enum_member_removed/vnext.yaml rename semantic-conventions/src/tests/data/compat/{enum_value_removed => enum_member_removed}/vprev.yaml (100%) rename semantic-conventions/src/tests/data/compat/{enum_value_removed => enum_value_changed}/vnext.yaml (100%) create mode 100644 semantic-conventions/src/tests/data/compat/enum_value_changed/vprev.yaml diff --git a/semantic-conventions/src/opentelemetry/semconv/templating/compatibility.py b/semantic-conventions/src/opentelemetry/semconv/templating/compatibility.py index d1d4ee45..0a28366f 100644 --- a/semantic-conventions/src/opentelemetry/semconv/templating/compatibility.py +++ b/semantic-conventions/src/opentelemetry/semconv/templating/compatibility.py @@ -87,6 +87,9 @@ def _check_attribute(self, prev: SemanticAttribute, problems: list[Problem]): ) ) else: + # enum type change inevitably causes some values to be removed + # which will be reported in _check_member method as well. + # keeping this check to provide more detailed error message if cur.attr_type.enum_type != prev.attr_type.enum_type: problems.append( Problem( @@ -116,13 +119,23 @@ def _check_member( problems: list[Problem], ): for member in members: - if prev.value == member.value: + if prev.member_id == member.member_id: + if prev.value != member.value: + member_value = ( + f'"{member.value}"' + if isinstance(member.value, str) + else member.value + ) + problems.append( + Problem( + "enum attribute member", + f"{fqn}.{prev.member_id}", + f"value changed from '{prev.value}' to '{member_value}'", + ) + ) return - problems.append( - Problem( - "attribute", fqn, f"enum member with value '{prev.value}' was removed" - ) + Problem("enum attribute member", f"{fqn}.{prev.member_id}", "was removed") ) def _check_metric(self, prev: MetricSemanticConvention, problems: list[Problem]): diff --git a/semantic-conventions/src/tests/data/compat/enum_member_removed/vnext.yaml b/semantic-conventions/src/tests/data/compat/enum_member_removed/vnext.yaml new file mode 100644 index 00000000..9098298c --- /dev/null +++ b/semantic-conventions/src/tests/data/compat/enum_member_removed/vnext.yaml @@ -0,0 +1,17 @@ +groups: + - id: first + type: attribute_group + brief: "First group." + note: "First group note." + prefix: "first" + attributes: + - id: third_attr + type: + members: + - id: enum_two + brief: "enum two" + value: "two" + brief: "third attribute" + note: "third attribute note" + examples: ["two"] + stability: stable \ No newline at end of file diff --git a/semantic-conventions/src/tests/data/compat/enum_value_removed/vprev.yaml b/semantic-conventions/src/tests/data/compat/enum_member_removed/vprev.yaml similarity index 100% rename from semantic-conventions/src/tests/data/compat/enum_value_removed/vprev.yaml rename to semantic-conventions/src/tests/data/compat/enum_member_removed/vprev.yaml diff --git a/semantic-conventions/src/tests/data/compat/enum_value_removed/vnext.yaml b/semantic-conventions/src/tests/data/compat/enum_value_changed/vnext.yaml similarity index 100% rename from semantic-conventions/src/tests/data/compat/enum_value_removed/vnext.yaml rename to semantic-conventions/src/tests/data/compat/enum_value_changed/vnext.yaml diff --git a/semantic-conventions/src/tests/data/compat/enum_value_changed/vprev.yaml b/semantic-conventions/src/tests/data/compat/enum_value_changed/vprev.yaml new file mode 100644 index 00000000..ab72dfa0 --- /dev/null +++ b/semantic-conventions/src/tests/data/compat/enum_value_changed/vprev.yaml @@ -0,0 +1,17 @@ +groups: + - id: first + type: attribute_group + brief: "first group" + note: "first group note" + prefix: "first" + attributes: + - id: third_attr + type: + members: + - id: enum_one + brief: "enum one" + value: "one" + brief: "third attribute" + note: "third attribute note" + examples: ["one"] + stability: stable \ No newline at end of file diff --git a/semantic-conventions/src/tests/data/compat/success/vnext.yaml b/semantic-conventions/src/tests/data/compat/success/vnext.yaml index e3d89d07..388e6efc 100644 --- a/semantic-conventions/src/tests/data/compat/success/vnext.yaml +++ b/semantic-conventions/src/tests/data/compat/success/vnext.yaml @@ -20,10 +20,10 @@ groups: - id: third_attr type: members: - - id: one + - id: enum_one brief: "Enum one." value: "one" - - id: two + - id: enum_two brief: "Enum two." value: "two" brief: "Third attribute." diff --git a/semantic-conventions/src/tests/semconv/templating/test_compatibility.py b/semantic-conventions/src/tests/semconv/templating/test_compatibility.py index 85f88fe1..2497fdd5 100644 --- a/semantic-conventions/src/tests/semconv/templating/test_compatibility.py +++ b/semantic-conventions/src/tests/semconv/templating/test_compatibility.py @@ -142,23 +142,37 @@ def testEnumTypeChanged(self): "enum type changed from 'string' to 'int'", ), Problem( - "attribute", - "first.third_attr", - "enum member with value 'one' was removed", + "enum attribute member", + "first.third_attr.enum_one", + "value changed from 'one' to '1'", ), ] self.assert_errors(expected_errors, problems) - def testEnumValueRemoved(self): - cur = self.parse_semconv("compat/enum_value_removed/vnext.yaml") - prev = self.parse_semconv("compat/enum_value_removed/vprev.yaml") + def testEnumValueChanged(self): + cur = self.parse_semconv("compat/enum_value_changed/vnext.yaml") + prev = self.parse_semconv("compat/enum_value_changed/vprev.yaml") checker = CompatibilityChecker(cur, prev) problems = checker.check() expected_errors = [ Problem( - "attribute", - "first.third_attr", - "enum member with value 'one' was removed", + "enum attribute member", + "first.third_attr.enum_one", + "value changed from 'one' to '\"1\"'", + ) + ] + self.assert_errors(expected_errors, problems) + + def testEnumMemberRemoved(self): + cur = self.parse_semconv("compat/enum_member_removed/vnext.yaml") + prev = self.parse_semconv("compat/enum_member_removed/vprev.yaml") + checker = CompatibilityChecker(cur, prev) + problems = checker.check() + expected_errors = [ + Problem( + "enum attribute member", + "first.third_attr.enum_one", + "was removed", ) ] self.assert_errors(expected_errors, problems) From 0ae36a1d905122d9c2342b31aed05eece7ba5a5c Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Thu, 22 Feb 2024 12:20:50 -0800 Subject: [PATCH 11/11] unused files --- .../markdown/enum_extend_list/enum_attr.yaml | 27 ------------------- .../markdown/enum_extend_list/expected.md | 24 ----------------- .../data/markdown/enum_extend_list/input.md | 8 ------ 3 files changed, 59 deletions(-) delete mode 100644 semantic-conventions/src/tests/data/markdown/enum_extend_list/enum_attr.yaml delete mode 100644 semantic-conventions/src/tests/data/markdown/enum_extend_list/expected.md delete mode 100644 semantic-conventions/src/tests/data/markdown/enum_extend_list/input.md diff --git a/semantic-conventions/src/tests/data/markdown/enum_extend_list/enum_attr.yaml b/semantic-conventions/src/tests/data/markdown/enum_extend_list/enum_attr.yaml deleted file mode 100644 index 1714c7aa..00000000 --- a/semantic-conventions/src/tests/data/markdown/enum_extend_list/enum_attr.yaml +++ /dev/null @@ -1,27 +0,0 @@ -groups: - - id: db - prefix: db - brief: "" - attributes: - - id: system - type: - allow_custom_values: false - members: - - id: mysql - value: "mysql" - - id: postgresql - value: "postgresql" - - id: mssql - value: "mssql" - brief: "" - examples: ["mysql"] - - id: db.redis - brief: "" - prefix: db.redis - attributes: - - ref: system - type: - members: - - id: redis - value: "redis" - examples: ["redis"] diff --git a/semantic-conventions/src/tests/data/markdown/enum_extend_list/expected.md b/semantic-conventions/src/tests/data/markdown/enum_extend_list/expected.md deleted file mode 100644 index ac5222cd..00000000 --- a/semantic-conventions/src/tests/data/markdown/enum_extend_list/expected.md +++ /dev/null @@ -1,24 +0,0 @@ - -| Attribute | Type | Description | Examples | Requirement Level | -|---|---|---|---|---| -| `db.system` | string | | `"one"` | Recommended | - -`db.system` MUST be one of the following: - -| Value | Description | -|---|---| -| `one` | one | - - - -| Attribute | Type | Description | Examples | Requirement Level | -|---|---|---|---|---| -| `first.enum_attr` | string | | `"one"` | Recommended | - -`first.enum_attr` MUST be one of the following: - -| Value | Description | -|---|---| -| `one` | one | -| `two` | two | - diff --git a/semantic-conventions/src/tests/data/markdown/enum_extend_list/input.md b/semantic-conventions/src/tests/data/markdown/enum_extend_list/input.md deleted file mode 100644 index 8bc34d41..00000000 --- a/semantic-conventions/src/tests/data/markdown/enum_extend_list/input.md +++ /dev/null @@ -1,8 +0,0 @@ - - - - - - - -