Closed Bug 1219733 Opened 5 years ago Closed 5 years ago

Allow "bug_numbers" field in Histograms.json

Categories

(Toolkit :: Telemetry, defect, P1)

defect
Points:
1

Tracking

()

RESOLVED FIXED
mozilla45
Tracking Status
firefox44 --- fixed
firefox45 --- fixed
b2g-v2.5 --- fixed

People

(Reporter: gfritzsche, Assigned: gfritzsche)

References

Details

(Whiteboard: [measurement:client])

Attachments

(1 file)

Per this thread we now require bug numbers in Telemetry probe descriptions:
https://mail.mozilla.org/pipermail/fhr-dev/2015-October/000655.html

However, it would be much better to keep that data in a separate field "bug_number".
We can allow that field from the Python script now and later backfill the field for old entries from hg history, then make it a required field.
This allows for an array of bug numbers, as discussed on IRC.
Attachment #8680647 - Flags: review?(vdjeric)
Also added first example entries for the bug 1162538 histograms to make sure this builds properly.
Depends on: 1219768
Blocks: 1219768
No longer depends on: 1219768
Summary: Allow "bug_number" field in Histograms.json → Allow "bug_numbers" field in Histograms.json
Comment on attachment 8680647 [details] [diff] [review]
Allow a 'bug_numbers' field in Histograms.json entries

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

Mark my other email address (vladan.bugzilla@gmail.com) for reviews in the future, this review didn't show up in my Bugzilla dash :(

::: toolkit/components/telemetry/Histograms.json
@@ +4725,5 @@
>      "kind": "linear",
>      "high": "13",
>      "n_buckets": 12,
> +    "description": "Number of directories in the archive at scan",
> +    "bug_numbers": [1162538]

very small nit: i prefer the last line to be the description :)

::: toolkit/components/telemetry/histogram_tools.py
@@ +239,5 @@
> +        if not isinstance(bug_numbers, list):
> +            raise ValueError, 'bug_numbers field for "%s" should be an array' % (name)
> +
> +        if not all(type(num) is int for num in bug_numbers):
> +            raise ValueError, 'bug_numbers array for "%s" should only contain numbers' % (name)

numbers -> integers
Attachment #8680647 - Flags: review?(vdjeric) → review+
https://hg.mozilla.org/mozilla-central/rev/be3cc4130caf
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
removing the b2g 2.5 flag since this commit has been reverted due to an incorrect merge, sorry for the confusion
How would you feel about uplifting this to 44? I need to uplift bug 1198209 and it wouldn't be the first time my patch burned in aurora because of a stray bug_numbers field :S
Flags: needinfo?(gfritzsche)
Sure, there is no client-side risk here.
Flags: needinfo?(gfritzsche)
Comment on attachment 8680647 [details] [diff] [review]
Allow a 'bug_numbers' field in Histograms.json entries

Approval Request Comment
[Feature/regressing bug #]: Telemetry.
[User impact if declined]: This allows a "bug_numbers" field in our histogram description, so we can link them to the bug history.
[Describe test coverage new/current, TreeHerder]: None needed, was fine on m-c.
[Risks and why]: No client-risk, this just adds a field for human or server-side consumption.
[String/UUID change made/needed]: None.
Attachment #8680647 - Flags: approval-mozilla-aurora?
Comment on attachment 8680647 [details] [diff] [review]
Allow a 'bug_numbers' field in Histograms.json entries

This has been on m-c for almost a month, seems safe to uplift to Aurora44.
Attachment #8680647 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.