From f8fca86b438bad4fff7a457afac228229a225871 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Tue, 7 Sep 2021 16:15:30 +0200 Subject: [PATCH 01/10] Run & fix flake8. --- semantic-conventions/.flake8 | 15 +++++++++++++++ semantic-conventions/dev-requirements.txt | 1 + .../opentelemetry/semconv/model/constraints.py | 4 ++-- .../semconv/model/semantic_attribute.py | 1 - .../semconv/model/semantic_convention.py | 6 +++--- .../src/tests/semconv/model/test_correct_parse.py | 1 - .../src/tests/semconv/model/test_utils.py | 3 --- .../src/tests/semconv/templating/test_code.py | 2 -- 8 files changed, 21 insertions(+), 12 deletions(-) create mode 100644 semantic-conventions/.flake8 diff --git a/semantic-conventions/.flake8 b/semantic-conventions/.flake8 new file mode 100644 index 00000000..eea000cf --- /dev/null +++ b/semantic-conventions/.flake8 @@ -0,0 +1,15 @@ +# Adapted from https://black.readthedocs.io/en/stable/guides/using_black_with_other_tools.html#flake8 +[flake8] +extend-ignore = E203 +max-line-length = 120 +exclude = + .bzr + .git + .hg + .svn + .tox + CVS + .venv*/ + venv*/ + target + __pycache__ diff --git a/semantic-conventions/dev-requirements.txt b/semantic-conventions/dev-requirements.txt index a8207ac2..16e86f7d 100644 --- a/semantic-conventions/dev-requirements.txt +++ b/semantic-conventions/dev-requirements.txt @@ -1,3 +1,4 @@ black==21.6b0 mypy==0.770 pytest==6.1.1 +flake8==3.9.2 diff --git a/semantic-conventions/src/opentelemetry/semconv/model/constraints.py b/semantic-conventions/src/opentelemetry/semconv/model/constraints.py index eb1d5771..cf58704c 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/constraints.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/constraints.py @@ -12,8 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. -from dataclasses import dataclass, field, replace -from typing import List, Tuple, Set +from dataclasses import dataclass, replace +from typing import List, Tuple from opentelemetry.semconv.model.exceptions import ValidationError from opentelemetry.semconv.model.semantic_attribute import SemanticAttribute diff --git a/semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py b/semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py index bd3e30db..6cc1b3ba 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py @@ -12,7 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -import numbers import re from collections.abc import Iterable from dataclasses import dataclass, replace diff --git a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py index cfedc352..2b490edc 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py @@ -15,7 +15,7 @@ import sys from dataclasses import dataclass, field from enum import Enum -from typing import List, Union, Iterable, Tuple +from typing import Union import typing from ruamel.yaml import YAML @@ -25,7 +25,6 @@ from opentelemetry.semconv.model.semantic_attribute import ( HasAttributes, SemanticAttribute, - StabilityLevel, Required, unique_attributes, ) @@ -463,7 +462,8 @@ def _populate_events(self): if not isinstance(event, EventSemanticConvention): raise ValidationError.from_yaml_pos( semconv._position, - "Semantic Convention {} has {} as event but the latter is not a semantic convention for events!".format( + "Semantic Convention {} has {} as event but" + " the latter is not a semantic convention for events!".format( semconv.semconv_id, event_id ), ) diff --git a/semantic-conventions/src/tests/semconv/model/test_correct_parse.py b/semantic-conventions/src/tests/semconv/model/test_correct_parse.py index b48f9b88..c64c9ec1 100644 --- a/semantic-conventions/src/tests/semconv/model/test_correct_parse.py +++ b/semantic-conventions/src/tests/semconv/model/test_correct_parse.py @@ -21,7 +21,6 @@ StabilityLevel, ) from opentelemetry.semconv.model.semantic_convention import ( - parse_semantic_convention_groups, SemanticConventionSet, SpanSemanticConvention, EventSemanticConvention, diff --git a/semantic-conventions/src/tests/semconv/model/test_utils.py b/semantic-conventions/src/tests/semconv/model/test_utils.py index ebabe11c..4be8f3c2 100644 --- a/semantic-conventions/src/tests/semconv/model/test_utils.py +++ b/semantic-conventions/src/tests/semconv/model/test_utils.py @@ -16,12 +16,9 @@ from opentelemetry.semconv.model.utils import ( validate_id, validate_values, - check_no_missing_keys, ) from opentelemetry.semconv.model.exceptions import ValidationError -from ruamel.yaml.comments import CommentedMap - _POSITION = [10, 2] diff --git a/semantic-conventions/src/tests/semconv/templating/test_code.py b/semantic-conventions/src/tests/semconv/templating/test_code.py index 1c91935e..78278917 100644 --- a/semantic-conventions/src/tests/semconv/templating/test_code.py +++ b/semantic-conventions/src/tests/semconv/templating/test_code.py @@ -1,6 +1,4 @@ import io -import tempfile -import os from opentelemetry.semconv.model.semantic_convention import SemanticConventionSet from opentelemetry.semconv.templating.code import CodeRenderer From c664783b35a0607e24146d53c04df877abc17a86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Tue, 7 Sep 2021 16:58:54 +0200 Subject: [PATCH 02/10] pylint main code. --- semantic-conventions/.pylintrc | 7 ++ .../src/opentelemetry/semconv/main.py | 14 +-- .../opentelemetry/semconv/model/exceptions.py | 2 +- .../semconv/model/semantic_attribute.py | 95 ++++++++---------- .../semconv/model/semantic_convention.py | 98 +++++++++---------- .../opentelemetry/semconv/templating/code.py | 24 ++--- .../semconv/templating/markdown/__init__.py | 15 ++- 7 files changed, 115 insertions(+), 140 deletions(-) create mode 100644 semantic-conventions/.pylintrc diff --git a/semantic-conventions/.pylintrc b/semantic-conventions/.pylintrc new file mode 100644 index 00000000..af797212 --- /dev/null +++ b/semantic-conventions/.pylintrc @@ -0,0 +1,7 @@ +[MESSAGES CONTROL] +# TODO: We should actually fix the warnings instead of disabling them +# except for the stuff up to fixme, which is fine to ignore / defer to black. +disable = bad-whitespace, wrong-import-order, wrong-hanging-indentation, line-too-long, missing-docstring, unused-import, fixme, invalid-name, too-many-instance-attributes, too-many-locals, too-many-statements, too-many-branches, too-many-arguments, too-many-public-methods, too-few-public-methods, protected-access + +[FORMAT] +good-names=e,ex diff --git a/semantic-conventions/src/opentelemetry/semconv/main.py b/semantic-conventions/src/opentelemetry/semconv/main.py index 10a0e978..f45d9686 100644 --- a/semantic-conventions/src/opentelemetry/semconv/main.py +++ b/semantic-conventions/src/opentelemetry/semconv/main.py @@ -20,12 +20,8 @@ from typing import List from opentelemetry.semconv.model.semantic_convention import ( + CONVENTION_CLS_BY_GROUP_TYPE, SemanticConventionSet, - SpanSemanticConvention, - ResourceSemanticConvention, - EventSemanticConvention, - MetricSemanticConvention, - UnitSemanticConvention, ) from opentelemetry.semconv.templating.code import CodeRenderer @@ -206,13 +202,7 @@ def setup_parser(): ) parser.add_argument( "--only", - choices=[ - SpanSemanticConvention.GROUP_TYPE_NAME, - ResourceSemanticConvention.GROUP_TYPE_NAME, - EventSemanticConvention.GROUP_TYPE_NAME, - MetricSemanticConvention.GROUP_TYPE_NAME, - UnitSemanticConvention.GROUP_TYPE_NAME, - ], + choices=list(CONVENTION_CLS_BY_GROUP_TYPE.keys()), help="Process only semantic conventions of the specified type.", ) parser.add_argument( diff --git a/semantic-conventions/src/opentelemetry/semconv/model/exceptions.py b/semantic-conventions/src/opentelemetry/semconv/model/exceptions.py index a6aef64c..5c45ca6c 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/exceptions.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/exceptions.py @@ -28,7 +28,7 @@ def from_yaml_pos(cls, pos, msg): return cls(pos[0] + 1, pos[1] + 1, msg) def __init__(self, line, column, message): - super(ValidationError, self).__init__(line, column, message) + super().__init__(line, column, message) self.message = message self.line = line self.column = column diff --git a/semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py b/semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py index 6cc1b3ba..53ab5425 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py @@ -16,7 +16,7 @@ from collections.abc import Iterable from dataclasses import dataclass, replace from enum import Enum -from typing import List, Union +from typing import List, Union, Dict from ruamel.yaml.comments import CommentedMap, CommentedSeq @@ -40,20 +40,6 @@ class StabilityLevel(Enum): DEPRECATED = 3 -class HasAttributes: - def _set_attributes(self, prefix, stability, node): - self.attrs_by_name = SemanticAttribute.parse( - prefix, stability, node.get("attributes") - ) - - @property - def attributes(self): - if not hasattr(self, "attrs_by_name"): - return [] - - return list(self.attrs_by_name.values()) - - def unique_attributes(attributes): output = [] for x in attributes: @@ -96,7 +82,7 @@ def is_enum(self): return isinstance(self.attr_type, EnumAttributeType) @staticmethod - def parse(prefix, semconv_stability, yaml_attributes): + def parse(prefix, semconv_stability, yaml_attributes) -> "Dict[str, SemanticAttribute]": """This method parses the yaml representation for semantic attributes creating the respective SemanticAttribute objects. """ @@ -396,7 +382,7 @@ def to_bool(key, parent_object): if isinstance(yaml_value, str): if AttributeType.bool_type_true.fullmatch(yaml_value): return True - elif AttributeType.bool_type_false.fullmatch(yaml_value): + if AttributeType.bool_type_false.fullmatch(yaml_value): return False position = parent_object.lc.data[key] msg = "Value '{}' for {} field is not allowed".format(yaml_value, key) @@ -430,47 +416,46 @@ def parse(attribute_type): if isinstance(attribute_type, str): if AttributeType.is_simple_type(attribute_type): return attribute_type - else: # Wrong type used - rise the exception and fill the missing data in the parent + # Wrong type used - raise the exception and fill the missing data in the parent + raise ValidationError( + 0, 0, "Invalid type: {} is not allowed".format(attribute_type) + ) + allowed_keys = ["allow_custom_values", "members"] + mandatory_keys = ["members"] + validate_values(attribute_type, allowed_keys, mandatory_keys) + custom_values = ( + bool(attribute_type.get("allow_custom_values")) + if "allow_custom_values" in attribute_type + else False + ) + members = [] + if attribute_type["members"] is None or len(attribute_type["members"]) < 1: + # Missing members - rise the exception and fill the missing data in the parent + raise ValidationError(0, 0, "Enumeration without values!") + + allowed_keys = ["id", "value", "brief", "note"] + mandatory_keys = ["id", "value"] + for member in attribute_type["members"]: + validate_values(member, allowed_keys, mandatory_keys) + if not EnumAttributeType.is_valid_enum_value(member["value"]): raise ValidationError( - 0, 0, "Invalid type: {} is not allowed".format(attribute_type) + 0, 0, "Invalid value used in enum: <{}>".format(member["value"]) ) - else: - allowed_keys = ["allow_custom_values", "members"] - mandatory_keys = ["members"] - validate_values(attribute_type, allowed_keys, mandatory_keys) - custom_values = ( - bool(attribute_type.get("allow_custom_values")) - if "allow_custom_values" in attribute_type - else False - ) - members = [] - if attribute_type["members"] is None or len(attribute_type["members"]) < 1: - # Missing members - rise the exception and fill the missing data in the parent - raise ValidationError(0, 0, "Enumeration without values!") - - allowed_keys = ["id", "value", "brief", "note"] - mandatory_keys = ["id", "value"] - for member in attribute_type["members"]: - validate_values(member, allowed_keys, mandatory_keys) - if not EnumAttributeType.is_valid_enum_value(member["value"]): - raise ValidationError( - 0, 0, "Invalid value used in enum: <{}>".format(member["value"]) - ) - members.append( - EnumMember( - member_id=member["id"], - value=member["value"], - brief=member.get("brief").strip() - if "brief" in member - else member["id"], - note=member.get("note").strip() if "note" in member else "", - ) + members.append( + EnumMember( + member_id=member["id"], + value=member["value"], + brief=member.get("brief").strip() + if "brief" in member + else member["id"], + note=member.get("note").strip() if "note" in member else "", ) - enum_type = AttributeType.get_type(members[0].value) - for m in members: - if enum_type != AttributeType.get_type(m.value): - raise ValidationError(0, 0, "Enumeration type inconsistent!") - return EnumAttributeType(custom_values, members, enum_type) + ) + enum_type = AttributeType.get_type(members[0].value) + for m in members: + if enum_type != AttributeType.get_type(m.value): + raise ValidationError(0, 0, "Enumeration type inconsistent!") + return EnumAttributeType(custom_values, members, enum_type) @dataclass diff --git a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py index 2b490edc..9962928b 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py @@ -23,7 +23,6 @@ from opentelemetry.semconv.model.constraints import Include, AnyOf, parse_constraints from opentelemetry.semconv.model.exceptions import ValidationError from opentelemetry.semconv.model.semantic_attribute import ( - HasAttributes, SemanticAttribute, Required, unique_attributes, @@ -61,17 +60,7 @@ def parse_semantic_convention_type(type_value): # Gracefully transition to the new types if type_value is None: return SpanSemanticConvention - enum_map = { - cls.GROUP_TYPE_NAME: cls - for cls in [ - SpanSemanticConvention, - ResourceSemanticConvention, - EventSemanticConvention, - MetricSemanticConvention, - UnitSemanticConvention, - ] - } - return enum_map.get(type_value) + return CONVENTION_CLS_BY_GROUP_TYPE.get(type_value) def parse_semantic_convention_groups(yaml_file): @@ -110,6 +99,13 @@ def SemanticConvention(group): class BaseSemanticConvention(ValidatableYamlNode): """Contains the model extracted from a yaml file""" + @property + def attributes(self): + if not hasattr(self, "attrs_by_name"): + return [] + + return list(self.attrs_by_name.values()) + def __init__(self, group): super().__init__(group) @@ -125,10 +121,9 @@ def __init__(self, group): self.extends = group.get("extends", "").strip() self.events = group.get("events", ()) self.constraints = parse_constraints(group.get("constraints", ())) - - @property - def attributes(self): - return [] + self.attrs_by_name = SemanticAttribute.parse( + self.prefix, self.stability, group.get("attributes") + ) def contains_attribute(self, attr: "SemanticAttribute"): for local_attr in self.attributes: @@ -140,25 +135,19 @@ def contains_attribute(self, attr: "SemanticAttribute"): return False def all_attributes(self): - attr: SemanticAttribute - return unique_attributes( - [attr for attr in self.attributes] + self.conditional_attributes() - ) + return unique_attributes(self.attributes + self.conditional_attributes()) def sampling_attributes(self): - attr: SemanticAttribute return unique_attributes( [attr for attr in self.attributes if attr.sampling_relevant] ) def required_attributes(self): - attr: SemanticAttribute return unique_attributes( [attr for attr in self.attributes if attr.required == Required.ALWAYS] ) def conditional_attributes(self): - attr: SemanticAttribute return unique_attributes( [attr for attr in self.attributes if attr.required == Required.CONDITIONAL] ) @@ -185,7 +174,7 @@ def validate_values(self): validate_id(self.prefix, self._position) -class ResourceSemanticConvention(HasAttributes, BaseSemanticConvention): +class ResourceSemanticConvention(BaseSemanticConvention): GROUP_TYPE_NAME = "resource" allowed_keys = ( @@ -200,12 +189,8 @@ class ResourceSemanticConvention(HasAttributes, BaseSemanticConvention): "constraints", ) - def __init__(self, group): - super().__init__(group) - self._set_attributes(self.prefix, self.stability, group) - -class SpanSemanticConvention(HasAttributes, BaseSemanticConvention): +class SpanSemanticConvention(BaseSemanticConvention): GROUP_TYPE_NAME = "span" allowed_keys = ( @@ -224,7 +209,6 @@ class SpanSemanticConvention(HasAttributes, BaseSemanticConvention): def __init__(self, group): super().__init__(group) - self._set_attributes(self.prefix, self.stability, group) self.span_kind = SpanKind.parse(group.get("span_kind")) if self.span_kind is None: position = group.lc.data["span_kind"] @@ -232,7 +216,10 @@ def __init__(self, group): raise ValidationError.from_yaml_pos(position, msg) -class EventSemanticConvention(HasAttributes, BaseSemanticConvention): +print("MRO: ", SpanSemanticConvention.__mro__) + + +class EventSemanticConvention(BaseSemanticConvention): GROUP_TYPE_NAME = "event" allowed_keys = ( @@ -247,10 +234,6 @@ class EventSemanticConvention(HasAttributes, BaseSemanticConvention): "constraints", ) - def __init__(self, group): - super().__init__(group) - self._set_attributes(self.prefix, self.stability, group) - class UnitSemanticConvention(BaseSemanticConvention): GROUP_TYPE_NAME = "units" @@ -429,23 +412,22 @@ def _populate_anyof_attributes(self): any_of: AnyOf for semconv in self.models.values(): for any_of in semconv.constraints: - if isinstance(any_of, AnyOf): - index = -1 - for attr_ids in any_of.choice_list_ids: - constraint_attrs = [] - index += 1 - for attr_id in attr_ids: - ref_attr = self._lookup_attribute(attr_id) - if ref_attr is None: - raise ValidationError.from_yaml_pos( - any_of._yaml_src_position[index], - "Any_of attribute '{}' of semantic convention {} does not exists!".format( - attr_id, semconv.semconv_id - ), - ) - constraint_attrs.append(ref_attr) - if constraint_attrs: - any_of.add_attributes(constraint_attrs) + if not isinstance(any_of, AnyOf): + continue + for index, attr_ids in enumerate(any_of.choice_list_ids): + constraint_attrs = [] + for attr_id in attr_ids: + ref_attr = self._lookup_attribute(attr_id) + if ref_attr is None: + raise ValidationError.from_yaml_pos( + any_of._yaml_src_position[index], + "Any_of attribute '{}' of semantic convention {} does not exists!".format( + attr_id, semconv.semconv_id + ), + ) + constraint_attrs.append(ref_attr) + if constraint_attrs: + any_of.add_attributes(constraint_attrs) def _populate_events(self): for semconv in self.models.values(): @@ -555,3 +537,15 @@ def attributes(self): for semconv in self.models.values(): output.extend(semconv.attributes) return output + + +CONVENTION_CLS_BY_GROUP_TYPE = { + cls.GROUP_TYPE_NAME: cls + for cls in [ + SpanSemanticConvention, + ResourceSemanticConvention, + EventSemanticConvention, + MetricSemanticConvention, + UnitSemanticConvention, + ] +} diff --git a/semantic-conventions/src/opentelemetry/semconv/templating/code.py b/semantic-conventions/src/opentelemetry/semconv/templating/code.py index 380da50a..303e9a73 100644 --- a/semantic-conventions/src/opentelemetry/semconv/templating/code.py +++ b/semantic-conventions/src/opentelemetry/semconv/templating/code.py @@ -38,15 +38,15 @@ def render_markdown( heading=None, block_code=None, block_quote=None, - list=None, + list=None, # pylint:disable=redefined-builtin list_item=None, code=None, ): class CustomRender(mistune.HTMLRenderer): - def link(self, url, text=None, title=None): + def link(self, url, text=None, title=None): # pylint:disable=arguments-renamed if link: return link.format(url, text, title) - return super().link(url, text, title) if html else url + return super().link(url, text, title) if html else link def image(self, src, alt="", title=None): if image: @@ -63,10 +63,10 @@ def strong(self, text): return strong.format(text) return super().strong(text) if html else text - def inline_html(self, html_text): + def inline_html(self, html): if inline_html: - return inline_html.format(html_text) - return super().inline_html(html_text) if html else html_text + return inline_html.format(html) + return super().inline_html(html) if html else html def paragraph(self, text): if paragraph: @@ -135,12 +135,12 @@ def to_html_links(doc_string: typing.Optional[typing.Union[str, TextWithLinks]]) def regex_replace(text: str, pattern: str, replace: str): # convert standard dollar notation to python - replace = re.sub(r"\$", r"\\", replace) + replace = re.sub(r"\$", r"\\", replace) # TODO This is *very* surprising behavior return re.sub(pattern, replace, text, 0, re.U) -def merge(list: typing.List, elm): - return list.extend(elm) +def merge(elems: typing.List, elm): + return elems.extend(elm) def to_const_name(name: str) -> str: @@ -206,10 +206,10 @@ def setup_environment(env: Environment): @staticmethod def prefix_output_file(file_name, pattern, semconv): - base = os.path.basename(file_name) - dir = os.path.dirname(file_name) + basename = os.path.basename(file_name) + dirname = os.path.dirname(file_name) value = getattr(semconv, pattern) - return os.path.join(dir, to_camelcase(value, True), base) + return os.path.join(dirname, to_camelcase(value, True), basename) def render( self, diff --git a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py index 0b9ad3bd..ec9d62d0 100644 --- a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py +++ b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py @@ -179,7 +179,7 @@ def to_markdown_anyof(self, anyof: AnyOf, output: io.StringIO): This method renders anyof constraints into markdown lists """ if anyof.inherited and not self.render_ctx.is_full: - return "" + return output.write( "\n**Additional attribute requirements:** At least one of the following sets of attributes is " "required:\n\n" @@ -199,7 +199,8 @@ def to_markdown_notes(self, output: io.StringIO): output.write("\n**[{}]:** {}\n".format(counter, note)) counter += 1 - def to_markdown_unit_table(self, members, output): + @staticmethod + def to_markdown_unit_table(members, output: io.StringIO): output.write("\n") output.write( "| Name | Kind of Quantity | Unit String |\n" @@ -271,12 +272,10 @@ def to_markdown_constraint( """ if isinstance(obj, AnyOf): self.to_markdown_anyof(obj, output) - return - elif isinstance(obj, Include): - return - raise Exception( - "Trying to generate Markdown for a wrong type {}".format(type(obj)) - ) + elif not isinstance(obj, Include): + raise Exception( + "Trying to generate Markdown for a wrong type {}".format(type(obj)) + ) def render_md(self): for md_filename in self.file_names: From 2cbd7d09309d1f786c635f6708ab3bc2dc453145 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Tue, 7 Sep 2021 17:52:18 +0200 Subject: [PATCH 03/10] fup main --- .../src/opentelemetry/semconv/model/semantic_attribute.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py b/semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py index 53ab5425..d977cb5f 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py @@ -82,7 +82,9 @@ def is_enum(self): return isinstance(self.attr_type, EnumAttributeType) @staticmethod - def parse(prefix, semconv_stability, yaml_attributes) -> "Dict[str, SemanticAttribute]": + def parse( + prefix, semconv_stability, yaml_attributes + ) -> "Dict[str, SemanticAttribute]": """This method parses the yaml representation for semantic attributes creating the respective SemanticAttribute objects. """ From dc711171b50871f4e3354ed9a08d06186be4e60f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Tue, 7 Sep 2021 17:52:51 +0200 Subject: [PATCH 04/10] pylint test code. --- semantic-conventions/src/tests/conftest.py | 3 +++ .../tests/semconv/model/test_correct_parse.py | 18 ++++-------------- .../semconv/model/test_error_detection.py | 4 ---- .../semconv/model/test_semantic_convention.py | 2 +- .../src/tests/semconv/model/test_utils.py | 14 ++++++++------ .../tests/semconv/templating/test_markdown.py | 1 + 6 files changed, 17 insertions(+), 25 deletions(-) diff --git a/semantic-conventions/src/tests/conftest.py b/semantic-conventions/src/tests/conftest.py index 5f449543..f1c83e43 100644 --- a/semantic-conventions/src/tests/conftest.py +++ b/semantic-conventions/src/tests/conftest.py @@ -6,6 +6,9 @@ _TEST_DIR = os.path.dirname(__file__) +# Fixtures in pytest work with reused outer names, so shut up pylint here. +# pylint:disable=redefined-outer-name + @pytest.fixture def test_file_path(): diff --git a/semantic-conventions/src/tests/semconv/model/test_correct_parse.py b/semantic-conventions/src/tests/semconv/model/test_correct_parse.py index c64c9ec1..2ffa08da 100644 --- a/semantic-conventions/src/tests/semconv/model/test_correct_parse.py +++ b/semantic-conventions/src/tests/semconv/model/test_correct_parse.py @@ -16,10 +16,7 @@ import unittest from opentelemetry.semconv.model.constraints import Include, AnyOf -from opentelemetry.semconv.model.semantic_attribute import ( - SemanticAttribute, - StabilityLevel, -) +from opentelemetry.semconv.model.semantic_attribute import StabilityLevel from opentelemetry.semconv.model.semantic_convention import ( SemanticConventionSet, SpanSemanticConvention, @@ -235,12 +232,10 @@ def test_event(self): } self.semantic_convention_check(event, expected) constraint = event.constraints[0] - self.assertTrue(isinstance(constraint, AnyOf)) + self.assertIsInstance(constraint, AnyOf) constraint: AnyOf - for choice_index in range(len(constraint.choice_list_ids)): - attr_list = constraint.choice_list_ids[choice_index] - for attr_index in range(len(attr_list)): - attr = attr_list[attr_index] + for choice_index, attr_list in enumerate(constraint.choice_list_ids): + for attr_index, attr in enumerate(attr_list): self.assertEqual( event.attrs_by_name.get(attr), constraint.choice_list_attributes[choice_index][attr_index], @@ -313,7 +308,6 @@ def test_ref(self): client = list(semconv.models.values())[1] server = list(semconv.models.values())[2] - attr: SemanticAttribute self.assertIsNotNone(client.attributes[1].ref) self.assertIsNotNone(client.attributes[1].attr_type) @@ -679,7 +673,3 @@ def semantic_convention_check(self, s, expected): def load_file(self, filename): return os.path.join(self._TEST_DIR, "..", "..", "data", filename) - - -if __name__ == "__main__": - unittest.main() diff --git a/semantic-conventions/src/tests/semconv/model/test_error_detection.py b/semantic-conventions/src/tests/semconv/model/test_error_detection.py index 3617d347..204289d3 100644 --- a/semantic-conventions/src/tests/semconv/model/test_error_detection.py +++ b/semantic-conventions/src/tests/semconv/model/test_error_detection.py @@ -451,7 +451,3 @@ def open_yaml(self, path): def load_file(self, filename): return os.path.join(self._TEST_DIR, "..", "..", "data", filename) - - -if __name__ == "__main__": - unittest.main() diff --git a/semantic-conventions/src/tests/semconv/model/test_semantic_convention.py b/semantic-conventions/src/tests/semconv/model/test_semantic_convention.py index 15024fb6..f2d9029e 100644 --- a/semantic-conventions/src/tests/semconv/model/test_semantic_convention.py +++ b/semantic-conventions/src/tests/semconv/model/test_semantic_convention.py @@ -26,7 +26,7 @@ def test_parse_basic(open_test_file): assert conventions is not None assert len(conventions) == 2 - first, second = conventions + first, second = conventions # pylint:disable=unbalanced-tuple-unpacking assert first.semconv_id == "first_group_id" assert first.brief == "first description" diff --git a/semantic-conventions/src/tests/semconv/model/test_utils.py b/semantic-conventions/src/tests/semconv/model/test_utils.py index 4be8f3c2..44b9e7d6 100644 --- a/semantic-conventions/src/tests/semconv/model/test_utils.py +++ b/semantic-conventions/src/tests/semconv/model/test_utils.py @@ -23,7 +23,7 @@ @pytest.mark.parametrize( - "id", + "semconv_id", [ "abc.def", # all lowercase letters "abc.de_fg", # lowercase letters, plus underscore @@ -33,18 +33,20 @@ "abc.123.ab9.de_fg.hi-jk", ], ) -def test_validate_id__valid(id): +def test_validate_id__valid(semconv_id): # valid ids are namespaced, dot separated. The topmost name must start with # a lowercase letter. The rest of the id may contain only lowercase letters, # numbers, underscores, and dashes - validate_id(id, _POSITION) + validate_id(semconv_id, _POSITION) -@pytest.mark.parametrize("id", ["123", "ABC", "abc+def", "ab<", "ab^", "abc.de\xfe"]) -def test_validate_id__invalid(id): +@pytest.mark.parametrize( + "semconv_id", ["123", "ABC", "abc+def", "ab<", "ab^", "abc.de\xfe"] +) +def test_validate_id__invalid(semconv_id): with pytest.raises(ValidationError) as err: - validate_id(id, _POSITION) + validate_id(semconv_id, _POSITION) assert err.value.message.startswith("Invalid id") diff --git a/semantic-conventions/src/tests/semconv/templating/test_markdown.py b/semantic-conventions/src/tests/semconv/templating/test_markdown.py index 1dfa963e..3ee8f21d 100644 --- a/semantic-conventions/src/tests/semconv/templating/test_markdown.py +++ b/semantic-conventions/src/tests/semconv/templating/test_markdown.py @@ -159,6 +159,7 @@ def do_render(): do_render() result = output.getvalue() assert result == (dirpath / expected_name).read_text(encoding="utf-8") + return None _TEST_DIR = os.path.dirname(__file__) From d0180cce28b171b64710ea9766a31656662de8c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Tue, 7 Sep 2021 18:15:53 +0200 Subject: [PATCH 05/10] Add pylint execution. --- .github/workflows/semconvgen.yml | 21 +++++++++++++++------ semantic-conventions/dev-requirements.txt | 1 + semantic-conventions/setup.py | 2 +- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/.github/workflows/semconvgen.yml b/.github/workflows/semconvgen.yml index 1668d2e8..fe8170ac 100644 --- a/.github/workflows/semconvgen.yml +++ b/.github/workflows/semconvgen.yml @@ -12,19 +12,28 @@ on: jobs: tests: runs-on: ubuntu-latest + defaults: + run: + working-directory: semantic-conventions/ steps: - uses: actions/checkout@v1 - uses: actions/setup-python@v2 - - name: install dependencies + - name: install dev-dependencies run: | - pip install -U pip setuptools wheel - pip install -r semantic-conventions/dev-requirements.txt - - name: Check formatting - run: black --check --diff semantic-conventions/ + pip install -U pip + pip install -U setuptools wheel + pip install -r dev-requirements.txt + - name: Check formatting (black) + run: black --check --diff . + - name: Fast basic linting (flake8) + run: flake8 . - name: install - run: pip install -U -e ./semantic-conventions/ + run: pip install -U -e . - name: run tests run: pytest -v + - name: Slow linting (pylint) + run: pylint *.py src/ + build-and-publish-docker: runs-on: ubuntu-latest diff --git a/semantic-conventions/dev-requirements.txt b/semantic-conventions/dev-requirements.txt index 16e86f7d..59bb43a8 100644 --- a/semantic-conventions/dev-requirements.txt +++ b/semantic-conventions/dev-requirements.txt @@ -2,3 +2,4 @@ black==21.6b0 mypy==0.770 pytest==6.1.1 flake8==3.9.2 +pylint==2.10.2 diff --git a/semantic-conventions/setup.py b/semantic-conventions/setup.py index 01c03004..b560307d 100644 --- a/semantic-conventions/setup.py +++ b/semantic-conventions/setup.py @@ -8,7 +8,7 @@ ) PACKAGE_INFO = {} with open(VERSION_FILENAME, encoding="utf-8") as f: - exec(f.read(), PACKAGE_INFO) + exec(f.read(), PACKAGE_INFO) # pylint:disable=exec-used VERSION_SUFFIX = os.environ.get("SEMCONGEN_VERSION_SUFFIX") From a1bd14fb9f57dbe68c6feeed448c8261f86c3e4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Tue, 7 Sep 2021 18:17:39 +0200 Subject: [PATCH 06/10] Run & fix mypy. --- .github/workflows/semconvgen.yml | 2 ++ semantic-conventions/dev-requirements.txt | 2 +- semantic-conventions/mypy.ini | 7 +++++++ .../src/opentelemetry/semconv/model/constraints.py | 4 ++-- .../src/opentelemetry/semconv/model/semantic_attribute.py | 7 ++++--- .../src/opentelemetry/semconv/model/semantic_convention.py | 6 ++++-- .../src/opentelemetry/semconv/model/utils.py | 5 +++-- .../src/opentelemetry/semconv/templating/code.py | 4 +++- .../opentelemetry/semconv/templating/markdown/__init__.py | 3 +-- 9 files changed, 27 insertions(+), 13 deletions(-) create mode 100644 semantic-conventions/mypy.ini diff --git a/.github/workflows/semconvgen.yml b/.github/workflows/semconvgen.yml index fe8170ac..9350db85 100644 --- a/.github/workflows/semconvgen.yml +++ b/.github/workflows/semconvgen.yml @@ -33,6 +33,8 @@ jobs: run: pytest -v - name: Slow linting (pylint) run: pylint *.py src/ + - name: Type checking (mypy) + run: mypy src/ build-and-publish-docker: diff --git a/semantic-conventions/dev-requirements.txt b/semantic-conventions/dev-requirements.txt index 59bb43a8..f651b0b1 100644 --- a/semantic-conventions/dev-requirements.txt +++ b/semantic-conventions/dev-requirements.txt @@ -1,5 +1,5 @@ black==21.6b0 -mypy==0.770 pytest==6.1.1 +mypy==0.910 flake8==3.9.2 pylint==2.10.2 diff --git a/semantic-conventions/mypy.ini b/semantic-conventions/mypy.ini new file mode 100644 index 00000000..9ecdc1d5 --- /dev/null +++ b/semantic-conventions/mypy.ini @@ -0,0 +1,7 @@ +[mypy] + +[mypy-jinja2.*] +ignore_missing_imports = True + +[mypy-mistune.*] +ignore_missing_imports = True diff --git a/semantic-conventions/src/opentelemetry/semconv/model/constraints.py b/semantic-conventions/src/opentelemetry/semconv/model/constraints.py index cf58704c..651fcde3 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/constraints.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/constraints.py @@ -39,9 +39,9 @@ class AnyOf: _yaml_src_position Contains the position in the YAML file of the AnyOf attribute """ - choice_list_ids: Tuple[Tuple[str, ...]] + choice_list_ids: Tuple[Tuple[str, ...], ...] inherited: bool = False - choice_list_attributes: Tuple[Tuple[SemanticAttribute, ...]] = () + choice_list_attributes: Tuple[Tuple[SemanticAttribute, ...], ...] = () _yaml_src_position: int = 0 def __eq__(self, other): diff --git a/semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py b/semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py index d977cb5f..c8ee0420 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/semantic_attribute.py @@ -16,7 +16,7 @@ from collections.abc import Iterable from dataclasses import dataclass, replace from enum import Enum -from typing import List, Union, Dict +from typing import List, Optional, Union, Dict from ruamel.yaml.comments import CommentedMap, CommentedSeq @@ -88,7 +88,7 @@ def parse( """This method parses the yaml representation for semantic attributes creating the respective SemanticAttribute objects. """ - attributes = {} + attributes = {} # type: Dict[str, SemanticAttribute] allowed_keys = ( "id", "type", @@ -136,6 +136,7 @@ def parse( } required_msg = "" required_val = attribute.get("required", "") + required: Optional[Required] if isinstance(required_val, CommentedMap): required = Required.CONDITIONAL required_msg = required_val.get("conditional", None) @@ -204,7 +205,7 @@ def parse( "Attribute id " + fqn + " is already present at line " - + str(attributes.get(fqn).position[0] + 1) + + str(attributes[fqn].position[0] + 1) ) raise ValidationError.from_yaml_pos(position, msg) attributes[fqn] = attr diff --git a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py index 9962928b..23503a0a 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py @@ -99,6 +99,8 @@ def SemanticConvention(group): class BaseSemanticConvention(ValidatableYamlNode): """Contains the model extracted from a yaml file""" + GROUP_TYPE_NAME: str + @property def attributes(self): if not hasattr(self, "attrs_by_name"): @@ -541,11 +543,11 @@ def attributes(self): CONVENTION_CLS_BY_GROUP_TYPE = { cls.GROUP_TYPE_NAME: cls - for cls in [ + for cls in ( SpanSemanticConvention, ResourceSemanticConvention, EventSemanticConvention, MetricSemanticConvention, UnitSemanticConvention, - ] + ) } diff --git a/semantic-conventions/src/opentelemetry/semconv/model/utils.py b/semantic-conventions/src/opentelemetry/semconv/model/utils.py index 7ce5961a..28f230e2 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/utils.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/utils.py @@ -13,6 +13,7 @@ # limitations under the License. import re +from typing import Tuple from opentelemetry.semconv.model.exceptions import ValidationError @@ -51,8 +52,8 @@ def check_no_missing_keys(yaml, mandatory): class ValidatableYamlNode: - allowed_keys = () - mandatory_keys = ("id", "brief") + allowed_keys = () # type: Tuple[str, ...] + mandatory_keys = ("id", "brief") # type: Tuple[str, ...] def __init__(self, yaml_node): self.id = yaml_node.get("id").strip() diff --git a/semantic-conventions/src/opentelemetry/semconv/templating/code.py b/semantic-conventions/src/opentelemetry/semconv/templating/code.py index 303e9a73..3d666b7e 100644 --- a/semantic-conventions/src/opentelemetry/semconv/templating/code.py +++ b/semantic-conventions/src/opentelemetry/semconv/templating/code.py @@ -188,7 +188,9 @@ def get_data_single_file( data.update(self.parameters) return data - def get_data_multiple_files(self, semconv, template_path) -> dict: + def get_data_multiple_files( + self, semconv, template_path + ) -> typing.Dict[str, typing.Any]: """Returns a dictionary with the data from a single SemanticConvention to fill the template.""" data = {"template": template_path, "semconv": semconv} data.update(self.parameters) diff --git a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py index ec9d62d0..723da8b5 100644 --- a/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py +++ b/semantic-conventions/src/opentelemetry/semconv/templating/markdown/__init__.py @@ -218,8 +218,7 @@ def to_markdown_enum(self, output: io.StringIO): """ attr: SemanticAttribute for attr in self.render_ctx.enums: - enum: EnumAttributeType - enum = attr.attr_type + enum = typing.cast(EnumAttributeType, attr.attr_type) output.write("\n`" + attr.fqn + "` ") if enum.custom_values: output.write( From 2130f9039be666750c28a1770f88bb591c5bbbba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Tue, 7 Sep 2021 18:24:19 +0200 Subject: [PATCH 07/10] upgrade black, pytest --- semantic-conventions/dev-requirements.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/semantic-conventions/dev-requirements.txt b/semantic-conventions/dev-requirements.txt index f651b0b1..e6b8a863 100644 --- a/semantic-conventions/dev-requirements.txt +++ b/semantic-conventions/dev-requirements.txt @@ -1,5 +1,5 @@ -black==21.6b0 -pytest==6.1.1 +black==21.8b0 mypy==0.910 +pytest==6.2.5 flake8==3.9.2 pylint==2.10.2 From 3c55a7e13a85672d7cf3b8bcf2a025591da3e686 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Wed, 8 Sep 2021 09:03:46 +0200 Subject: [PATCH 08/10] Update semantic-conventions/src/opentelemetry/semconv/templating/code.py --- .../src/opentelemetry/semconv/templating/code.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/semantic-conventions/src/opentelemetry/semconv/templating/code.py b/semantic-conventions/src/opentelemetry/semconv/templating/code.py index 3d666b7e..7b95b0d0 100644 --- a/semantic-conventions/src/opentelemetry/semconv/templating/code.py +++ b/semantic-conventions/src/opentelemetry/semconv/templating/code.py @@ -46,7 +46,7 @@ class CustomRender(mistune.HTMLRenderer): def link(self, url, text=None, title=None): # pylint:disable=arguments-renamed if link: return link.format(url, text, title) - return super().link(url, text, title) if html else link + return super().link(url, text, title) if html else url def image(self, src, alt="", title=None): if image: From 38e978ab4dbb82774c8033edea9c1aa169178212 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Wed, 8 Sep 2021 10:08:33 +0200 Subject: [PATCH 09/10] Fix name clash. --- .../src/opentelemetry/semconv/templating/code.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/semantic-conventions/src/opentelemetry/semconv/templating/code.py b/semantic-conventions/src/opentelemetry/semconv/templating/code.py index 7b95b0d0..24d982dc 100644 --- a/semantic-conventions/src/opentelemetry/semconv/templating/code.py +++ b/semantic-conventions/src/opentelemetry/semconv/templating/code.py @@ -63,10 +63,10 @@ def strong(self, text): return strong.format(text) return super().strong(text) if html else text - def inline_html(self, html): + def inline_html(self, html_text): # pylint:disable=arguments-renamed if inline_html: - return inline_html.format(html) - return super().inline_html(html) if html else html + return inline_html.format(html_text) + return super().inline_html(html_text) if html else html_text def paragraph(self, text): if paragraph: From a06432a3508c13cef2c7b7b04fa4243c132d9f10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Fri, 10 Sep 2021 09:49:34 +0200 Subject: [PATCH 10/10] Remove left-over debugging print statement. --- .../src/opentelemetry/semconv/model/semantic_convention.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py index 23503a0a..dc51ee60 100644 --- a/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py +++ b/semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py @@ -218,9 +218,6 @@ def __init__(self, group): raise ValidationError.from_yaml_pos(position, msg) -print("MRO: ", SpanSemanticConvention.__mro__) - - class EventSemanticConvention(BaseSemanticConvention): GROUP_TYPE_NAME = "event"