Closed
Bug 636085
Opened 14 years ago
Closed 14 years ago
generate_update_statistics should only report Sparkle pings
Categories
(Camino Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: alqahira, Assigned: alqahira)
Details
Attachments
(1 file)
9.79 KB,
patch
|
ss
:
review+
|
Details | Diff | Splinter Review |
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•14 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.
Assignee | ||
Comment 2•14 years ago
|
||
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™ ;)
Assignee | ||
Comment 3•14 years ago
|
||
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)
Assignee | ||
Comment 4•14 years ago
|
||
(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•14 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•14 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•14 years ago
|
||
Comment on attachment 515177 [details] [diff] [review]
Exclude non-sparkle pings
r=me
Attachment #515177 -
Flags: review?(samuel.sidler) → review+
Assignee | ||
Comment 8•14 years ago
|
||
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
Closed: 14 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.
Description
•