Closed Bug 1376599 Opened 3 years ago Closed 3 years ago

Add "type" key to experiment annotations

Categories

(Toolkit :: Telemetry, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox55 --- fixed
firefox56 --- fixed

People

(Reporter: sunahsuh, Assigned: gfritzsche)

References

Details

Attachments

(1 file)

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: nobody → gfritzsche
Priority: -- → P1
Attachment #8882042 - Flags: review?(alessio.placitelli)
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+
Keywords: checkin-needed
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
https://hg.mozilla.org/mozilla-central/rev/72aa740f3363
https://hg.mozilla.org/mozilla-central/rev/3dac05357e71
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
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+
(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.
Depends on: 1378345
(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.