Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds required stability property to enum members #267

Merged
merged 11 commits into from
Mar 6, 2024
2 changes: 2 additions & 0 deletions semantic-conventions/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ Please update the changelog as part of any significant pull request.

## Unreleased

- BREAKING: Add `stability` (required) and `deprecated` (optional) properties to `EnumMember`
([#267](https:/open-telemetry/build-tools/pull/267))
- Change minimum support python version to 3.10 in setup.cfg and Dockerfile
([#285](https:/open-telemetry/build-tools/pull/285))
- BREAKING: Make stability required (also: fix ref and extends, render badges on metrics).
Expand Down
9 changes: 7 additions & 2 deletions semantic-conventions/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -133,13 +133,18 @@ The `{semconv version}` (e.g. `1.24.0`) is the previously released version of se
Following checks are performed

- On all attributes and metrics (experimental and stable):
- attributes and metrics must not be removed.
- attributes and metrics must not be removed
- enum attribute members 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 enum attribute members:
- stability must not be changed
- `id` and `value` must not be changed

- On stable metrics:
- stability must not be changed
- instrument and unit must not be changed
Expand Down
14 changes: 14 additions & 0 deletions semantic-conventions/semconv.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,9 @@
"note": {
"type": "string",
"description": "longer description. It defaults to an empty string."
},
"stability": {
"allOf": [{ "$ref": "#/definitions/StabilityLevel" }]
}
}
}
Expand Down Expand Up @@ -343,6 +346,14 @@
}
]
},
"StabilityLevel": {
"description": "specifies the stability level. Can be 'stable' or 'experimental' (the default).",
"type": "string",
"enum": [
"stable",
"experimental"
]
},
"Attribute": {
"type": "object",
"allOf": [
Expand Down Expand Up @@ -416,6 +427,9 @@
"deprecated": {
"type": "string",
"description": "specifies if the attribute is deprecated. The string provided as <description> MUST specify why it's deprecated and/or what to use instead."
},
"stability": {
"allOf": [{ "$ref": "#/definitions/StabilityLevel" }]
}
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,17 @@ def parse(
stability = SemanticAttribute.parse_stability(
attribute.get("stability"), position_data, strict_validation
)
if stability == StabilityLevel.EXPERIMENTAL and isinstance(
attr_type, EnumAttributeType
):
for member in attr_type.members:
if member.stability == StabilityLevel.STABLE:
msg = (
f"Member '{member.member_id}' is marked as stable "
+ "but it is not allowed on experimental attribute!"
)
raise ValidationError.from_yaml_pos(position_data["type"], msg)

deprecated = SemanticAttribute.parse_deprecated(
attribute.get("deprecated"), position_data
)
Expand Down Expand Up @@ -482,7 +493,7 @@ def parse(attribute_type):
attribute_type.lc.data["members"], "Enumeration without members!"
)

allowed_keys = ["id", "value", "brief", "note"]
allowed_keys = ["id", "value", "brief", "note", "stability", "deprecated"]
mandatory_keys = ["id", "value"]
for member in attribute_type["members"]:
validate_values(member, allowed_keys, mandatory_keys)
Expand All @@ -492,12 +503,26 @@ def parse(attribute_type):
f"Invalid value used in enum: <{member['value']}>",
)
validate_id(member["id"], member.lc.data["id"])

stability_str = member.get("stability")
if not stability_str:
raise ValidationError.from_yaml_pos(
member.lc.data["id"],
f"Enumeration member '{member['value']}' must have a stability level",
)

stability = SemanticAttribute.parse_stability(stability_str, member.lc.data)
deprecated = SemanticAttribute.parse_deprecated(
member.get("deprecated"), member.lc.data
)
members.append(
EnumMember(
member_id=member["id"],
value=member["value"],
brief=member.get("brief", member["id"]).strip(),
note=member.get("note", "").strip(),
stability=stability,
deprecated=deprecated,
)
)
enum_type = AttributeType.get_type(members[0].value)
Expand All @@ -516,6 +541,8 @@ class EnumMember:
value: str
brief: str
note: str
stability: StabilityLevel
deprecated: str


class MdLink:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,49 +67,66 @@ def _check_attribute(self, prev: SemanticAttribute, problems: list[Problem]):
problems.append(Problem("attribute", prev.fqn, "was removed"))
return

self._check_stability(
prev.stability, cur.stability, "attribute", prev.fqn, problems
)
if prev.stability == StabilityLevel.STABLE:
if cur.stability != prev.stability:
self._check_attribute_type(prev, cur, problems)

if (
isinstance(prev.attr_type, EnumAttributeType)
and
# this makes mypy happy, we already checked that type is the same for stable attributes
isinstance(cur.attr_type, EnumAttributeType)
):
for member in prev.attr_type.members:
self._check_member(prev.fqn, member, cur.attr_type.members, problems)

def _check_stability(
self,
prev: StabilityLevel,
cur: StabilityLevel,
signal: str,
fqn: str,
problems: list[Problem],
):
if prev == StabilityLevel.STABLE and cur != prev:
problems.append(
Problem(signal, fqn, f"stability changed from '{prev}' to '{cur}'")
)

def _check_attribute_type(
self, prev: SemanticAttribute, cur: SemanticAttribute, problems: list[Problem]
):
if isinstance(prev.attr_type, EnumAttributeType):
if not isinstance(cur.attr_type, EnumAttributeType):
problems.append(
Problem(
"attribute",
prev.fqn,
f"stability changed from '{prev.stability}' to '{cur.stability}'",
f"type changed from '{prev.attr_type}' to '{cur.attr_type}'",
)
)

if isinstance(prev.attr_type, EnumAttributeType):
if not isinstance(cur.attr_type, EnumAttributeType):
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(
"attribute",
prev.fqn,
f"type changed from '{prev.attr_type}' to '{cur.attr_type}'",
f"enum type changed from '{prev.attr_type.enum_type}' to '{cur.attr_type.enum_type}'",
)
)
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(
"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}'",
)
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,
Expand All @@ -118,8 +135,22 @@ def _check_member(
members: list[EnumMember],
problems: list[Problem],
):
found = False
for member in members:
if prev.member_id == member.member_id:
found = True
if prev.stability != StabilityLevel.STABLE:
# we allow stability and value changes for non-stable members
break

self._check_stability(
prev.stability,
member.stability,
"enum attribute member",
f"{fqn}.{prev.member_id}",
problems,
)

if prev.value != member.value:
member_value = (
f'"{member.value}"'
Expand All @@ -133,10 +164,12 @@ def _check_member(
f"value changed from '{prev.value}' to '{member_value}'",
)
)
return
problems.append(
Problem("enum attribute member", f"{fqn}.{prev.member_id}", "was removed")
)
if not found:
problems.append(
Problem(
"enum attribute member", f"{fqn}.{prev.member_id}", "was removed"
)
)

def _check_metric(self, prev: MetricSemanticConvention, problems: list[Problem]):
for cur in self.current_semconv.models.values():
Expand All @@ -145,14 +178,13 @@ def _check_metric(self, prev: MetricSemanticConvention, problems: list[Problem])
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}'",
)
)
self._check_stability(
prev.stability,
cur.stability,
"metric",
prev.metric_name,
problems,
)
if cur.unit != prev.unit:
problems.append(
Problem(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,10 @@ def to_markdown_attr(
if isinstance(attribute.attr_type, EnumAttributeType)
else AttributeType.get_instantiated_type(attribute.attr_type)
)
description = self._description_with_badge(attribute) + attribute.brief
description = (
self._description_with_badge(attribute.stability, attribute.deprecated)
+ attribute.brief
)
if attribute.note:
self.render_ctx.add_note(attribute.note)
description += f" [{len(self.render_ctx.notes)}]"
Expand Down Expand Up @@ -242,7 +245,10 @@ def to_markdown_metric_table(
"| -------- | --------------- | ----------- | -------------- |\n"
)

description = self._description_with_badge(semconv) + semconv.brief
description = (
self._description_with_badge(semconv.stability, semconv.deprecated)
+ semconv.brief
)
if semconv.note:
self.render_ctx.add_note(semconv.note)
description += f" [{len(self.render_ctx.notes)}]"
Expand Down Expand Up @@ -327,7 +333,10 @@ def to_markdown_enum(self, output: io.StringIO):
counter = 1
notes = []
for member in enum.members:
description = member.brief
description = (
self._description_with_badge(member.stability, member.deprecated)
+ member.brief
)
if member.note:
description += f" [{counter}]"
counter += 1
Expand Down Expand Up @@ -528,22 +537,18 @@ def _render_group(self, semconv, parameters, output):

output.write("<!-- endsemconv -->")

def _description_with_badge(
self, item: typing.Union[SemanticAttribute | BaseSemanticConvention]
):
def _description_with_badge(self, stability: StabilityLevel, deprecated: str):
description = ""
if item.deprecated and self.options.enable_deprecated:
if "deprecated" in item.deprecated.lower():
description = f"**{item.deprecated}**<br>"
if deprecated and self.options.enable_deprecated:
if "deprecated" in deprecated.lower():
description = f"**{deprecated}**<br>"
else:
deprecated_msg = self.options.deprecated_md_snippet().format(
item.deprecated
)
deprecated_msg = self.options.deprecated_md_snippet().format(deprecated)
description = f"{deprecated_msg}<br>"
elif item.stability == StabilityLevel.STABLE and self.options.enable_stable:
elif stability == StabilityLevel.STABLE and self.options.enable_stable:
description = f"{self.options.stable_md_snippet()}<br>"
elif (
item.stability == StabilityLevel.EXPERIMENTAL
stability == StabilityLevel.EXPERIMENTAL
and self.options.enable_experimental
):
description = f"{self.options.experimental_md_snippet()}<br>"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ groups:
members:
- id: enum_two
brief: "enum two"
stability: experimental
value: "two"
brief: "third attribute"
note: "third attribute note"
examples: ["two"]
stability: stable
stability: experimental
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ groups:
members:
- id: enum_one
brief: "enum one"
stability: experimental
value: "one"
brief: "third attribute"
note: "third attribute note"
examples: ["one"]
stability: stable
stability: experimental
Loading
Loading