Skip to content

Commit

Permalink
Add support for the text metric type
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
badboy committed Aug 18, 2021
1 parent 5d58a11 commit 93fd81c
Show file tree
Hide file tree
Showing 6 changed files with 200 additions and 0 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

## Unreleased

- Add support for Text metric type ([#374](https://github.com/mozilla/glean_parser/pull/374))

## 3.8.0 (2021-08-18)

- Expose ping reasons enum on JavaScript / TypeScript templates. ([bug 1719136](https://bugzilla.mozilla.org/show_bug.cgi?id=1719136))
Expand Down
4 changes: 4 additions & 0 deletions glean_parser/metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -396,4 +396,8 @@ def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)


class Text(Metric):
typename = "text"


ObjectTree = Dict[str, Dict[str, Union[Metric, pings.Ping]]]
41 changes: 41 additions & 0 deletions glean_parser/schemas/metrics.2-0-0.schema.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@ definitions:
`labeled_boolean`, `labeled_string`, `labeled_counter`.
- `text`: Record long text data.
type: string
enum:
- event
Expand All @@ -137,6 +139,7 @@ definitions:
- labeled_string
- labeled_counter
- rate
- text

description:
title: Description
Expand Down Expand Up @@ -625,3 +628,41 @@ additionalProperties:
description: |
`denominator_metric` is only allowed for `rate`.
maxLength: 0
-
if:
properties:
type:
const: text
then:
properties:
lifetime:
description: >
Text metrics must have ping or application lifetime.
enum:
- ping
- application

data_sensitivity:
description: >
Text metrics require Category 3 (`web_activity`)
or Category 4 (`highly_sensitive`).
type: array
items:
enum:
- web_activity
- highly_sensitive

send_in_pings:
description: |
Text metrics can only be sent in custom pings.
Built-in pings are not allowed.
type: array
items:
allOf:
- $ref: "#/definitions/kebab_case"
- not:
description: >
Text metrics can only be sent in custom pings.
Built-in pings are not allowed."
pattern:
"^(metrics|baseline|events|deletion-request|default|glean_.*)$"
44 changes: 44 additions & 0 deletions tests/data/text.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# Any copyright is dedicated to the Public Domain.
# https://creativecommons.org/publicdomain/zero/1.0/

---
$schema: moz://mozilla.org/schemas/glean/metrics/2-0-0

valid.text:
lifetime:
type: text
lifetime: ping
send_in_pings:
- custom
description: |
dummy metric
bugs:
- https://bugzilla.mozilla.org/11137353
data_reviews:
- http://example.com/reviews
notification_emails:
- CHANGE-ME@example.com
expires: 2100-01-01
data_sensitivity:
- highly_sensitive
no_lint:
- EXPIRATION_DATE_TOO_FAR

sensitivity:
type: text
lifetime: ping
send_in_pings:
- custom
description: |
dummy metric
bugs:
- https://bugzilla.mozilla.org/11137353
data_reviews:
- http://example.com/reviews
notification_emails:
- CHANGE-ME@example.com
expires: 2100-01-01
data_sensitivity:
- web_activity
no_lint:
- EXPIRATION_DATE_TOO_FAR
63 changes: 63 additions & 0 deletions tests/data/text_invalid.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# Any copyright is dedicated to the Public Domain.
# https://creativecommons.org/publicdomain/zero/1.0/

---
$schema: moz://mozilla.org/schemas/glean/metrics/2-0-0

invalid.text:
lifetime:
type: text
lifetime: user
send_in_pings:
- custom
description: |
dummy metric
bugs:
- https://bugzilla.mozilla.org/11137353
data_reviews:
- http://example.com/reviews
notification_emails:
- CHANGE-ME@example.com
expires: 2100-01-01
data_sensitivity:
- highly_sensitive
no_lint:
- EXPIRATION_DATE_TOO_FAR

sensitivity:
type: text
lifetime: ping
send_in_pings:
- custom
description: |
dummy metric
bugs:
- https://bugzilla.mozilla.org/11137353
data_reviews:
- http://example.com/reviews
notification_emails:
- CHANGE-ME@example.com
expires: 2100-01-01
data_sensitivity:
- technical
no_lint:
- EXPIRATION_DATE_TOO_FAR

builtin_pings:
type: text
lifetime: ping
send_in_pings:
- metrics
description: |
dummy metric
bugs:
- https://bugzilla.mozilla.org/11137353
data_reviews:
- http://example.com/reviews
notification_emails:
- CHANGE-ME@example.com
expires: 2100-01-01
data_sensitivity:
- highly_sensitive
no_lint:
- EXPIRATION_DATE_TOO_FAR
44 changes: 44 additions & 0 deletions tests/test_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -667,3 +667,47 @@ def test_rates():
assert (
category["the_denominator"].type == "counter"
) # Hasn't been transformed to "denominator" yet


def test_text_valid():
"""
Ensure that `text` metrics parse properly.
"""

all_metrics = parser.parse_objects(
[ROOT / "data" / "text.yaml"],
config={"allow_reserved": False},
)

errors = list(all_metrics)
assert len(errors) == 0

assert all_metrics.value["valid.text"]["lifetime"].lifetime == metrics.Lifetime.ping

assert all_metrics.value["valid.text"]["sensitivity"].data_sensitivity == [
metrics.DataSensitivity.web_activity
]


def test_text_invalid():
"""
Ensure that `text` metrics parse properly.
"""

all_metrics = parser.parse_objects(
[ROOT / "data" / "text_invalid.yaml"],
config={"allow_reserved": False},
)

errors = list(all_metrics)
assert len(errors) == 3

for error in errors:
if "sensitivity" in error:
assert "'technical' is not one of" in error

if "lifetime" in error:
assert "'user' is not one of" in error

if "builtin_pings" in error:
assert "Built-in pings are not allowed" in error

0 comments on commit 93fd81c

Please # to comment.