1921089 - glean_parser: forbid redefinition of categories, metrics, pings in definitions files

Status: RESOLVED FIXED

(Data Platform and Tools :: Glean: SDK, defect, P4)

Description

[Chris H-C :chutten]

YAML will allow you to write e.g.

category:
  metric:
    type: not_even_a_metric_type
    ...
  metric:
    type: counter
    ...
category:
  a_different_metric:
    type: timing_distribution

and happily overwrite category.metric with the second definition and then overwrite all of category.* with category.a_different_metric later.

This is a rather annoying thing to discover by accident. We should look into whether or not we can prevent this (while still permitting the time-saving spread operations like used to set test defaults in the SDK).

Comment 1

[Chris H-C :chutten]

Actually, it's unclear whether YAML permits this or whether it's just our choice of YAML parser and parsing options that does. I shouldn't be so quick to tar the language with wrinkles it might not have.

Comment 2

[Jan-Erik Rediger [:janerik]]

PyYAML (the yaml library we use to load it) by default just uses ... the last seen key:

❯ python3
Python 3.12.6 (main, Sep  6 2024, 19:03:47) [Clang 15.0.0 (clang-1500.3.9.4)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import yaml
>>> src = '''
... category:
...   metric:
...     type: not_even_a_metric_type
...   metric:
...     type: counter
... category:
...   a_different_metric:
...     type: timing_distribution
... '''
>>> yaml.safe_load(src)
{'category': {'a_different_metric': {'type': 'timing_distribution'}}}

It seems possible to overwrite that behavior: https://stackoverflow.com/questions/44904290/getting-duplicate-keys-in-yaml-using-python/48460700#48460700

We could use that to either error out, warn or merge things together.

Comment 3

[Chris H-C :chutten]

Here's an example in the tree that Florian found: https://searchfox.org/mozilla-central/rev/9fa446ad77af13847a7da250135fc58b1a1bd5b9/toolkit/components/resistfingerprinting/metrics.yaml#1642,1660

Comment 4

[BMO Github Automation]

Attachment: [mozilla/glean_parser] bug 1921089 - Forbid redefinition of metrics in definitions files (#794)

Comment 5

[Chris H-C :chutten]

chutten merged PR [mozilla/glean_parser]: bug 1921089 - Forbid redefinition of metrics in definitions files (#794) in 1289aef.

Needs to be released and vendored, but this ought to handle it.