diff --git a/.chloggen/1302.yaml b/.chloggen/1302.yaml new file mode 100644 index 0000000000..4f2cc12296 --- /dev/null +++ b/.chloggen/1302.yaml @@ -0,0 +1,4 @@ +change_type: enhancement +component: docs +note: Add note on tooling limitations for attribute names and enforce name format. +issues: [1124] diff --git a/Makefile b/Makefile index 574758d50a..2a783c5b19 100644 --- a/Makefile +++ b/Makefile @@ -226,6 +226,7 @@ check-policies: test-policies: docker run --rm -v $(PWD)/policies:/policies -v $(PWD)/policies_test:/policies_test \ ${OPA_CONTAINER} test \ + --var-values \ --explain fails \ /policies \ /policies_test diff --git a/docs/general/attribute-naming.md b/docs/general/attribute-naming.md index 3b3445cb46..cded4a49ca 100644 --- a/docs/general/attribute-naming.md +++ b/docs/general/attribute-naming.md @@ -116,6 +116,12 @@ denote old attribute names in rename operations). the following Unicode code points: Latin alphabet, Numeric, Underscore, Dot (as namespace delimiter). +> Note: +> Semantic Conventions tooling limits names to lowercase +> Latin alphabet, Numeric, Underscore, Dot (as namespace delimiter). +> Names must start with a letter, end with an alphanumeric character, and must not +> contain two or more consecutive delimiters (Underscore or Dot). + ## Recommendations for Application Developers As an application developer when you need to record an attribute first consult diff --git a/policies/attribute_name_collisions.rego b/policies/attribute_name_collisions.rego index 7e7cbad1a5..c52f0267fc 100644 --- a/policies/attribute_name_collisions.rego +++ b/policies/attribute_name_collisions.rego @@ -4,12 +4,12 @@ import rego.v1 # Data structures to make checking things faster. attribute_names := { obj | - group := input.groups[_]; - attr := group.attributes[_]; - obj := { "name": attr.name, "const_name": to_const_name(attr.name), "namespace_prefix": to_namespace_prefix(attr.name) } + group := input.groups[_]; + attr := group.attributes[_]; + obj := { "name": attr.name, "const_name": to_const_name(attr.name), "namespace_prefix": to_namespace_prefix(attr.name) } } - +# check that attribute constant names do not collide deny contains attr_registry_collision(description, name) if { some i name := attribute_names[i].name @@ -26,6 +26,7 @@ deny contains attr_registry_collision(description, name) if { description := sprintf("Attribute '%s' has the same constant name '%s' as '%s'.", [name, const_name, collisions]) } +# check that attribute names do not collide with namespaces deny contains attr_registry_collision(description, name) if { some i name := attribute_names[i].name @@ -38,7 +39,19 @@ deny contains attr_registry_collision(description, name) if { ] count(collisions) > 0 # TODO (https://github.com/open-telemetry/weaver/issues/279): provide other violation properties once weaver supports it. - description := sprintf("Attribute '%s' is used as a namespace in '%s'.", [name, collisions]) + description := sprintf("Attribute '%s' name is used as a namespace in the following attributes '%s'.", [name, collisions]) +} + +# check that attribute is not defined or referenced more than once within the same group +deny contains attr_registry_collision(description, name) if { + group := input.groups[_] + attr := group.attributes[_] + name := attr.name + + collisions := [n | n := group.attributes[_].name; n == name ] + count(collisions) > 1 + + description := sprintf("Attribute '%s' is already defined in the group '%s'. Attributes must be unique.", [name, group.id]) } attr_registry_collision(description, attr_name) = violation if { diff --git a/policies/attribute_name_collisions_test.rego b/policies/attribute_name_collisions_test.rego new file mode 100644 index 0000000000..b86e79fece --- /dev/null +++ b/policies/attribute_name_collisions_test.rego @@ -0,0 +1,19 @@ +package after_resolution +import future.keywords + +test_fails_on_const_name_collision if { + collision := {"groups": [ + {"id": "test1", "attributes": [{"name": "foo.bar.baz"}]}, + {"id": "test2", "attributes": [{"name": "foo.bar_baz"}]} + ]} + # each attribute counts as a collision, so there are 2 collisions + count(deny) == 2 with input as collision +} + +test_fails_on_namespace_collision if { + collision := {"groups": [ + {"id": "test1", "attributes": [{"name": "foo.bar.baz"}]}, + {"id": "test2", "attributes": [{"name": "foo.bar"}]} + ]} + count(deny) == 1 with input as collision +} diff --git a/policies/yaml_schema.rego b/policies/yaml_schema.rego index 0d8cad7dd3..2dfe53d593 100644 --- a/policies/yaml_schema.rego +++ b/policies/yaml_schema.rego @@ -1,15 +1,52 @@ package before_resolution -yaml_schema_violation(description, group, attr) = violation { - violation := { - "id": description, - "type": "semconv_attribute", - "category": "yaml_schema_violation", - "attr": attr, - "group": group, - } +# checks attribute name format +deny[yaml_schema_violation(description, group.id, name)] { + group := input.groups[_] + attr := group.attributes[_] + name := attr.id + + not regex.match(name_regex, name) + + description := sprintf("Attribute name '%s' is invalid. Attribute name %s", [name, invalid_name_helper]) +} + +# checks metric name format +deny[yaml_schema_violation(description, group.id, name)] { + group := input.groups[_] + name := group.metric_name + + name != null + not regex.match(name_regex, name) + + description := sprintf("Metric name '%s' is invalid. Metric name %s'", [name, invalid_name_helper]) +} + +# checks event name format +deny[yaml_schema_violation(description, group.id, name)] { + group := input.groups[_] + group.type == "event" + name := group.name + + name != null + not regex.match(name_regex, name) + + description := sprintf("Event name '%s' is invalid. Event name %s'", [name, invalid_name_helper]) +} + +# checks attribute member id format +deny[yaml_schema_violation(description, group.id, attr_name)] { + group := input.groups[_] + attr := group.attributes[_] + attr_name := attr.id + name := attr.type.members[_].id + + not regex.match(name_regex, name) + + description := sprintf("Member id '%s' on attribute '%s' is invalid. Member id %s'", [name, attr_name, invalid_name_helper]) } +# check that attribute is fully qualified with their id, prefix is no longer supported deny[yaml_schema_violation(description, group.id, "")] { group := input.groups[_] @@ -19,3 +56,19 @@ deny[yaml_schema_violation(description, group.id, "")] { # TODO (https://github.com/open-telemetry/weaver/issues/279): provide other violation properties once weaver supports it. description := sprintf("Group '%s' uses prefix '%s'. All attribute should be fully qualified with their id, prefix is no longer supported.", [group.id, group.prefix]) } + +yaml_schema_violation(description, group, attr) = violation { + violation := { + "id": description, + "type": "semconv_attribute", + "category": "yaml_schema_violation", + "attr": attr, + "group": group, + } +} + +# not valid: '1foo.bar', 'foo.bar.', 'foo.bar_', 'foo..bar', 'foo._bar' ... +# valid: 'foo.bar', 'foo.1bar', 'foo.1_bar' +name_regex := "^[a-z][a-z0-9]*([._][a-z0-9]+)*$" + +invalid_name_helper := "must consist of lowercase alphanumeric characters separated by '_' and '.'" diff --git a/policies/yaml_schema_test.rego b/policies/yaml_schema_test.rego new file mode 100644 index 0000000000..ebcd86f036 --- /dev/null +++ b/policies/yaml_schema_test.rego @@ -0,0 +1,67 @@ +package before_resolution + +import future.keywords + +test_fails_on_invalid_attribute_name if { + every name in invalid_names { + count(deny) == 1 with input as {"groups": create_attribute_group(name)} + } +} + +test_fails_on_invalid_metric_name if { + every name in invalid_names { + count(deny) == 1 with input as {"groups": create_metric(name)} + } +} + +test_fails_on_invalid_event_name if { + every name in invalid_names { + count(deny) == 1 with input as {"groups": create_event(name)} + } +} + +test_passes_on_valid_names if { + every name in valid_names { + count(deny) == 0 with input as {"groups": create_attribute_group(name)} + count(deny) == 0 with input as {"groups": create_metric(name)} + count(deny) == 0 with input as {"groups": create_event(name)} + } +} + +test_fails_if_prefix_is_present if { + count(deny) == 1 with input as {"groups": [{"id": "test", "prefix": "foo"}]} +} + +create_attribute_group(attr) = json { + json := [{"id": "yaml_schema.test", "attributes": [{"id": attr}]}] +} + +create_metric(name) = json { + json := [{"id": "yaml_schema.test", "type": "metric", "metric_name": name}] +} + +create_event(name) = json { + json := [{"id": "yaml_schema.test", "type": "event", "name": name}] +} + +invalid_names := [ + "1foo.bar", + "_foo.bar", + ".foo.bar", + "foo.bar_", + "foo.bar.", + "foo..bar", + "foo._bar", + "foo__bar", + "foo_.bar", + "foo,bar", + "fü.bär", +] + +valid_names := [ + "foo.bar", + "foo.1bar", + "foo_1.bar", + "foo.bar.baz", + "foo.bar_baz", +]