add stability check for enum members

semantic-conventions/README.md

@@ -130,13 +130,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

semantic-conventions/src/opentelemetry/semconv/templating/compatibility.py

@@ -67,49 +67,61 @@ 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):
+            for member in prev.attr_type.members:
+                self._check_member(prev.fqn, member, cur.attr_type.members, problems)
+
+    def _check_stability(
+        self,
+        prev: SemanticAttribute,
+        cur: SemanticAttribute,
+        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: EnumAttributeType, cur: EnumAttributeType, 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,
@@ -118,8 +130,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}"'
@@ -133,10 +159,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():
@@ -145,14 +173,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(

semantic-conventions/src/tests/data/compat/enum_member_removed/vnext.yaml

@@ -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

semantic-conventions/src/tests/data/compat/enum_member_removed/vprev.yaml

@@ -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

semantic-conventions/src/tests/data/compat/enum_type_changed/vnext.yaml

@@ -10,8 +10,20 @@ groups:
           members:
             - id: enum_one
               brief: "enum one"
+              stability: stable
               value: 1
         brief: "third attribute"
         note: "third attribute note"
-        examples: [1]
+        examples: [3]
+        stability: stable
+      - id: forth_attr
+        type:
+          members:
+            - id: enum_two
+              brief: "enum two"
+              stability: experimental
+              value: 2
+        brief: "forth attribute"
+        note: "forth attribute note"
+        examples: [2]
         stability: stable

semantic-conventions/src/tests/data/compat/enum_type_changed/vprev.yaml

@@ -10,8 +10,20 @@ groups:
           members:
             - id: enum_one
               brief: "enum one"
+              stability: stable
               value: "one"
         brief: "third attribute"
         note: "third attribute note"
-        examples: ["one"]
+        examples: ["three"]
+        stability: stable
+      - id: forth_attr
+        type:
+          members:
+            - id: enum_two
+              brief: "enum two"
+              stability: experimental
+              value: "one"
+        brief: "forth attribute"
+        note: "forth attribute note"
+        examples: ["four"]
         stability: stable

semantic-conventions/src/tests/data/compat/enum_value_changed/vnext.yaml

@@ -10,7 +10,12 @@ groups:
           members:
             - id: enum_one
               brief: "enum one"
+              stability: stable
               value: "1"
+            - id: enum_two
+              brief: "enum two"
+              stability: experimental
+              value: "_two_"
         brief: "third attribute"
         note: "third attribute note"
         examples: ["one"]

semantic-conventions/src/tests/data/compat/enum_value_changed/vprev.yaml

@@ -10,7 +10,12 @@ groups:
           members:
             - id: enum_one
               brief: "enum one"
+              stability: stable
               value: "one"
+            - id: enum_two
+              brief: "enum two"
+              stability: experimental
+              value: "two"
         brief: "third attribute"
         note: "third attribute note"
         examples: ["one"]

semantic-conventions/src/tests/data/compat/success/vnext.yaml

@@ -22,9 +22,11 @@ groups:
           members:
             - id: enum_one
               brief: "Enum one."
-              value: "one"
+              stability: stable
+              value: "_one_"
             - id: enum_two
               brief: "Enum two."
+              stability: stable
               value: "two"
         brief: "Third attribute."
         note: "Third attribute note."

semantic-conventions/src/tests/data/compat/success/vprev.yaml

@@ -21,9 +21,11 @@ groups:
           members:
             - id: enum_one
               brief: "enum one"
+              stability: experimental
               value: "one"
             - id: enum_two
               brief: "enum two"
+              stability: stable
               value: "two"
         brief: "third attribute"
         note: "third attribute note"

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

@@ -130,7 +130,7 @@ def testTypeChanged(self):
         ]
         self.assert_errors(expected_errors, problems)
 
-    def testEnumTypeChanged(self):
+    def testStableEnumTypeChanged(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)
@@ -146,10 +146,15 @@ def testEnumTypeChanged(self):
                 "first.third_attr.enum_one",
                 "value changed from 'one' to '1'",
             ),
+            Problem(
+                "attribute",
+                "first.forth_attr",
+                "enum type changed from 'string' to 'int'",
+            ),
         ]
         self.assert_errors(expected_errors, problems)
 
-    def testEnumValueChanged(self):
+    def testStableEnumValueChanged(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)