Closed Bug 1381617 Opened 7 years ago Closed 7 years ago

Scalars.yaml expires: "Never" doesn't work but doesn't error

Categories

(Toolkit :: Telemetry, enhancement, P1)

enhancement
Points:
1

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: benjamin, Assigned: Dexter)

Details

(Whiteboard: [measurement:client])

Attachments

(1 file, 2 obsolete files)

I was adding a new scalar and mis-capitalized the expiry in Scalars.yaml:

  plugin_instantiated:
    bug_numbers:
      - 1381591
    description: >-
      The number of plugin instances that were created.
    expires: 'Never' # Jan-2021 but we don't have a version number for that
    kind: uint
    notification_emails:
      - bsmedberg@mozilla.com
    release_channel_collection: opt-out
    record_in_processes:
      - 'main'
      - 'content'

This built correctly but didn't record anything. It would probably be good to add some stricter validation for this field so that the only things that are accepted are "never" lowercase, or a two-digit number.
Assignee: nobody → alessio.placitelli
Points: --- → 1
Priority: -- → P1
Whiteboard: [measurement:client]
Attached patch bug1381617.patch (obsolete) — Splinter Review
Chris, I found that the problem Benjamin mentioned also affects Histogram and events. I refactored the validation code in a shared function, which now consistently checks all the collection primitives.
Attachment #8887424 - Flags: review?(chutten)
Comment on attachment 8887424 [details] [diff] [review]
bug1381617.patch

Review of attachment 8887424 [details] [diff] [review]:
-----------------------------------------------------------------

One nit.

::: toolkit/components/telemetry/shared_telemetry_utils.py
@@ +120,5 @@
>  
> +def validate_expiration_version(expiration):
> +    """ Makes sure the expiration version has the expected format.
> +
> +    Allowed examples: "1.0", "20", "300.0a1", "60.0a1", "30.5a1", "never'

mismatched quotes on "never'
Attachment #8887424 - Flags: review?(chutten) → review+
Attached patch bug1381617.patch (obsolete) — Splinter Review
Attachment #8887424 - Attachment is obsolete: true
Attachment #8887527 - Flags: review+
Attached patch bug1381617.patchSplinter Review
Fix the linting problems.
Attachment #8887527 - Attachment is obsolete: true
Attachment #8887547 - Flags: review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/092390ed4c628d13875ddf3f9412e3de2b29a98c
Bug 1381617 - Enable stricter scalar, histogram and events expiration validation. r=chutten
Pushed by alessio.placitelli@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/092390ed4c62
Enable stricter scalar, histogram and events expiration validation. r=chutten
https://hg.mozilla.org/mozilla-central/rev/092390ed4c62
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: