Limit the number of Suggest impression cap reset telemetry events recorded at once
Categories
(Firefox :: Address Bar, task, P1)
Tracking
()
People
(Reporter: adw, Assigned: adw)
References
Details
Attachments
(2 files)
See bug 1761058 comment 9. For impression cap telemetry, we record reset events for every single elapsed period of time starting from when caps are first introduced. That's by design, but one consequence is that we record events even for periods where Firefox wasn't running. For example if there's an hourly cap and you start Firefox, quit, and don't restart Firefox for a week, then once you do start Firefox again we'll end up recording 24 * 7 = 168 reset events all at once. That might hang Firefox, and even if it didn't I'm not sure we want events corresponding to periods where Firefox wasn't running.
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 1•3 years ago
|
||
This records multiple reset "events" inside a single telemetry event instead of
using one telemetry event per reset event like we currently do. It also stops
recording reset events for interval periods that elapsed while the app wasn't
running. This will prevent us from recording a bunch of events at once like we
currently do. Please see the bug for more background.
A new eventCount
property in the telemetry event's extra
indicates the
number of reset events that are being reported in the telemetry event.
I talked with Rebecca about these changes and she's OK with them.
Assignee | ||
Comment 2•3 years ago
|
||
Currently we record reset telemetry only when the user performs a search. When I
talked with Rebecca today about the part-1 patch, she mentioned we ought to have
a better guarantee about when this telemetry is recorded. Many users don't
perform searches very often, so we may miss out on this telemetry for them.
There are a couple of ways to mitigate it: Record telemetry periodically and on
shutdown, and this patch does both. I chose one hour for the periodic reporting
period, which seems not too long and not too short.
Now that we are recording periodically, I also moved the existing
_resetElapsedImpressionCounters()
call from when any search starts (in
startQuery()
) to when suggestions are triggered (in _canAddSuggestion()
),
which is right before the impression counters are examined to determine if the
suggestion has hit the cap.
So in summary, we'll now record reset telemetry (when necessary) in these cases:
- When the user triggers a suggestion
- Every hour
- On shutdown
Assignee | ||
Comment 3•3 years ago
•
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f62e2562b80a5bbf494dbbc4316661c6b7d87fb2
https://treeherder.mozilla.org/#/jobs?repo=try&revision=06a17d782007425253310bad7ce82130e3d564f8
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c8e81763ca1c9b234d55e7af4bfee5b4e17300e3
https://treeherder.mozilla.org/#/jobs?repo=try&revision=01a33f86059077962f37a88806d103ff5ae3ba2c
Comment 5•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/4211f9ae0855
https://hg.mozilla.org/mozilla-central/rev/639f39489cfe
Assignee | ||
Comment 6•3 years ago
|
||
Hi Cosmin, I finally wrote some STR for this. Could you verify it if you have time please?
STR prerequisites
Set these prefs to enable caps:
browser.urlbar.quicksuggest.impressionCaps.nonSponsoredEnabled = true
browser.urlbar.quicksuggest.impressionCaps.sponsoredEnabled = true
Use the following snippet to set up some caps. Please run the snippet as part of step 1 in the STR.
(function() {
Cu.import("resource:///modules/UrlbarQuickSuggest.jsm");
UrlbarQuickSuggest._setConfig({
impression_caps: {
sponsored: {
custom: [
{ interval_s: 1, max_count: 1 },
],
},
nonsponsored: {
custom: [
{ interval_s: 1, max_count: 1 },
],
},
},
});
})();
FYI the snippet sets up the following caps:
- Sponsored:
- 1 per 1 second
- Non-sponsored:
- 1 per 1 second
STR
- Run the snippet above to define the caps.
- Wait 5 or 10 seconds. The exact number isn't important but please keep track of approximately how many seconds you waited.
- Type something to trigger a Suggest suggestion, e.g. "am" to trigger the Amazon suggestion. Try to trigger the suggestion only once, i.e., type the smallest string you need to trigger the suggestion (see NOTE below). No need to click it.
- Open about:telemetry#events-tab_search=impression_cap (events tab, search for "impression_cap")
- Verify at least 2 events appear with the following:
- category: contextservices.quicksuggest
- method: impression_cap
- object: reset
- extra:
- One event should have
"type":"sponsored"
- The other event should have
"type":"nonsponsored"
- For both events,
"eventCount"
should be the approximate number of seconds you waited in step 2. It's OK if it's not exactly right but it should be close.
- One event should have
NOTE: If you ended up triggering the suggestion more than once in step 3, other "reset" events may appear. That's OK. If they do, they will probably have "eventCount":"1"
in their extra
. The important thing to verify is that 2 events are recorded with "eventCount"
approximately equal to the number of seconds you waited in step 2.
Comment 7•3 years ago
|
||
We have verified this issue on the latest Nightly 102.0a1 (Build ID: 20220508190220) and the latest Beta 101.0b4 (Build ID: 20220508185621) on Windows 10 x64, macOS 10.15.7 and Ubuntu 20.04 x64.
- In order to verify this issue we have used the STR described in comment 6.
Description
•