generate_update_statistics should only report Sparkle pings

RESOLVED FIXED

Status

Camino Graveyard
General
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Smokey Ardisson (offline for a while; not following bugs - do not email), Assigned: Smokey Ardisson (offline for a while; not following bugs - do not email))

Tracking

Details

Attachments

(1 attachment)

I thought we were already doing this, but I guess not; tonight I noticed (because they had broken syntax which affected the 2.0 l10n columns) pings from some Verizon FIOS customer in DC using Camino itself with bogus OS versions and &-escaped &s in the query string.

And, indeed, on the same day (19 Feb), there were 693 pings that didn't have Sparkle in the UA.

Comment 1

7 years ago
This should be as easy as adding...

  grep "Sparkle"

... just before or after each of the...

  grep " /update-check?os="

... lines throughout generate_update_statistics (with the appropriate pipe). I haven't tested it, but there's no reason it should be any harder than that.
Yeah, I planned on using "Sparkle/".  I filed this mostly because I didn't have time to really mess with it that night, and also because I wondered if there were a way to do it without triggering Yet Another Grep Invocation™ ;)
Created attachment 515177 [details] [diff] [review]
Exclude non-sparkle pings

I opted to put it just before the grep " /update-check?os=" check, for two reasons:

1) It keeps all of the "used-everywhere" parts of the code in the same place/order in every check, and 
2) It keeps both "let's exclude stuff" checks (badIPs and Sparkle) adjacent to each other.

I spot-checked Feb 21 and the difference between the total pings generated earlier in the week and the total pings generated with my changes are exactly the same as the number of pings matching all the other criteria but missing Sparkle (i.e., using grep -v "Sparkle/").
Assignee: nobody → alqahira
Status: NEW → ASSIGNED
Attachment #515177 - Flags: review?(samuel.sidler)
(In reply to comment #2)
> were a way to do it without triggering Yet Another Grep Invocation™ ;)

Ideally, I think, we'd want to factor out the 

grep "$date" $file | grep -Ev "$badips" | grep "Sparkle/"

and 

grep "$date" $file | grep -Ev "$badips" | grep "Sparkle/" | grep " /update-check?os="

things that are common and used all over and only run those once, rather than on every stat.  I haven't figured out how to do that, though (and it's really another bug :P ).

Comment 5

7 years ago
(In reply to comment #4)
> I haven't figured out how to do that, though

Convert this to a Real Language, instead of bash? ;)

I can try to rubyify this at some point if you like.
OS: Mac OS X → Windows 7

Comment 6

7 years ago
(In reply to comment #4)
> things that are common and used all over and only run those once, rather than
> on every stat.  I haven't figured out how to do that, though (and it's really
> another bug :P ).

As Stuart says, we can convert it to a Real Language like Python (see what I did there?), but in the mean time, we can make the things you want to factor out into one big variable (e.g. $everything and $everything_with_update-check) and simply define it after the other variables. Messy and ugly, but it'll save time when we have updates like this in the future (though... it's rare enough and find/replace works well).

Comment 7

7 years ago
Comment on attachment 515177 [details] [diff] [review]
Exclude non-sparkle pings

r=me
Attachment #515177 - Flags: review?(samuel.sidler) → review+
http://hg.mozilla.org/camino/rev/d52651dc5f21

(In reply to comment #6)
> did there?), but in the mean time, we can make the things you want to factor
> out into one big variable (e.g. $everything and $everything_with_update-check)
> and simply define it after the other variables.

That doesn't actually work; I tried that before making comment 4 ;)

Filed bug 637091 on the Real Language stuff ;-)
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
OS: Windows 7 → Mac OS X
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.