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.

Sign up for GitHub

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

Support reading additional files with references #156

Closed
wants to merge 2 commits into from
+111 −16
Closed

Support reading additional files with references #156

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
df4e77e
Support reading additional files with references
lmolkova Mar 9, 2023
018e874
lint
lmolkova Mar 9, 2023
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
35 changes: 31 additions & 4 deletions semantic-conventions/src/opentelemetry/semconv/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,30 @@ def parse_semconv(args, parser) -> SemanticConventionSet:
semconv = SemanticConventionSet(args.debug)
find_yaml(args)
for file in sorted(args.files):
if not file.endswith(".yaml") and not file.endswith(".yml"):
if not is_yaml_file(file):
parser.error(f"{file} is not a yaml file.")
semconv.parse(file)

if args.reference_only is not None:
reference_files = [
f
for f in glob_file_list(args.yaml_root, args.reference_only)
if is_yaml_file(f)
]
for file in sorted(reference_files):
semconv.parse(file, True)

semconv.finish()
if semconv.has_error():
sys.exit(1)
return semconv


def exclude_file_list(folder: str, pattern: str) -> List[str]:
def is_yaml_file(file: str) -> bool:
return file.endswith(".yaml") or file.endswith(".yml")


def glob_file_list(folder: str, pattern: str) -> List[str]:
if not pattern:
return []
sep = "/"
Expand Down Expand Up @@ -85,7 +99,7 @@ def process_markdown(semconv, args):
enable_deprecated=args.md_enable_deprecated,
use_badge=args.md_use_badges,
break_count=args.md_break_conditional,
exclude_files=exclude_file_list(args.markdown_root, args.exclude),
exclude_files=glob_file_list(args.markdown_root, args.exclude),
)
md_renderer = MarkdownRenderer(args.markdown_root, semconv, options)
md_renderer.render_md()
Expand All @@ -94,8 +108,15 @@ def process_markdown(semconv, args):
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)
glob_file_list(args.yaml_root if args.yaml_root else "", args.exclude)
)
reference_only = set(
glob_file_list(
args.yaml_root if args.yaml_root else "", args.reference_only
)
)
exclude.update(reference_only)

yaml_files = set(
glob.glob(f"{args.yaml_root}/**/*.yaml", recursive=True)
).union(set(glob.glob(f"{args.yaml_root}/**/*.yml", recursive=True)))
Expand Down Expand Up @@ -223,6 +244,12 @@ def setup_parser():
parser.add_argument(
"--exclude", "-e", help="Exclude the matching files using GLOB syntax", type=str
)
parser.add_argument(
"--reference-only",
"-r",
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we actually want a short-name for this flag.

help="Additional files to resolve references only (and extends) using GLOB syntax",
Copy link
Member

Choose a reason for hiding this comment

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

I think this feature warrants a bit more description. This suggestion is also a check if I understood this PR correctly 😃

Suggested change
help="Additional files to resolve references only (and extends) using GLOB syntax",
help="""Additional files that will be loaded and available for referencing, but will not be part of the output.
As semantic conventions can, in various ways, e.g. using `ref` or `include`, reference other conventions you
sometimes need to add files/groups to your input but you don't want the groups in there be processed by
the code generator template or markdown generator. In this case, add only the subset of files you want to
process with your template / markdown generator to `--yaml-root`, and add any additional required
(transitively depended-upon) files with `--reference-only`.
""",

type=str,
)
parser.add_argument(
"files",
metavar="YAML_FILE",
Expand Down
30 changes: 18 additions & 12 deletions semantic-conventions/src/opentelemetry/semconv/model/semantic_convention.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,9 +300,12 @@ class SemanticConventionSet:

debug: bool
models: typing.Dict[str, BaseSemanticConvention] = field(default_factory=dict)
extended_models: typing.Dict[str, BaseSemanticConvention] = field(
default_factory=dict
)
errors: bool = False

def parse(self, file):
def parse(self, file, ref_only=False):
Copy link
Member

Choose a reason for hiding this comment

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

Maybe instead add parse_into(self, file, models)?

If you prefer keeping the boolean argument, please add * before to enforce using named arguments for it:

Suggested change
def parse(self, file, ref_only=False):
def parse(self, file, *, ref_only=False):

with open(file, "r", encoding="utf-8") as yaml_file:
try:
semconv_models = parse_semantic_convention_groups(yaml_file)
Expand All @@ -314,7 +317,10 @@ def parse(self, file):
f"Semantic convention '{model.semconv_id}' is already defined.",
file=sys.stderr,
)
self.models[model.semconv_id] = model
if not ref_only:
self.models[model.semconv_id] = model

self.extended_models[model.semconv_id] = model
except ValidationError as e:
self.errors = True
print(f"Error parsing {file}\n", file=sys.stderr)
Expand All @@ -325,7 +331,7 @@ def has_error(self):

def check_unique_fqns(self):
group_by_fqn: typing.Dict[str, str] = {}
for model in self.models.values():
for model in self.extended_models.values():
for attr in model.attributes:
if not attr.ref:
if attr.fqn in group_by_fqn:
Expand All @@ -351,7 +357,7 @@ def finish(self):
fixpoint = True
if index > 0:
self.debug = False
for semconv in self.models.values():
for semconv in self.extended_models.values():
# Ref first, extends and includes after!
fixpoint_ref = self.resolve_ref(semconv)
fixpoint_inc = self.resolve_include(semconv)
Expand All @@ -370,7 +376,7 @@ def _populate_extends(self):
This internal method goes through every semantic convention to resolve parent/child relationships.
:return: None
"""
unprocessed = self.models.copy()
unprocessed = self.extended_models.copy()
# Iterate through the list and remove the semantic conventions that have been processed.
while len(unprocessed) > 0:
semconv = next(iter(unprocessed.values()))
Expand All @@ -386,7 +392,7 @@ def _populate_extends_single(self, semconv, unprocessed):
"""
# Resolve parent of current Semantic Convention
if semconv.extends:
extended = self.models.get(semconv.extends)
extended = self.extended_models.get(semconv.extends)
if extended is None:
raise ValidationError.from_yaml_pos(
semconv._position,
Expand All @@ -398,7 +404,7 @@ def _populate_extends_single(self, semconv, unprocessed):
not_yet_processed = extended.extends in unprocessed
if extended.extends and not_yet_processed:
# Recursion on parent if was not already processed
parent_extended = self.models.get(extended.extends)
parent_extended = self.extended_models.get(extended.extends)
self._populate_extends_single(parent_extended, unprocessed)

# inherit prefix and constraints
Expand Down Expand Up @@ -442,7 +448,7 @@ def _sort_attributes_dict(

def _populate_anyof_attributes(self):
any_of: AnyOf
for semconv in self.models.values():
for semconv in self.extended_models.values():
for any_of in semconv.constraints:
if not isinstance(any_of, AnyOf):
continue
Expand All @@ -461,10 +467,10 @@ def _populate_anyof_attributes(self):
any_of.add_attributes(constraint_attrs)

def _populate_events(self):
for semconv in self.models.values():
for semconv in self.extended_models.values():
events: typing.List[EventSemanticConvention] = []
for event_id in semconv.events:
event = self.models.get(event_id)
event = self.extended_models.get(event_id)
if event is None:
raise ValidationError.from_yaml_pos(
semconv._position,
Expand Down Expand Up @@ -511,7 +517,7 @@ def resolve_include(self, semconv):
fixpoint_inc = True
for constraint in semconv.constraints:
if isinstance(constraint, Include):
include_semconv = self.models.get(constraint.semconv_id)
include_semconv = self.extended_models.get(constraint.semconv_id)
# include required attributes and constraints
if include_semconv is None:
raise ValidationError.from_yaml_pos(
Expand Down Expand Up @@ -552,7 +558,7 @@ def _lookup_attribute(self, attr_id: str) -> Union[SemanticAttribute, None]:
return next(
(
attr
for model in self.models.values()
for model in self.extended_models.values()
for attr in model.attributes
if attr.fqn == attr_id and attr.ref is None
),
Expand Down
62 changes: 62 additions & 0 deletions semantic-conventions/src/tests/semconv/model/test_correct_parse.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -727,6 +727,68 @@ def test_scope_attribute(self):
}
self.semantic_convention_check(list(semconv.models.values())[0], expected)

def test_reference_only_ref(self):
semconv = SemanticConventionSet(debug=False)
semconv.parse(self.load_file("yaml/http.yaml"))
self.assertEqual(len(semconv.models), 3)
semconv.parse(self.load_file("yaml/general.yaml"), True)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
semconv.parse(self.load_file("yaml/general.yaml"), True)
semconv.parse(self.load_file("yaml/general.yaml"), ref_only=True)

etc
(or see comment before for maybe using an API that does not rely on a boolean)

semconv.finish()

self.assertEqual(len(semconv.models), 3)

def test_reference_only_extends(self):
semconv = SemanticConventionSet(debug=False)
semconv.parse(self.load_file("yaml/extends/child.http.yaml"))
self.assertEqual(len(semconv.models), 1)
semconv.parse(self.load_file("yaml/http.yaml"), True)
semconv.parse(self.load_file("yaml/general.yaml"), True)
semconv.finish()

self.assertEqual(len(semconv.models), 1)
expected = {
"id": "child.http.server",
"prefix": "child.http",
"extends": "http.server",
"n_constraints": 1,
"attributes": ["http.server_name"],
}
self.semantic_convention_check(list(semconv.models.values())[0], expected)

def test_reference_only_include(self):
semconv = SemanticConventionSet(debug=False)
semconv.parse(self.load_file("yaml/http.yaml"), True)
semconv.parse(self.load_file("yaml/faas.yaml"))
semconv.parse(self.load_file("yaml/general.yaml"), True)
semconv.finish()
self.assertEqual(len(semconv.models), 5)

faas_http = [s for s in semconv.models.values() if s.semconv_id == "faas.http"][
0
]
expected = {
"id": "faas.http",
"prefix": "faas",
"extends": "faas",
"n_constraints": 2,
"attributes": [
# Parent
"faas.trigger",
"faas.execution",
# Include
"http.method",
"http.url",
"http.target",
"http.host",
"http.scheme",
"http.status_code",
"http.status_text",
"http.flavor",
"http.user_agent",
"http.server_name",
],
}
self.semantic_convention_check(faas_http, expected)

_TEST_DIR = os.path.dirname(__file__)

def load_file(self, filename):
Expand Down