Closed Bug 1587579 Opened 10 months ago Closed 10 months ago

Add Normandy capability listing for available JEXL context variables

Categories

(Firefox :: Normandy Client, enhancement, P1)

71 Branch
enhancement

Tracking

()

RESOLVED FIXED
Firefox 71
Tracking Status
firefox70 + fixed
firefox71 --- fixed

People

(Reporter: mythmon, Assigned: mythmon)

References

(Blocks 1 open bug)

Details

(Whiteboard: [rca - requirement])

Attachments

(1 file)

The capabilities of Normandy should also include the list of top level properties accessible to recipe filter expressions, such as normandy.clientId. Most of the time these kind of properties are safe to send to old clients, which is why the were not originally included in the list of capabilities. In some cases they can cause problems though, and so we should add them.

For example, normandy.locale == "en-US" will always be safe because if locale is not available in the filter context, it will be undefined. However, something like

[normandy.clientId, "global-v1"]|bucketSample(0, 100, 1000)

is not safe if clientId is not available. This is because [undefined, "global-v1"] will hash to some value, and then either 100% of users or 0% of users will be enrolled in the recipe, depending on the other parameters. This is clearly not ideal.

Blocks: 1587608
Pushed by mcooper@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/614d53e0eb85
Add Normandy capability listing for available JEXL context variables r=Gijs

Comment on attachment 9100012 [details]
Bug 1587579 - Add Normandy capability listing for available JEXL context variables r?Gijs

Beta/Release Uplift Approval Request

  • User impact if declined: Without these changes, users of Firefox 70 may fail to receive Normandy recipes that target them, impacting experiments, rollouts, and hotfixes. Alternatively, without this patch our ability to correctly reason about Normandy's backwards-compatibility is severely hampered. To solve this we would have to make more drastic changes in the future.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This patch marks Normandy as having features that it already has, without impacting any existing features or compatibility markers. If these changes fail, the likely effect is equivalent to not taking the patch.
  • String changes made/needed: None
Attachment #9100012 - Flags: approval-mozilla-beta?
Status: ASSIGNED → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 71

Comment on attachment 9100012 [details]
Bug 1587579 - Add Normandy capability listing for available JEXL context variables r?Gijs

Fx70 is on release now - moving the request accordingly.

Attachment #9100012 - Flags: approval-mozilla-beta? → approval-mozilla-release?

Well, too late to make it into 70 release. We can try for a dot release though.

Comment on attachment 9100012 [details]
Bug 1587579 - Add Normandy capability listing for available JEXL context variables r?Gijs

Make sure all Normandy capabilities are reachable. OK for 70 uplift.

Attachment #9100012 - Flags: approval-mozilla-release? → approval-mozilla-release+

This bug has been identified as part of a pilot on determining root causes of blocking and dot release drivers.

It needs a root-cause set for it. Please see the list at https://docs.google.com/document/d/1FFEGsmoU8T0N8R9kk-MXWptOPtXXXRRIe4vQo3_HgMw/.

Add the root cause as a whiteboard tag in the form [rca - <cause> ] and remove the rca-needed keyword.

If you have questions, please contact :tmaity.

Keywords: rca-needed

The root cause here was a missing requirement. This change was identified late in the development of this feature. It needed to land in the same version as bug 1584368, to avoid future compatibility problems. We landed bug 1584368 without realizing this was needed. At that point, the change was to either back it out or rush this change, and we chose the latter.

Keywords: rca-needed
Whiteboard: [rca - requirement]
You need to log in before you can comment on or make changes to this bug.