Bug 1442326 - Do not filter Android sync event pings anymore #388
Conversation
Codecov Report
@@ Coverage Diff @@
## master #388 +/- ##
==========================================
+ Coverage 69.34% 69.37% +0.03%
==========================================
Files 46 46
Lines 3875 3873 -2
Branches 140 140
==========================================
Hits 2687 2687
+ Misses 1188 1186 -2
Continue to review full report at Codecov.
|
|
This should be a bit more conservative to prevent reading through excess data. |
| @@ -72,8 +72,6 @@ object SyncEventView { | |||
| case "4" | "5" => true | |||
| }.where("docType") { | |||
| case "sync" => true | |||
| }.where("appName") { | |||
| case "Firefox" => true | |||
acmiyaguchi
Mar 1, 2018
Contributor
We may not want to include pings that get put into the OTHER bucket. I suggest using something like case "Firefox" | "Fennec".
We may not want to include pings that get put into the OTHER bucket. I suggest using something like case "Firefox" | "Fennec".
thomcc
Mar 1, 2018
Contributor
Worth noting deleting it brings it in line with Sync{,Flat}View, see https://github.com/mozilla/telemetry-batch-view/blob/master/src/main/scala/com/mozilla/telemetry/views/SyncFlatView.scala#L70-L79, https://github.com/mozilla/telemetry-batch-view/blob/master/src/main/scala/com/mozilla/telemetry/views/SyncView.scala#L69-L78
Worth noting deleting it brings it in line with Sync{,Flat}View, see https://github.com/mozilla/telemetry-batch-view/blob/master/src/main/scala/com/mozilla/telemetry/views/SyncFlatView.scala#L70-L79, https://github.com/mozilla/telemetry-batch-view/blob/master/src/main/scala/com/mozilla/telemetry/views/SyncView.scala#L69-L78
|
It seems like this is consistent with the rest of the sync code, so it looks good to me. |
Firefox Android sends pings with
appName: "Fennec".Let's remove that filter on the SyncEventView and bring it in line with the other views.