Closed
Bug 1376599
Opened 7 years ago
Closed 7 years ago
Add "type" key to experiment annotations
Categories
(Toolkit :: Telemetry, enhancement, P1)
Toolkit
Telemetry
Tracking
()
RESOLVED
FIXED
mozilla56
People
(Reporter: bugzilla, Assigned: gfritzsche)
References
Details
Attachments
(1 file)
12.71 KB,
patch
|
Dexter
:
review+
jcristau
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
Currently, if pref rollouts will end up using experiment annotations, this will blow up the lua filter that creates the telemetry-experiments source. If we add a "type" field to the annotation (and get normandy to annotate those with its own type) we should be able to ignore pref rollouts in the filter. We *might not* have enough time to do this before the rollouts go out, in which case we need to work with normandy to use a naming convention that we'll add to the filter (but we should still add the type annotation.)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → gfritzsche
Priority: -- → P1
Assignee | ||
Comment 1•7 years ago
|
||
Attachment #8882042 -
Flags: review?(alessio.placitelli)
Comment 2•7 years ago
|
||
Comment on attachment 8882042 [details] [diff] [review] Allow annotating experiments with a type Review of attachment 8882042 [details] [diff] [review]: ----------------------------------------------------------------- Looks good!
Attachment #8882042 -
Flags: review?(alessio.placitelli) → review+
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 3•7 years ago
|
||
Comment on attachment 8882042 [details] [diff] [review] Allow annotating experiments with a type Approval Request Comment [Feature/Bug causing the regression]: Telemetry experiments annotations. [User impact if declined]: Data pipeline can't effectively filter out pref-flip experiments (as we can't tell them apart from pref rollouts etc.). [Is this code covered by automated tests?]: Yes. [Has the fix been verified in Nightly?]: Pending. [Needs manual test from QE? If yes, steps to reproduce]: No. [List of other uplifts needed for the feature/fix]: None. [Is the change risky?]: No. [Why is the change risky/not risky?]: Its a small optional addition to the Telemetry annotation API. [String changes made/needed]: None.
Attachment #8882042 -
Flags: approval-mozilla-beta?
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/72aa740f3363 Allow annotating experiments with a type. r=Dexter
Keywords: checkin-needed
Pushed by archaeopteryx@coole-files.de: https://hg.mozilla.org/integration/mozilla-inbound/rev/3dac05357e71 Allow annotating experiments with a type: Fix eslint issues. r=eslint-fix on a CLOSED TREE
Comment 6•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/72aa740f3363 https://hg.mozilla.org/mozilla-central/rev/3dac05357e71
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Comment 7•7 years ago
|
||
Comment on attachment 8882042 [details] [diff] [review] Allow annotating experiments with a type add type annotation for experiments, beta55+ you may want to s/lenght/length/ in experiments.rst.
Attachment #8882042 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 8•7 years ago
|
||
(In reply to Julien Cristau [:jcristau] from comment #7) > Comment on attachment 8882042 [details] [diff] [review] > Allow annotating experiments with a type > > add type annotation for experiments, beta55+ > > you may want to s/lenght/length/ in experiments.rst. Documentation is built from mozilla-central, so that typo should be fixed there. We should still be good to uplift this patch to beta. I've filed bug 1378345 for the typo.
Comment 9•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/e2dc26e7f164 https://hg.mozilla.org/releases/mozilla-beta/rev/95bd58aaf678
status-firefox55:
--- → fixed
Comment 10•7 years ago
|
||
(In reply to Georg Fritzsche [:gfritzsche] from comment #3) > [Is this code covered by automated tests?]: Yes. > [Has the fix been verified in Nightly?]: Pending. > [Needs manual test from QE? If yes, steps to reproduce]: No. Setting qe-verify- based on Georg's assessment on manual testing needs and the fact that this fix has automated coverage.
Flags: qe-verify-
You need to log in
before you can comment on or make changes to this bug.
Description
•