The default bug view has changed. See this FAQ.

Telemetry.canSend check doesn't prevent pings from being sent

RESOLVED FIXED in Firefox 14

Status

()

Toolkit
Telemetry
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: (dormant account), Assigned: froydnj)

Tracking

unspecified
mozilla16
Points:
---

Firefox Tracking Flags

(firefox14 fixed, firefox15 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
10:48 <@taras> looking at the code
10:48 <@taras> the canSend check
10:48 <@taras> causes us to still send the data
10:48 <@taras> but as a test-ping :)
10:51 <@taras> so the 404, wont be noticed
10:51 <@taras> 1/second thing is weird though
10:51 <@taras> must be some coincidence in our browser-idle scheduling + when devs go to work
(Reporter)

Updated

5 years ago
Depends on: 758193
Depends on: 762620
No longer depends on: 758193
(Assignee)

Comment 1

5 years ago
Created attachment 631080 [details] [diff] [review]
be more careful with canSend logic
Attachment #631080 - Flags: review?(taras.mozilla)
(Reporter)

Comment 2

5 years ago
Comment on attachment 631080 [details] [diff] [review]
be more careful with canSend logic

Logic one can actually follow
Attachment #631080 - Flags: review?(taras.mozilla) → review+
(Reporter)

Comment 3

5 years ago
Lets uplift this into aurora/beta
(Assignee)

Comment 4

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/ea069bf72d62
Status: NEW → ASSIGNED
OS: Windows 7 → All
Hardware: x86_64 → All
(Assignee)

Comment 5

5 years ago
Comment on attachment 631080 [details] [diff] [review]
be more careful with canSend logic

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 722240
User impact if declined: Decreased battery life and excess internet traffic.
Testing completed (on m-c, etc.): Going on to m-c tomorrow, 8 June.
Risk to taking this patch (and alternatives if risky): Always a risk with telemetry patches that something can go wrong, but this seems pretty safe.
String or UUID changes made by this patch: None.
Attachment #631080 - Flags: approval-mozilla-beta?
Attachment #631080 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 6

5 years ago
Actually, how much of a problem is this, really?  Telemetry.canSend should only ever be false for *non*-official builds (developer builds, Iceweasel, etc.) so this shouldn't be a problem on our official builds.
(Assignee)

Comment 7

5 years ago
That is, bug 758193 would seem to be the aurora/beta candidate here, not this bug.
(Assignee)

Comment 8

5 years ago
Comment on attachment 631080 [details] [diff] [review]
be more careful with canSend logic

Removing a? flags until we've discussed comment 6 and comment 7 in some more detail.
Attachment #631080 - Flags: approval-mozilla-beta?
Attachment #631080 - Flags: approval-mozilla-aurora?
(Reporter)

Comment 9

5 years ago
(In reply to Nathan Froyd (:froydnj) from comment #6)
> Actually, how much of a problem is this, really?  Telemetry.canSend should
> only ever be false for *non*-official builds (developer builds, Iceweasel,
> etc.) so this shouldn't be a problem on our official builds.

It's not much of a problem. Fennec team is doing development on the beta branch, so it would reduce traffic from developer devices. Indeed, there isn't any impact on official builds. Beta/aurora uplift is a nice to have.
https://hg.mozilla.org/mozilla-central/rev/ea069bf72d62

(Merged by Ed Morley)
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
(Assignee)

Comment 11

5 years ago
Comment on attachment 631080 [details] [diff] [review]
be more careful with canSend logic

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 722240
User impact if declined: Decreased battery life, extra data usage.  Note that per comment 9, this is only a concern for Mozilla developers if they are developing off aurora/beta.
Testing completed (on m-c, etc.): Local testing, on m-c as of today.
Risk to taking this patch (and alternatives if risky): Epsilon.
String or UUID changes made by this patch: None.
Attachment #631080 - Flags: approval-mozilla-beta?
Attachment #631080 - Flags: approval-mozilla-aurora?
Comment on attachment 631080 [details] [diff] [review]
be more careful with canSend logic

Won't affect official builds - approved for branches.
Attachment #631080 - Flags: approval-mozilla-beta?
Attachment #631080 - Flags: approval-mozilla-beta+
Attachment #631080 - Flags: approval-mozilla-aurora?
Attachment #631080 - Flags: approval-mozilla-aurora+
(Assignee)

Comment 13

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/131e571d4959
https://hg.mozilla.org/releases/mozilla-beta/rev/7912483cf789
To clarify based on a discussion on IRC: This *DOES* affect official builds because Fennec (XUL and Mobile) have (unintentionally it seems) have had telemetry disabled for a while.

With the bug, these devices were still able to send test-pings (disastrously so due to bug 762620). With the fix, the official builds for those devices will be affected and will no longer send any telemetry (but this is what we want).

Updated

5 years ago
status-firefox14: --- → fixed
status-firefox15: --- → fixed
You need to log in before you can comment on or make changes to this bug.