Rename the TelemetryEvent class to better reflect its broader scope
Categories
(Firefox :: Address Bar, task, P1)
Tracking
()
People
(Reporter: adw, Assigned: scunnane)
References
Details
(Whiteboard: [sng-scrubbed])
TelemetryEvent
does more than record telemetry events: It also notifies providers of engagements, abandonments, and the start of search sessions. After bug 1827762 this is especially important because UrlbarProvider.onEngagement()
is now the preferred way to handle result engagements that require actions other than loading a URL.
Additionally, Wil is working on exposure telemetry and he'll be modifying TelemetryEvent
to keep a list of all results that were shown to the user during a search session. It's natural for TelemetryEvent
to be the component that keeps this list since it's the place that records the start and end of sessions.
To better reflect this broader scope, I propose the following renames:
TelemetryEvent
->SearchSession
record()
->end()
didEnd()
might be a better name because end()
makes it sound like the class is in charge of ending the session rather than being the entry point for reacting to the fact it did end. Similarly, didStart()
might be better than start()
.
Marco and Dão, wdyt?
(This bug is spun out from bug 1827762, see comments in D174941)
Comment 2•2 years ago
|
||
I think the renaming makes sense, its scope changed from when it was created
I don't have a strong opinion over those method names. I think start
is not problematic. For end
maybe a more verbose .concludeAndNotify
?
what about discard
, will it remain the same?
Comment 3•2 years ago
|
||
(In reply to Drew Willcoxon :adw from comment #0)
To better reflect this broader scope, I propose the following renames:
TelemetryEvent
->SearchSession
I'm not particularly thrilled about this suggestion, partly because of the concern that Mark raised. Perhaps also worth pointing out that most of our related classes start with Urlbar
. How about UrlbarSession
? UrlbarSearchSession
?
Reporter | ||
Comment 4•2 years ago
|
||
UrlbarSession
is better, I like it! (For posterity, Mark suggested "search" might be mistaken for search service type stuff, a good point)
It would be nice to move it to its own file too IMO.
(In reply to Marco Bonardo [:mak] from comment #2)
I don't have a strong opinion over those method names. I think
start
is not problematic. Forend
maybe a more verbose.concludeAndNotify
?
endAndNotify
?
what about
discard
, will it remain the same?
I can't think of a better name and it seems OK to me but definitely open to ideas.
Comment 5•2 years ago
|
||
UrlbarSession
sounds good to me too!
And also, I agree that implementing in a new JS file.
Assignee | ||
Updated•1 year ago
|
Updated•1 year ago
|
Updated•1 year ago
|
Description
•