Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Bug 1726326 - Add support for the text metric type #374

Merged
merged 1 commit into from
Aug 19, 2021
Merged

Conversation

badboy
Copy link
Member

@badboy badboy commented Aug 18, 2021

This applies the following additional restrictions:

  1. Metrics of this type will need to be data collection category 3 or 4.
  2. Metrics of this type will only be allowed for custom pings. Sending a text metric in built-in pings is forbidden.
  3. Metrics of this type will only be allowed to have a ping or application lifetime. Other lifetimes will be forbidden.

Restrictions are applied through the jsonschema.
This leads to okayish error messages for 1) and 3)
and a suboptimal, but understandable error message for 2).


See this output:

❯ python -m glean_parser -- translate --output tmp --format kotlin tests/data/text_invalid.yaml
==============================================================================
/Users/jer/mozilla/src/glean_parser/tests/data/text_invalid.yaml:
    ```
    invalid.text:
      lifetime:
        lifetime: user
    ```

    'user' is not one of ['ping', 'application']

    Documentation for this node:
        Text metrics must have ping or application lifetime.

==============================================================================
/Users/jer/mozilla/src/glean_parser/tests/data/text_invalid.yaml:
    ```
    invalid.text:
      sensitivity:
        data_sensitivity:
        - technical
    ```

    'technical' is not one of ['web_activity', 'highly_sensitive']
==============================================================================
/Users/jer/mozilla/src/glean_parser/tests/data/text_invalid.yaml:
    ```
    invalid.text:
      builtin_pings:
        send_in_pings:
        - metrics
    ```

    OrderedDict([('description', 'Text metrics can only be sent in custom
    pings. Built-in pings are not allowed.'), ('pattern',
    '^(metrics|baseline|events|deletion-request|default|glean_.*)$')]) is
    not allowed for 'metrics'
ERROR running glean_parser v3.7.1.dev3+g0f92902.d20210809

@badboy badboy requested a review from brizental August 18, 2021 14:00
@badboy badboy changed the title Add support for the text metric type Bug 1726326 - Add support for the text metric type Aug 18, 2021
This applies the following additional restrictions:

1) Metrics of this type will need to be data collection category 3 or 4.
2) Metrics of this type will only be allowed for custom pings. Sending a text metric in built-in pings is forbidden.
3) Metrics of this type will only be allowed to have a ping or application lifetime. Other lifetimes will be forbidden.

Restrictions are applied through the jsonschema.
This leads to okayish error messages for 1) and 3)
and a suboptimal, but understandable error message for 2).
@badboy badboy merged commit 1c66cb2 into main Aug 19, 2021
@badboy badboy deleted the text-metric branch August 19, 2021 08:45
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants