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

Add backward compatibility check mode #271

Merged
merged 11 commits into from
Feb 24, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
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
2 changes: 2 additions & 0 deletions semantic-conventions/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ Please update the changelog as part of any significant pull request.

- BREAKING: Make stability and deprecation independent properties.
([#244](https://github.com/open-telemetry/build-tools/pull/244))
- Add backward-compatibility check mode.
([#271](https://github.com/open-telemetry/build-tools/pull/271))

## v0.23.0

Expand Down
33 changes: 31 additions & 2 deletions semantic-conventions/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ convention that have the tag `network`.
`<!-- semconv http.server(tag=network, full) -->` will print the constraints and attributes of both `http` and `http.server`
semantic conventions that have the tag `network`.

`<!-- semconv metric.http.server.active_requests(metric_table) -->` will print a table describing a single metric
`http.server.active_requests`.
`<!-- semconv metric.http.server.active_requests(metric_table) -->` will print a table describing a single metric
`http.server.active_requests`.

## Code Generator

Expand Down Expand Up @@ -116,3 +116,32 @@ comma using the `--parameters [{key=value},]+` or `-D` flag.
The image also supports customising
[Whitespace Control in Jinja templates](https://jinja.palletsprojects.com/en/3.1.x/templates/#whitespace-control)
via the additional flag `--trim-whitespace`. Providing the flag will enable both `lstrip_blocks` and `trim_blocks`.

## Version compatibility check

You can check compatibility between the local one specified with `--yaml-root` and sepcific OpenTelemetry semantic convention version using the following command:

```bash
docker run --rm otel/semconvgen --yaml-root {yaml_folder} compatibility --previous-version {semconv version}
```

The `{semconv version}` (e.g. `1.24.0`) is the previously released version of semantic conventions.

Following checks are performed

- On all attributes and metrics (experimental and stable):
- attributes and metrics 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 metrics:
- stability must not be changed
- instrument and unit must not be changed
- new attributes should not be added.
This check does not take into account opt-in attributes. Adding new attributes to metric is not always breaking,
so it's considered non-critical and it's possible to suppress it with `--ignore-warnings`


2 changes: 1 addition & 1 deletion semantic-conventions/dev-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@ mypy==0.910
pytest==8.0.1
flake8==7.0.0
pylint==3.0.3
isort==5.13.2
isort==5.13.2
4 changes: 4 additions & 0 deletions semantic-conventions/mypy.ini
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,7 @@ ignore_missing_imports = True

[mypy-mistune.*]
ignore_missing_imports = True

[mypy-requests.*]
ignore_missing_imports = True

1 change: 1 addition & 0 deletions semantic-conventions/setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ install_requires =
ruamel.yaml~=0.16
Jinja2~=3.0
mistune==2.0.0a6
requests==2.31.0

[options.packages.find]
where = src
Expand Down
112 changes: 92 additions & 20 deletions semantic-conventions/src/opentelemetry/semconv/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,25 +16,33 @@

import argparse
import glob
import os
import sys
import tempfile
import zipfile
from typing import List

import requests

from opentelemetry.semconv.model.semantic_convention import (
CONVENTION_CLS_BY_GROUP_TYPE,
SemanticConventionSet,
)
from opentelemetry.semconv.templating.code import CodeRenderer
from opentelemetry.semconv.templating.compatibility import CompatibilityChecker
from opentelemetry.semconv.templating.markdown import MarkdownRenderer
from opentelemetry.semconv.templating.markdown.options import MarkdownOptions


def parse_semconv(args, parser) -> SemanticConventionSet:
semconv = SemanticConventionSet(args.debug)
find_yaml(args)
for file in sorted(args.files):
def parse_semconv(
yaml_root: str, exclude: str, debug: bool, parser
) -> SemanticConventionSet:
semconv = SemanticConventionSet(debug)
files = find_yaml(yaml_root, exclude)
for file in sorted(files):
if not file.endswith(".yaml") and not file.endswith(".yml"):
parser.error(f"{file} is not a yaml file.")
semconv.parse(file)
semconv.parse(file, False)
semconv.finish()
if semconv.has_error():
sys.exit(1)
Expand Down Expand Up @@ -64,8 +72,8 @@ def main():
parser = setup_parser()
args = parser.parse_args()
check_args(args, parser)
semconv = parse_semconv(args, parser)
semconv_filter = parse_only_filter(args, parser)
semconv = parse_semconv(args.yaml_root, args.exclude, args.debug, parser)
semconv_filter = parse_only_filter(args.only, parser)
filter_semconv(semconv, semconv_filter)
if len(semconv.models) == 0:
parser.error("No semantic convention model found!")
Expand All @@ -76,6 +84,8 @@ def main():
renderer.render(semconv, args.template, args.output, args.pattern)
elif args.flavor == "markdown":
process_markdown(semconv, args)
elif args.flavor == "compatibility":
check_compatibility(semconv, args, parser)


def process_markdown(semconv, args):
Expand All @@ -92,16 +102,32 @@ def process_markdown(semconv, args):
md_renderer.render_md()


def find_yaml(args):
if args.yaml_root is not None:
exclude = set(
exclude_file_list(args.yaml_root if args.yaml_root else "", args.exclude)
def check_compatibility(semconv, args, parser):
prev_semconv_path = download_previous_version(args.previous_version)
prev_semconv = parse_semconv(prev_semconv_path, args.exclude, args.debug, parser)
compatibility_checker = CompatibilityChecker(semconv, prev_semconv)
problems = compatibility_checker.check()

if any(problems):
print(f"Found {len(problems)} compatibility issues:")
for problem in sorted(str(p) for p in problems):
print(f"\t{problem}")

if not args.ignore_warnings or (
args.ignore_warnings and any(problem.critical for problem in problems)
):
sys.exit(1)


def find_yaml(yaml_root: str, exclude: str) -> List[str]:
if yaml_root is not None:
excluded_files = set(exclude_file_list(yaml_root if yaml_root else "", exclude))
yaml_files = set(glob.glob(f"{yaml_root}/**/*.yaml", recursive=True)).union(
set(glob.glob(f"{yaml_root}/**/*.yml", recursive=True))
)
yaml_files = set(
glob.glob(f"{args.yaml_root}/**/*.yaml", recursive=True)
).union(set(glob.glob(f"{args.yaml_root}/**/*.yml", recursive=True)))
file_names = yaml_files - exclude
args.files.extend(file_names)
return list(yaml_files - excluded_files)

return []


def check_args(arguments, parser):
Expand All @@ -110,11 +136,11 @@ def check_args(arguments, parser):
parser.error("Either --yaml-root or YAML_FILE must be present")


def parse_only_filter(arguments, parser):
if not arguments.only:
return None
def parse_only_filter(only: str, parser) -> List[str]:
if not only:
return []

types = [t.strip() for t in arguments.only.split(",")]
types = [t.strip() for t in only.split(",")]
unknown_types = [t for t in types if t not in CONVENTION_CLS_BY_GROUP_TYPE.keys()]
if unknown_types:
parser.error(
Expand Down Expand Up @@ -188,6 +214,12 @@ def add_md_parser(subparsers):
required=False,
action="store_true",
)
parser.add_argument(
"--check-compat",
help="Check backward compatibility with previous version of semantic conventions.",
type=str,
required=False,
)
parser.add_argument(
"--md-use-badges",
help="Use stability badges instead of labels for attributes.",
Expand Down Expand Up @@ -216,6 +248,23 @@ def add_md_parser(subparsers):
)


def add_compat_check_parser(subparsers):
parser = subparsers.add_parser("compatibility")
parser.add_argument(
"--previous-version",
help="Check backward compatibility with specified older version of semantic conventions.",
type=str,
required=True,
)
parser.add_argument(
"--ignore-warnings",
help="Ignore non-critical compatibility problems.",
required=False,
default=False,
action="store_true",
)


def setup_parser():
parser = argparse.ArgumentParser(
description="Process Semantic Conventions yaml files."
Expand Down Expand Up @@ -258,9 +307,32 @@ def setup_parser():
subparsers = parser.add_subparsers(dest="flavor")
add_code_parser(subparsers)
add_md_parser(subparsers)
add_compat_check_parser(subparsers)

return parser


def download_previous_version(version: str) -> str:
filename = f"v{version}.zip"
tmppath = tempfile.mkdtemp()
path_to_zip = os.path.join(tmppath, filename)
path_to_semconv = os.path.join(tmppath, f"v{version}")

semconv_vprev = (
f"https://github.com/open-telemetry/semantic-conventions/archive/{filename}"
lmolkova marked this conversation as resolved.
Show resolved Hide resolved
)

response = requests.get(semconv_vprev, allow_redirects=True, timeout=30)
response.raise_for_status()

with open(path_to_zip, "wb") as zip_file:
zip_file.write(response.content)

with zipfile.ZipFile(path_to_zip, "r") as zip_ref:
zip_ref.extractall(path_to_semconv)

return os.path.join(path_to_semconv, f"semantic-conventions-{version}", "model")


if __name__ == "__main__":
main()
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,9 @@ def is_enum(self):
return isinstance(self.attr_type, EnumAttributeType)

@staticmethod
def parse(prefix, yaml_attributes) -> "Dict[str, SemanticAttribute]":
def parse(
prefix, yaml_attributes, strict_validation=True
) -> "Dict[str, SemanticAttribute]":
"""This method parses the yaml representation for semantic attributes
creating the respective SemanticAttribute objects.
"""
Expand Down Expand Up @@ -177,7 +179,7 @@ def parse(prefix, yaml_attributes) -> "Dict[str, SemanticAttribute]":

tag = attribute.get("tag", "").strip()
stability = SemanticAttribute.parse_stability(
attribute.get("stability"), position_data
attribute.get("stability"), position_data, strict_validation
)
deprecated = SemanticAttribute.parse_deprecated(
attribute.get("deprecated"), position_data
Expand Down Expand Up @@ -280,7 +282,7 @@ def parse_attribute(attribute):
return attr_type, str(brief), examples

@staticmethod
def parse_stability(stability, position_data):
def parse_stability(stability, position_data, strict_validation=True):
if stability is None:
return StabilityLevel.EXPERIMENTAL

Expand All @@ -291,6 +293,15 @@ def parse_stability(stability, position_data):
val = stability_value_map.get(stability)
if val is not None:
return val

# TODO: remove this branch - it's necessary for now to allow back-compat checks against old spec versions
# where we used 'deprecated' as stability level
if not strict_validation and stability == "deprecated":
print(
'WARNING: Using "deprecated" as stability level is no longer supported. Use "experimental" instead.'
)
return StabilityLevel.EXPERIMENTAL

msg = f"Value '{stability}' is not allowed as a stability marker"
raise ValidationError.from_yaml_pos(position_data["stability"], msg)

Expand Down
Loading
Loading