The default bug view has changed. See this FAQ.

Add telemetry probe for the opening time of the site identity popup

RESOLVED FIXED in Firefox 13

Status

()

Firefox
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jaws, Assigned: jaws)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 13
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Comment hidden (empty)
Summary: Add telemetry probe for the opening time of the site identity block → Add telemetry probe for the opening time of the site identity popup
Blocks: 671038
Created attachment 593458 [details] [diff] [review]
Patch for bug 723090

Dietrich: With this patch applied, I get a lot of 0 millisecond duration data points added to the histogram for times when the site identity popup hasn't been opened.

I'm not sure if it is something wrong with my computer or with the patch that I have written. Can you apply the patch and see if you get the same issue?
Attachment #593458 - Flags: feedback?(dietrich)
Comment on attachment 593458 [details] [diff] [review]
Patch for bug 723090

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

I don't see anything that would cause values of 0 to be submitted. That is something you could control at the point of recording, but that shouldn't happen if the popup isn't actually being shown. You could do some debugging to see if that codepath is actually being executed, and you could also ask Taras about it to see if it's some kind of expected Telemetry behavior?

::: browser/base/content/browser.js
@@ +8207,5 @@
> +    try {
> +      Services.telemetry.getHistogramById("FX_IDENTITY_POPUP_OPEN_MS").add(openingDuration);
> +    } catch (ex) {
> +      Components.utils.reportError("Unable to report telemetry for FX_IDENTITY_POPUP_OPEN_MS.");
> +    }

for completeness, should null out _popupOpenTime here.
Attachment #593458 - Flags: feedback?(dietrich)
Created attachment 593769 [details] [diff] [review]
Patch for bug

I debugged my previous issue and it was because I added the new histogram in the middle of the others without doing a clobber build. In other words, the issue I was seeing won't actually happen in practice.

I talked with Taras and Nathan Froyd about the strings being used here and they asked that the description strings mention the units of measurement for time deltas. They also said that the data association of preexisting telemetry histograms will not break if only the strings are updated.
Attachment #593458 - Attachment is obsolete: true
Attachment #593769 - Flags: review?(dietrich)
Comment on attachment 593769 [details] [diff] [review]
Patch for bug

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

looks good, r=me. thanks!
Attachment #593769 - Flags: review?(dietrich) → review+
Thanks Dietrich!

https://hg.mozilla.org/integration/fx-team/rev/5f3eedc770fa
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/5f3eedc770fa
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 13
You need to log in before you can comment on or make changes to this bug.