Closed Bug 835357 Opened 11 years ago Closed 10 years ago

add telemetry to measure DNT usage

Categories

(Core :: Networking, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla31
Tracking Status
firefox29 --- fixed
firefox30 --- fixed
firefox31 --- fixed

People

(Reporter: geekboy, Assigned: geekboy)

References

Details

(Whiteboard: [qa-])

Attachments

(2 files, 4 obsolete files)

Do Not Track shipped a long time ago, and I'd like to know (1) if usage measurements from telemetry line up with our seen-in-the-wild header count and (2) if anyone actually opts *in* to tracking.

Want to add a feature usage measurement to see if this UI (which has top billing in the privacy prefs panel) is used as much as I hope.
Hi Sid,
I'm interested in resolving this bug, as far I can see this should be an enumerated histogram, is that right?
I think it could just be an integer, not a histogram, that reflects the DNT state (perhaps one of -1, 0, 1 where -1 means unset and the other values are the DNT header value).  Maybe in simpleMeasurements?

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/TelemetryPing.js#194

Taras, what do we usually do to measure whether a feature is used or not?  Do you have any pointers to examples of this already deployed?
Flags: needinfo?(taras.mozilla)
I think a 3 state enum histogram works fine for this.

Eg: 0,1 are DNT values, 2==unset
Flags: needinfo?(taras.mozilla)
Attached patch DoNotTrack Usage (obsolete) — Splinter Review
It counts the status changing of the DNT, so it counts even if the user is toggling between the options. I want to know if this is the behaviour we are looking for.
Attachment #781955 - Flags: feedback?(sstamm)
Comment on attachment 781955 [details] [diff] [review]
DoNotTrack Usage

Review of attachment 781955 [details] [diff] [review]:
-----------------------------------------------------------------

Based on the logic in privacy.js, this looks like it'll work.
http://mxr.mozilla.org/mozilla-central/source/browser/components/preferences/privacy.js#152
Attachment #781955 - Flags: feedback?(sstamm) → feedback+
Should I ask for review?
Yes, absolutely.  Maybe Taras for the histograms JSON and jduell for network.

Also, do you know if the histogram will ever get reset or if we'll be able to figure a change frequency (clicks over time or something)?  I think we'll want to know if someone is fiddling with the UI a *lot* or if they've just been changing it infrequently over a long time period.
I think we need something like an event which fires once per session (preferably at the final of the session), but I don't know if this is possible -even though I heard something about an option *gather-telemetry* which should be what we need-.
Taras, some help please?
Flags: needinfo?(taras.mozilla)
Flags: needinfo?(taras.mozilla)
Attached patch DoNotTrack Usage (obsolete) — Splinter Review
Attachment #781955 - Attachment is obsolete: true
Attachment #785938 - Flags: review?(jduell.mcbugs)
Attached patch DoNotTrack Usage v2 (obsolete) — Splinter Review
I think this aproximates better the user's choice.
Attachment #785938 - Attachment is obsolete: true
Attachment #785938 - Flags: review?(jduell.mcbugs)
Attachment #786238 - Flags: review?(jduell.mcbugs)
Attached patch DoNotTrack Usage v2 (obsolete) — Splinter Review
I think this aproximates better the user's choice.
Attachment #786238 - Attachment is obsolete: true
Attachment #786238 - Flags: review?(jduell.mcbugs)
Attachment #786239 - Flags: review?(jduell.mcbugs)
Comment on attachment 786239 [details] [diff] [review]
DoNotTrack Usage v2

Review of attachment 786239 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/protocol/http/nsHttpHandler.cpp
@@ +240,5 @@
> +        Telemetry::Accumulate(Telemetry::DNT_USAGE, DONOTTRACK_VALUE_UNSET);
> +    }
> +    else {
> +        Telemetry::Accumulate(Telemetry::DNT_USAGE, mDoNotTrackValue);
> +    }

You're accumulating telemetry as HTTP is shutting down.  Ask Taras and make sure that this doesn't mean the telemetry will never get reported.  

If you need to move it somewhere else, it's probably fine to put it in nsHttpHandler::Init.  We'll get the result the next time the browser is restarted, which I assume is fine.

r- for now until this get cleared up, but otherwise looks good.  Just resubmit for review if there's no need for a new patch.
Attachment #786239 - Flags: review?(jduell.mcbugs) → review-
> You're accumulating telemetry as HTTP is shutting down.  Ask Taras and make
> sure that this doesn't mean the telemetry will never get reported.


we have saved-session telemetry. As long as you write out your telemetry probes out before "profile-before-change2" observer fires you are golden.
Flags: needinfo?
I'm not familiar with the saved-session event, but from what I've read I should add "profile-before-change2" observer and write out the telemetry probes when Observe method is called?
(In reply to Robert Bindar from comment #14)
> I'm not familiar with the saved-session event, but from what I've read I
> should add "profile-before-change2" observer and write out the telemetry
> probes when Observe method is called?

No, Taras is saying that as long as your telemetry bits are logged before profile-before-change2 fires, those bits will be reported.  You should not be adding observers for profile-before-change2.

I think ~nsHttpHandler fires before profile-before-change2, probably in xpcom-shutdown.  So you should be good.  Maybe double-check with debugger breakpoints or similar.
Flags: needinfo?
I've checked and ~nsHttpHandler() does indeed fire before profile-before-change2, as Nathan said.
Attachment #786239 - Attachment is obsolete: true
Attachment #786887 - Flags: review?(jduell.mcbugs)
Comment on attachment 786887 [details] [diff] [review]
DoNotTrack Usage v2

Review of attachment 786887 [details] [diff] [review]:
-----------------------------------------------------------------

Looks like we're good then.
Attachment #786887 - Flags: review?(jduell.mcbugs) → review+
Assignee: sstamm → robertbindar
Keywords: checkin-needed
pushed to inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/c8143d4d3602
Status: NEW → ASSIGNED
Keywords: checkin-needed
Target Milestone: --- → mozilla26
https://hg.mozilla.org/mozilla-central/rev/c8143d4d3602
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Nathan: this doesn't seem to be working.  I just navigated to about:telemetry in my Firefox and DNT_USAGE is not accumulating data and there's no data on the server.  Are the calls to Telemetry::Accumulate actually happening?
Status: RESOLVED → REOPENED
Flags: needinfo?(nfroyd)
Resolution: FIXED → ---
(In reply to Sid Stamm [:geekboy or :sstamm] from comment #21)
> Nathan: this doesn't seem to be working.  I just navigated to
> about:telemetry in my Firefox and DNT_USAGE is not accumulating data and
> there's no data on the server.  Are the calls to Telemetry::Accumulate
> actually happening?

I suspect they're happening, but ~nsHttpHandler is getting called too late in the shutdown process for the values to actually be recorded in a ping that would get sent to the server.  When does gHttpHandler get destroyed?  At xpcom-shutdown, perhaps?
Flags: needinfo?(nfroyd)
jduell?  I think something is swallowing the telemetry accumulations here.
Flags: needinfo?(jduell.mcbugs)
I don't know why the telemetry isn't showing up (on the website you need to make sure it covers the dates that you started adding the telemetry--it's not updated live and it's usually out of date--right now it goes up to Jan 8th).

nsHttpHandler is ref-counted by every HTTP channel--it's possible it stays alive longer than we thought.  gdb may be your friend in figuring that out.

Is there some canonical early shutdown event when we could do this instead of waiting for ~httpHandler?
Flags: needinfo?(jduell.mcbugs)
(In reply to Jason Duell (:jduell) from comment #24)
> I don't know why the telemetry isn't showing up (on the website you need to
> make sure it covers the dates that you started adding the telemetry--it's
> not updated live and it's usually out of date--right now it goes up to Jan
> 8th).

FWIW, It's not even appearing in my about:telemetry page (should be there if my build accumulates it).
(In reply to Sid Stamm [:geekboy or :sstamm] from comment #25)
> (In reply to Jason Duell (:jduell) from comment #24)
> > I don't know why the telemetry isn't showing up (on the website you need to
> > make sure it covers the dates that you started adding the telemetry--it's
> > not updated live and it's usually out of date--right now it goes up to Jan
> > 8th).
> 
> FWIW, It's not even appearing in my about:telemetry page (should be there if
> my build accumulates it).

We won't show histograms without any data.  And the DNT histogram won't have any data in it until ~nsHttpHandler runs.
So profile-change-net-teardown is supposed to happen prior to profile-before-change:

https://wiki.mozilla.org/XPCOM_Shutdown

profile-before-change is effectively when telemetry saves its data.  Conveniently, nsHttpHandler already listens for profile-change-net-teardown:

http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/nsHttpHandler.cpp#1781

So accumulating there ought to be enough to have telemetry save the data.  It would be good to verify by grepping through the zipped-up saved pings, though.
Is this what you were suggesting, Nathan?  Also, I don't see it showing up in about:telemetry even with this patch (and telemetry enabled).  Is there a good way to manually trigger a telemetry ping or update the data stored on disk?  I also don't see anything except shutdown time stored in my profile dir on linux.
Assignee: robertbindar → sstamm
Attachment #8394482 - Flags: feedback?(nfroyd)
Comment on attachment 8394482 [details] [diff] [review]
move histogram accumulation earlier

Review of attachment 8394482 [details] [diff] [review]:
-----------------------------------------------------------------

Yeah, this is what I was thinking.

You're not going to see anything in about:telemetry during the session because bits are only going to get added to the relevant histogram during shutdown.  And there's no good way to see if the data gets added without shutting down your browser, as manually sending profile-net-change-teardown is going to bork a lot of things.

Once you shut down your browser in a build with MOZILLA_OFFICIAL=1 MOZILLA_TELEMETRY_REPORTING=1 set in your mozconfig, you should see a gzip'd ping in ${profile}/saved-telemetry-pings/${blah} that should have your saved data in it.
Attachment #8394482 - Flags: feedback?(nfroyd) → feedback+
Thanks Nathan, yep, that worked.  FWIW, I updated my debug build to use:

 export MOZILLA_OFFICIAL=1
 export MOZ_TELEMETRY_REPORTING=1

And the ping was not gzip'd (appeared to be just raw JSON).  DNT_USAGE histogram was present in there.
Comment on attachment 8394482 [details] [diff] [review]
move histogram accumulation earlier

Jason: Since you reviewed the initial patch, can you review this minor change?
Attachment #8394482 - Flags: review?(jduell.mcbugs)
Attachment #8394482 - Flags: review?(jduell.mcbugs) → review+
Thanks, mcmanus.

https://hg.mozilla.org/integration/mozilla-inbound/rev/42143f49bf7d

We should probably uplift this as far as possible, but I'll let it bake on inbound for the day before requesting uplift.
https://hg.mozilla.org/mozilla-central/rev/42143f49bf7d
Status: REOPENED → RESOLVED
Closed: 11 years ago10 years ago
Resolution: --- → FIXED
Target Milestone: mozilla26 → mozilla31
Comment on attachment 8394482 [details] [diff] [review]
move histogram accumulation earlier

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Original landing of this bug (26) didn't work -- no telemetry accumulated.
User impact if declined: we can't measure dnt usage in aurora/beta right now to see if the new UI is worthwile (I think it's not).
Testing completed (on m-c, etc.): It's on m-c, and we're getting data in the telmetry dashboard.
Risk to taking this patch (and alternatives if risky): really low risk.  Only minor changes to where the telemetry histogram is accumulated. 
String or IDL/UUID changes made by this patch: None.

We're getting data now, we should uplift this to fix the versions of Firefox that have the old patch but not the new one (all of them) so we can actually learn from this telemetry.
Attachment #8394482 - Flags: approval-mozilla-beta?
Attachment #8394482 - Flags: approval-mozilla-aurora?
Attachment #8394482 - Flags: approval-mozilla-beta?
Attachment #8394482 - Flags: approval-mozilla-beta+
Attachment #8394482 - Flags: approval-mozilla-aurora?
Attachment #8394482 - Flags: approval-mozilla-aurora+
Given this has been in release for several weeks I'm assuming this doesn't need QA verification. Sorry for not getting to this sooner.
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.