Add "type" key to experiment annotations

RESOLVED FIXED in Firefox 55

Status

()

enhancement
P1
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: sunahsuh, Assigned: gfritzsche)

Tracking

unspecified
mozilla56
Points:
---
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox55 fixed, firefox56 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
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

2 years ago
Assignee: nobody → gfritzsche
Priority: -- → P1
(Assignee)

Comment 1

2 years ago
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+
(Assignee)

Updated

2 years ago
Keywords: checkin-needed
(Assignee)

Comment 3

2 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?

Comment 4

2 years ago
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

Comment 5

2 years ago
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

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/72aa740f3363
https://hg.mozilla.org/mozilla-central/rev/3dac05357e71
Status: NEW → RESOLVED
Last Resolved: 2 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.