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

(chore) Move backwards compatibility checks to weaver. #1327

Merged
merged 22 commits into from
Aug 20, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
a5f6389
Initial cut at backwards compatibility check moving to weaver.
jsuereth Aug 7, 2024
737df29
Add policies to enforce attribute/enum backwards compatibility.
jsuereth Aug 8, 2024
f3acf8c
Add some metric policies.
jsuereth Aug 8, 2024
d1f56be
Finish implementing policies.
jsuereth Aug 8, 2024
f7cb36a
Update makefile to only have one policy check.
jsuereth Aug 8, 2024
f20ec2c
Fix nits.
jsuereth Aug 8, 2024
0a141fc
Merge branch 'main' into wip-backwards-compat-weaver
jsuereth Aug 8, 2024
6f1ed90
Update docs.
jsuereth Aug 8, 2024
9b77e46
Move to using policies directory.
jsuereth Aug 9, 2024
ba366a9
Add old backwards compat checks back.
jsuereth Aug 9, 2024
1a77174
Fix lint issues.
jsuereth Aug 9, 2024
5fa1fbd
Merge main - Fix test policies to not conflict.
jsuereth Aug 16, 2024
ac39d30
Add some tests for stability checks.
jsuereth Aug 16, 2024
2a18e5a
Added tests.
jsuereth Aug 16, 2024
f0f8508
Add policy unit tests to github actions.
jsuereth Aug 16, 2024
9ea051b
Merge branch 'main' into wip-backwards-compat-weaver
lmolkova Aug 19, 2024
f55a27d
Update attribute name rules to v1 syntax and fix shadowing issue.
jsuereth Aug 19, 2024
a2e8719
Minor formatting.
jsuereth Aug 19, 2024
3d358b1
Merge branch 'main' into wip-backwards-compat-weaver
jsuereth Aug 19, 2024
fa633ba
Merge branch 'main' into wip-backwards-compat-weaver
jsuereth Aug 19, 2024
293d472
Merge branch 'main' into wip-backwards-compat-weaver
jsuereth Aug 19, 2024
395ec0c
Merge branch 'main' into wip-backwards-compat-weaver
jsuereth Aug 20, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 8 additions & 12 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ CHLOGGEN_CONFIG := .chloggen/config.yaml
# see https://github.com/open-telemetry/build-tools/releases for semconvgen updates
# Keep links in model/README.md and .vscode/settings.json in sync!
SEMCONVGEN_VERSION=0.25.0
WEAVER_VERSION=0.7.0
WEAVER_VERSION=0.8.0

# From where to resolve the containers (e.g. "otel/weaver").
CONTAINER_REPOSITORY=docker.io
Expand Down Expand Up @@ -111,15 +111,6 @@ install-yamllint:
yamllint:
yamllint .

# Check semantic convention policies on YAML files
.PHONY: check-policies
check-policies:
docker run --rm -v $(PWD)/model:/source -v $(PWD)/policies:/policies -v $(PWD)/templates:/templates \
otel/weaver:${WEAVER_VERSION} registry check \
--registry=/source \
--diagnostic-format=ansi \
--policy=/policies/registry.rego

# Generate markdown tables from YAML definitions
.PHONY: table-generation
table-generation:
Expand Down Expand Up @@ -171,8 +162,13 @@ table-check:
LATEST_RELEASED_SEMCONV_VERSION := $(shell git ls-remote --tags https://github.com/open-telemetry/semantic-conventions.git | cut -f 2 | sort --reverse | head -n 1 | tr '/' ' ' | cut -d ' ' -f 3 | $(SED) 's/v//g')
.PHONY: compatibility-check
compatibility-check:
docker run --rm -v $(PWD)/model:/source -v $(PWD)/docs:/spec --pull=always \
$(SEMCONVGEN_CONTAINER) -f /source compatibility --previous-version $(LATEST_RELEASED_SEMCONV_VERSION)
docker run --rm -v $(PWD)/model:/source -v $(PWD)/policies:/policies -v $(PWD)/templates:/templates \
otel/weaver:${WEAVER_VERSION} registry check \
--registry=/source \
--baseline-registry=https://github.com/open-telemetry/semantic-conventions/archive/refs/tags/v$(LATEST_RELEASED_SEMCONV_VERSION).zip[model] \
--diagnostic-format=ansi \
--policy=/policies/compatibility.rego
jsuereth marked this conversation as resolved.
Show resolved Hide resolved


.PHONY: schema-check
schema-check:
Expand Down
218 changes: 218 additions & 0 deletions policies/compatibility.rego
Original file line number Diff line number Diff line change
@@ -0,0 +1,218 @@
package comparison_after_resolution

import rego.v1

# Semantic Convention Registry Compatibility Checker
#
# This file contains rules for checking backward compatibility
# between different versions of semantic convention registries.
# It builds upon the data structures and rules defined in the
# semconv package.

# Import the previous release (baseline) and current sets of attributes, metrics.
baseline_attributes := {attr |
some g in data.semconv.registry_baseline_groups
some attr in g.attributes
}
registry_attributes := {attr |
some g in data.semconv.registry_groups
some attr in g.attributes
}
registry_attribute_names := {attr.name |
some g in data.semconv.registry_groups
some attr in g.attributes
}


# Rules we enforce:
jsuereth marked this conversation as resolved.
Show resolved Hide resolved
# - Attributes
# - [x] Attributes cannot be removed
# - [x] Attributes cannot "degrade" stability (stable->experimental)
# - [x] Stable attributes cannot change type
# - Enum members
# - [x] Stable members cannot change stability
# - [X] Values cannot change
# - [x] ids cannot be removed
# - Metrics
# - [ ] metrics cannot be removed
# - [ ] Stable metrics cannot become unstable
# - [ ] Stable Metric units cannot change
# - [ ] Stable Metric instruments cannot change
# - [ ] Set of required/recommended attributes must remain the same


# Rule: Detect Removed Attributes
#
# This rule checks for attributes that existed in the baseline registry
# but are no longer present in the current registry. Removing attributes
# is considered a backward compatibility violation.
#
# In other words, we do not allow the removal of an attribute once added
# to the registry. It must exist SOMEWHERE in a group, but may be deprecated.
deny contains back_comp_violation(description, group_id, attr.name) if {
jsuereth marked this conversation as resolved.
Show resolved Hide resolved
# Check if an attribute from the baseline is missing in the current registry
some attr in baseline_attributes
not registry_attribute_names[attr.name]

# Generate human readable error.
group_id := data.semconv.baseline_group_ids_by_attribute[attr.name]
description := sprintf("Attribute '%s' no longer exists in the attribute registry", [attr.name])
}



# Rule: Detect Stable Attributes moving to unstable
#
# This rule checks for attributes that were stable in the baseline registry
# but are no longer stable in the current registry. Once stable, attributes
# remain forever but may be deprecated.
deny contains back_comp_violation(description, group_id, attr.name) if {
# Find stable baseline attributes in latest registry.
some attr in baseline_attributes
attr.stability == "stable"
some nattr in registry_attributes
attr.name == nattr.name

# Enforce the policy
attr.stability != nattr.stability

# Generate human readable error.
group_id := data.semconv.baseline_group_ids_by_attribute[attr.name]
description := sprintf("Attribute '%s' was stable, but has new stability marker", [attr.name])
}

# Rule: Detect Stable Attributes changing type
#
# This rule checks for attributes that were stable in the baseline registry
# but are no longer stable in the current registry. Once stable, attributes
# remain forever but may be deprecated.
deny contains back_comp_violation(description, group_id, attr.name) if {
# Find stable baseline attributes in latest registry.
some attr in baseline_attributes
attr.stability == "stable"
some nattr in registry_attributes
attr.name == nattr.name

# Enforce the policy
# TODO - deal with enum type changes, probably in enum sections
not is_enum(attr)
attr.type != nattr.type

# Generate human readable error.
group_id := data.semconv.baseline_group_ids_by_attribute[attr.name]
attr_type_string := type_string(attr)
nattr_type_string := type_string(nattr)
description := sprintf("Attribute '%s' was '%s', but has new type '%s'", [attr.name, attr_type_string, nattr_type_string])
}

# Rule: Detect Stable enum Attributes changing type
#
# This rule checks for attributes that were stable in the baseline registry
# but are no longer stable in the current registry. Once stable, attributes
# remain forever but may be deprecated.
deny contains back_comp_violation(description, group_id, attr.name) if {
# Find stable baseline attributes in latest registry.
some attr in baseline_attributes
attr.stability == "stable"
some nattr in registry_attributes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is one of the instances where rewriting registry_attributes as shown below will accelerate the rules. Instead of having a list of attributes, we rely on a map of attribute name -> attribute.

registry_attributes := [name: attr |
    some g in data.semconv.registry_groups
    some attr in g.attributes
    name := attr.name
]

attr.name == nattr.name
# Enforce the policy
attr.type != nattr.type
is_enum(attr)
not is_enum(nattr)

# Generate human readable error.
group_id := data.semconv.baseline_group_ids_by_attribute[attr.name]
nattr_type_string := type_string(nattr)
description := sprintf("Attribute '%s' was enum, but has new type '%s'", [attr.name, nattr_type_string])
}

# Rule: Detect Stable Enum members changing stability level
#
# This rule checks for enum values that were stable in the baseline registry
# but are no longer stable in the current registry.
deny contains back_comp_violation(description, group_id, attr.name) if {
# Find data we need to enforce: Enums in baseline/current.
some attr in baseline_attributes
attr.stability == "stable"
some nattr in registry_attributes
attr.name == nattr.name
is_enum(attr)
some member in attr.type.members
some nmember in nattr.type.members
member.id == nmember.id

# Enforce the policy
member.stability == "stable"
nmember.stability != "stable"

# Generate human readable error.
group_id := data.semconv.baseline_group_ids_by_attribute[attr.name]
description := sprintf("Enum '%s' had stable member '%s', but is no longer stable", [attr.name, member.id])
}

# Rule: Enum member values cannot change
#
# This rule checks for enum values that have the same id, but values
# are different.
deny contains back_comp_violation(description, group_id, attr.name) if {
# Find data we need to enforce: Enums in baseline/current.
some attr in baseline_attributes
attr.stability == "stable"
some nattr in registry_attributes
attr.name == nattr.name
is_enum(attr)
some member in attr.type.members
some nmember in nattr.type.members
member.id == nmember.id

# Enforce the policy
member.value != nmember.value

# Generate human readable error.
group_id := data.semconv.baseline_group_ids_by_attribute[attr.name]
description := sprintf("Enum '%s' had stable value '%s', but is now '%s'", [attr.name, member.value, nmember.value])
}

# Rule: Detect Stable Enum members missing
#
# This rule checks for enum values that were stable in the baseline registry
# but are no longer have the same values in the current registry. Once stable,
# enum values remain forever but may be deprecated.
deny contains back_comp_violation(description, group_id, attr.name) if {
# Find data we need to enforce: Enums in baseline/current.
some attr in baseline_attributes
attr.stability == "stable"
some nattr in registry_attributes
attr.name == nattr.name
is_enum(attr)
current_member_ids := {member.id | some member in nattr.type.members}
# Enforce the policy
some member in attr.type.members
not current_member_ids[member.id]
jsuereth marked this conversation as resolved.
Show resolved Hide resolved

# Generate human readable error.
group_id := data.semconv.baseline_group_ids_by_attribute[attr.name]
description := sprintf("Enum '%s' had member '%s', but is no longer defined", [attr.name, member.id])
}

# Rule: Detect Stable enum

# Helper Function: Create Backward Compatibility Violation Object
#
# This function generates a structured violation object for each
# detected backward compatibility issue.
back_comp_violation(description, group_id, attr_id) := violation if {
violation := {
"id": description,
"type": "semconv_attribute",
"category": "backward_compatibility",
"group": group_id,
"attr": attr_id,
}
}

# Helpers for enum values and type strings
is_enum(attr) := true if count(attr.type.members) > 0
type_string(attr) := attr.type if not is_enum(attr)
type_string(attr) := "enum" if is_enum(attr)
Loading