feat(glean_usage): Generate schema.yaml files for events_stream ETLs/views (DENG-11586) - #9932
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR adds schema.yaml templates for the generated events_stream_v1 tables, the per-app-ID events_stream views, and the cross-channel events_stream views. The schemas pull field descriptions from the upstream events views with !include-field(s). It also changes get_glean_app_event_extras_by_type to collect extra descriptions from the probe-info API, so the typed extras struct gets per-field descriptions. Where several events describe the same extra differently, the most common description is listed first. I checked the field order in events_stream_v1.schema.yaml against events_stream_v1.query.sql and it matches, including the metrics_as_struct/has_metrics, profile group ID and legacy client ID branches. I left one inline suggestion.
| extra = extras_by_type[extra_type][extra_name] | ||
| extra["name"] = extra_name | ||
| extra["type"] = extra_type | ||
| extra["descriptions"].extend(app_id_extra["descriptions"]) |
There was a problem hiding this comment.
suggestion: This merge loop (set name/type, extend descriptions) repeats the one in get_glean_app_event_extras_by_type at lines 246-251, and the nested defaultdict(lambda: defaultdict(lambda: {...})) factory is also written out twice. Pulling both into small helpers (e.g. _new_extras_by_type() and _merge_extras_by_type(target, source)) would keep the two call sites from drifting apart.
Also, the name and type keys are never read: macros.yaml and macros.sql take the name and type from the dict keys and only use extra['descriptions']. If you drop them, the inner value can be a plain defaultdict(list) of descriptions.
There was a problem hiding this comment.
I refactored some of the repeated logic into helper functions as suggested.
Regarding the technically unused name and type entries in the extras_by_type dicts, my thinking was that it's more intuitive for the innermost values in that nested dictionary setup to be full-fledged event extra dicts rather than just the descriptions. In an initial implementation I actually did just have lists of descriptions as the innermost values, but then I got hung up on having to explain that structure in comments or via cumbersome variable names like extra_descriptions_by_type_and_name. I find the current implementation to be simpler conceptually, and I don't think having those extra values really hurts anything.
This comment has been minimized.
This comment has been minimized.
Integration report
|
4 participants
Description
With field descriptions, as part of our metadata completeness effort (DENG-10445).
Related Tickets & Documents
Reviewer, please follow this checklist