1312499 - figure out how to match multiple substrings in one rule field

Status: RESOLVED FIXED

(Release Engineering Graveyard :: Applications: Balrog (backend), defect, P2)

Description

[[e-mail] (:bhearsum)]

https://bugzilla.mozilla.org/show_bug.cgi?id=1311515#c31 has presented a use case for having a single Rule match multiple sections of an individual field (%SYSTEM_CAPABILITIES% in this case) at the same time:
> So the concrete example that we might need for bug 1296630 is to *not* send
> a particular update to systems that meet *all* of the following conditions:
>  * GenuineIntel
>  * family 6 model 61 stepping 4
>  * microcode version is 0x19 or less (or, alternatively, microcode version
> is one of: 0xe, 0x11, 0x12, 0x13, 0x16, 0x18, 0x19)


We currently support looking for multiple things in one field, but they're all joined together with OR. Eg: You can specify a comma-separated list of %OS_VERSION% values to match in one rule, and if _any_ of them match, the rule matches.

The simplest way to go about this would be to add some support for operators in some fields, and do things like "foo && bar" or "(foo && bar) || baz" - but I'd really like to avoid this. We're already in a situation where Balrog's rules are very tricky to navigate, and adding the ability to do arbitrary queries like this will only make things worse.

I don't have a great alternative idea at the moment, unfortunately.

Comment 1

[Robert Strong (they/them - no direct email)]

We've also had the youtube unblocker add-on intentionally break update via default preferences so we're going to move away from a model where an add-on can break update so even if system add-ons were reliably deployed to clients they won't be able to help in the future.

If I can help with this let me know and I will do what I can.

Comment 2

[Robert Strong (they/them - no direct email)]

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #1)
> We've also had the youtube unblocker add-on intentionally break update via
> default preferences so we're going to move away from a model where an add-on
> can break update so even if system add-ons were reliably deployed to clients
> they won't be able to help in the future.
I had my head a little too deep into the changes I'm making and forgot that I'm planning on adding a new optional field to the update url that add-ons could change without braking app update. This won't solve the problem with system add-ons not being deployed to a high percentage of users though and this bug will make it much simpler to handle late changes in the release cycle such as what happened in bug 1311515 as well as handling these type of requests after a release.

Comment 3

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

I wish we have this functionality in place soon. Please see https://bugzilla.mozilla.org/show_bug.cgi?id=1311515#c93 as an example of a real issue we ran into after 50.1.0 went live.

In general, we would like to be able to mitigate CPU specific crashes that don't need a fix on our side, by holding back affected users to the older version of the release and prevent them from updating to the new version of the release. So in the example above, any end-user that has an AMD CPU family that is experiencing the crash in 50.1.0, stays at 50.0.2 and is not updated to 50.1.0.

Comment 4

[[e-mail] (:bhearsum)]

(In reply to Ritu Kothari (:ritu) from comment #3)
> I wish we have this functionality in place soon. Please see
> https://bugzilla.mozilla.org/show_bug.cgi?id=1311515#c93 as an example of a
> real issue we ran into after 50.1.0 went live.
> 
> In general, we would like to be able to mitigate CPU specific crashes that
> don't need a fix on our side, by holding back affected users to the older
> version of the release and prevent them from updating to the new version of
> the release. So in the example above, any end-user that has an AMD CPU
> family that is experiencing the crash in 50.1.0, stays at 50.0.2 and is not
> updated to 50.1.0.

I'll see what I can do about prioritizing this for Q1. We need a bit more than just this bug to support those cases though - we actually need to alter the update ping, too. It would actually be really helpful to have a fully spelled out list of things you want to be able to route updates based on. CPU & GPU information are the obvious ones, but is there other hardware or software information that we should be adding to the update ping?

Comment 5

[Marco Castelluccio [:marco]]

List of addons and/or modules (DLLs) might be useful, but could also make the ping much bigger.

Comment 6

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Philipp, Nick, please see comment 4. Is there any additional information we ought to capture in the update pings so as to mitigate some kinds of external crashes by holding back users on a previous dot/mainline release?

Comment 7

[[:philipp]]

i can certainly agree that cpu & gpu (vendor, adapter id, driver) info would be good to have in any case.

some big startup crash regressions on top of my head that would have benefited from being able to selectively disabling updates when certain modules (+ in particular versions) were present, like marco mentioned in comment #5, were things like bug 1235537 or bug 1233237, but not sure if that would be worth the efforts & adding more payload to the pings...

other than that i don't think something fundamental is missing (os version and locale is already covered in the current system i suppose)

Comment 8

[Nicholas Nethercote [inactive]]

I don't have anything to add beyond what Marco and Philipp said. bsmedberg might have some ideas.

Comment 9

[Benjamin Smedberg]

I have nothing to add. I seriously doubt that adding module info to the update ping is the right tradeoff in terms of request size or processing overhead; it would probably turn the GET into a POST.

Comment 10

[[e-mail] (:bhearsum)]

If we do add even more information to the update URL, we should move that out to another bug.

Comment 11

[[e-mail] (:bhearsum)]

Matt is going to take a look at this.

Comment 12

[[github robot]]

Commit pushed to master at https://github.com/mozilla/balrog

https://github.com/mozilla/balrog/commit/946ccd1d7420e1bb3092024a7405bad8437e4145
Bug 1312499 - figure out how to match multiple substrings in one rule field (#280). r=bhearsum

Comment 13

[[e-mail] (:bhearsum)]

This made it to production today. Great work Matt!