Add attribute name and const names checks (#1209)

.github/workflows/checks.yml

@@ -108,3 +108,10 @@ jobs:
       run: |
         make generate-gh-issue-templates
         git diff --exit-code '.github/ISSUE_TEMPLATE' || (echo 'Dropdowns in issue templates is out of date, please run "make generate-gh-issue-templates" and commit the changes in this PR. Please note, if you are running it on Mac OS X, please make sure to use GNU version of sed instead of default "sed".' && exit 1)
+
+  policies-check:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v1
+    - name: verify semantic conventions yaml definitions
+      run: make check-policies

Makefile

@@ -228,3 +228,11 @@ chlog-update: $(CHLOGGEN)
 .PHONY: generate-gh-issue-templates
 generate-gh-issue-templates:
 	$(TOOLS_DIR)/scripts/update-issue-template-areas.sh
+
+.PHONY: check-policies
+check-policies:
+	docker run --rm -v $(PWD)/model:/source -v $(PWD)/docs:/spec -v $(PWD)/policies:/policies \
+		otel/weaver:${WEAVER_VERSION} registry check \
+		--registry=/source \
+		--policy=/policies/registry.rego \
+		--policy=/policies/attribute_name_collisions.rego

policies/attribute_name_collisions.rego

@@ -0,0 +1,56 @@
+package after_resolution
+
+deny[attr_registry_collision(description, name)] {
+    names := attr_names_except(excluded_const_collisions)
+    name := names[_]
+    const_name := to_const_name(name)
+    collisions:= { n | n := attr_names_except(excluded_const_collisions)[_]; n != name; to_const_name(n) == const_name }
+    count(collisions) > 0
+
+    # TODO (https://github.com/open-telemetry/weaver/issues/279): provide other violation properties once weaver supports it.
+    description := sprintf("Attribute '%s' has the same constant name '%s' as '%s'.", [name, const_name, collisions])
+}
+
+deny[attr_registry_collision(description, name)] {
+    names := attr_names_except(excluded_namespace_collisions)
+    name := names[_]
+
+    collisions:= { n | n := input.groups[_].attributes[_].name; startswith(n, to_namespace_prefix(name)) }
+    count(collisions) > 0
+
+    # TODO (https://github.com/open-telemetry/weaver/issues/279): provide other violation properties once weaver supports it.
+    description := sprintf("Attribute '%s' name is used as a namespace in the following attributes '%s'.", [name, collisions])
+}
+
+attr_registry_collision(description, attr_name) = violation {
+    violation := {
+        "id": description,
+        "type": "semconv_attribute",
+        "category": "naming_collision",
+        "attr": attr_name,
+        "group": "",
+    }
+}
+
+to_namespace_prefix(name) = namespace {
+    namespace := concat("", [name, "."])
+}
+
+to_const_name(name) = const_name {
+    const_name := replace(name, ".", "_")
+}
+
+attr_names_except(excluded) = names {
+    names := { n | n := input.groups[_].attributes[_].name } - excluded
+}
+
+# These lists contain exceptions for existing collisions that were introduced unintentionally.
+# We'll have a way to specify how collision resolution happens in the schema -
+# see phase 2 in https://github.com/open-telemetry/semantic-conventions/issues/1118#issuecomment-2173803006
+# For now we'll exclude existing collisions from the checks.
+# ADDING NEW EXCEPTIONS IS NOT ALLOWED.
+
+# DO NOT ADD ATTRIBUTES TO THIS LIST
+excluded_const_collisions := {"messaging.client_id"}
+# DO NOT ADD ATTRIBUTES TO THIS LIST
+excluded_namespace_collisions := {"messaging.operation", "db.operation", "deployment.environment"}

policies/registry.rego

@@ -5,38 +5,59 @@ package before_resolution
 # used by semantic conventions.
 
 # Helper to create attribute registry violations.
-attr_registry_violation(violation_id, group_id, attr_id) = violation {
+attr_registry_violation(description, group_id, attr_id) = violation {
     violation := {
-        "id": violation_id,
-        "type": "semantic_convention_policies",
+        "id": description,
+        "type": "semconv_attribute",
         "category": "attribute_registry_checks",
         "group": group_id,
         "attr": attr_id,
     }
 }
 
 # We only allow attribute groups in the attribute registry.
-deny[attr_registry_violation("attribute_registry_can_only_contain_attribute_groups", group.id, "")] {
+deny[attr_registry_violation(description, group.id, "")] {
     group := input.groups[_]
     startswith(group.id, "registry.")
     group.type != "attribute_group"
+
+    # TODO (https://github.com/open-telemetry/weaver/issues/279): provide other violation properties once weaver supports it.
+    # violation_id := "attribute_registry_can_only_contain_attribute_groups"
+    description := sprintf("Registry group '%s' has invalid type '%s'. Groups in attribute registry must have `attribute_group` type.", [group.id, group.type])
 }
 
 # Any group that is NOT in the attribute registry that has an attribute id is
 # in violation of not using the attribute registry.
-deny[attr_registry_violation("attributes_must_be_defined_in_attribute_registry", group.id, attr.id)] {
+deny[attr_registry_violation(description, group.id, attr.id)] {
     group := input.groups[_]
     not startswith(group.id, "registry.")
     attr := group.attributes[_]
     attr.id != null
+
+    attr_name := get_attribute_name(attr, group)
+
+    # TODO (https://github.com/open-telemetry/weaver/issues/279): provide other violation properties once weaver supports it.
+    # violation_id := "attributes_must_be_defined_in_attribute_registry"
+    description := sprintf("Attribute '%s' is defined in the group '%s' which is not part of the attribute registy. Attributes can be defined in the registry group only.", [attr_name, group.id])
 }
 
 # A registry `attribute_group` containing at least one `ref` attribute is
 # considered invalid if it's not in the registry group.
-deny[attr_registry_violation("attributes_in_registry_cannot_reference_each_other", group.id, attr.ref)] {
+deny[attr_registry_violation(description, group.id, attr.ref)] {
     # TODO - this will need to be updated to support `embed` in the future.
     group := input.groups[_]
     startswith(group.id, "registry.")
     attr := group.attributes[_]
     attr.ref != null
+
+    # TODO (https://github.com/open-telemetry/weaver/issues/279): provide other violation properties once weaver supports it.
+    # violation_id := "attributes_in_registry_cannot_reference_each_other"
+    description := sprintf("Registy group '%s' references attribute '%s'. Registry groups can only define new attributes.", [group.id, attr.ref])
+}
+
+get_attribute_name(attr, group) = name {
+    full_name = concat(".", [group.prefix, attr.id])
+
+    # if there was no prefix, we have a leading dot
+    name := trim(full_name, ".")
 }