Update profile.jsm to report profile reset times

RESOLVED DUPLICATE of bug 1063710

Status

Firefox Health Report
Client: Desktop
RESOLVED DUPLICATE of bug 1063710
4 years ago
4 years ago

People

(Reporter: adw, Assigned: markh)

Tracking

unspecified
Points:
3
Dependency tree / graph
Bug Flags:
firefox-backlog +
qe-verify +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [fxgrowth])

(Reporter)

Description

4 years ago
Broken down from bug 860238.

profile.jsm should be updated to report all the times that the user's profile was reset/migrated.

http://mxr.mozilla.org/mozilla-central/source/services/healthreport/profile.jsm
(Reporter)

Updated

4 years ago
Depends on: 1063710

Updated

4 years ago
Flags: qe-verify?
Flags: firefox-backlog+
(Reporter)

Updated

4 years ago
Points: --- → 3
(Reporter)

Updated

4 years ago
Flags: qe-verify? → qe-verify+

Updated

4 years ago
Assignee: nobody → mhammond
Status: NEW → ASSIGNED
Iteration: --- → 35.2

Updated

4 years ago
QA Contact: kamiljoz

Updated

4 years ago
Whiteboard: [fxgrowth]
(Assignee)

Comment 1

4 years ago
This is closely related to the feedback request in bug 1063710, and please excuse my ignorance about FHR etc - but my quick searching shows services/metrics/dataprovider.jsm doesn't really have the concept of an array of numeric values.  Would using setLastNumeric() to record only the most-recent reset time be OK for this bug (leaving the option open to get the full reset history later if desired)?  If not, can anyone suggest how this should be reported?
Flags: needinfo?(benjamin)
Flags: needinfo?(adw)
(Reporter)

Comment 2

4 years ago
I don't know either but surely it's possible, if not now then to add it.  Otherwise maybe the number of resets plus the last reset time would be fine.
Flags: needinfo?(adw)

Comment 3

4 years ago
How would we even know "all the times"? FHR data doesn't persist across profile resets (AFAIK), so I figured the only data we would have is that this new profile is the result of a profile-reset, and hopefully the date of that reset. Everything else seems like overkill. Also note that we cut off data collection at 180 days anyway.

So yes, we should just store this as a single scalar datapoint.
Flags: needinfo?(benjamin)
(Reporter)

Comment 4

4 years ago
That's the point of bug 1063714, as suggested by Richard in bug 860238 comment 1.  But if we don't want all the times, great, this is easier.
(Assignee)

Comment 5

4 years ago
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #3)
> How would we even know "all the times"? FHR data doesn't persist across
> profile resets (AFAIK),

Just to expand on what Drew said, bug 1063710 is working on ensuring the date of resets (and the original profile creation date) is persisted across the profile resets - so we *can* get this data if it is useful.

> so I figured the only data we would have is that
> this new profile is the result of a profile-reset, and hopefully the date of
> that reset. Everything else seems like overkill. Also note that we cut off
> data collection at 180 days anyway.

With bug 1063710, ISTM the most useful data might be 2 values - the number of resets and the average number of days between resets (with a cap on the number of resets we track - say 20 or so).

Benjamin, can you clarify if that's reasonable/useful, or if you really do just want the date of the most recent reset and nothing else?  (And while you are here, there is a feedback request for you on bug 1063710 which is asking you if we do actually want to track the original profile creation date across resets)
Flags: needinfo?(benjamin)

Comment 6

4 years ago
From the high-level perspective, all I really care about is answering the following questions:

* have FHR be notified that the "old" profile is going away so we can mark the record as dead
* have the "new" record have a marker that it is a result of a profile reset, and the date of that reset

If you have other questions that you'd like to answer from the data, you can add additional fields, but in terms of having accurate active-user counts, having the date of the profile reset in the new FHR record is sufficient.

In the original breakdown, was there a bug about closing out the "old" FHR record?
Flags: needinfo?(benjamin)
I don't think we want to close an old record and start a new one.

FHR is about health. If a user is resetting their profile to fix something, we want the data from before the fix, then the event that they reset, then the data after the fix.

For example, if startup is slow, we want to know if it's faster after the reset. If an add-on is causing crashes, and isn't present after the reset, that's important to know.

Indeed, we want to know if the same user is resetting multiple times, how long since their profile was created, whether they reset every six months, whether there are characteristics that cause a user to reset...

Comment 8

4 years ago
I agree with Richard in comment #7. We want to be able to see what kind of effect a reset is having.
In addition, if  this is recorded, also record a reason for reset e..g browser is slow to start, too crashy etc. That will help direct analyses in  what to compare between the pre-reset and post-reset periods.

That said, my expectation of a reset (as a user) is to _not_ tie my past and future profile data together. It feels like tiresome data collection.
(In reply to Richard Newman [:rnewman] from comment #7)
> FHR is about health. If a user is resetting their profile to fix something,
> we want the data from before the fix, then the event that they reset, then
> the data after the fix.
> 
> For example, if startup is slow, we want to know if it's faster after the
> reset. If an add-on is causing crashes, and isn't present after the reset,
> that's important to know.
> 
> Indeed, we want to know if the same user is resetting multiple times, how
> long since their profile was created, whether they reset every six months,
> whether there are characteristics that cause a user to reset...

I agree that we'd want to know all these things. That's what led to bug 1063714 and this tree of bugs in general.
(Assignee)

Comment 11

4 years ago
ISTM there are some conflicting requirements here - is it possible for Benjamin and Gavin to have a chat about this and come to a resolution/decision?
Flags: needinfo?(gavin.sharp)
Flags: needinfo?(benjamin)

Updated

4 years ago
Iteration: 35.2 → 35.3

Comment 12

4 years ago
We discussed briefly on IRC this morning. Here's my summary:

* having the complete data could be used to do interesting reports in the future, but we don't have anyone signed up to do that reporting in the next 6 months
* the current FHR data format is going away, hopefully in the next 6 months
* I'm worried about the complexity of importing FHR and its preferences from one profile to another, and would prefer to keep the changes here as simple as possible
* The minimum necessary thing is to know when the old FHR record is "done" because of a reset and that the new record is from a reset

So that doesn't quite answer the question, because if you have a low-risk and low-complexity patch already in hand, that may be the thing to take. But otherwise, let's do the simplest possible thing and revisit when we have the unified data-collection system.
Flags: needinfo?(benjamin)
I don't know what "close the old record" would involve exactly - I suppose just annotating it with a "was reset" boolean? It would also require forcing an FHR sync before we actually restart to do the reset/delete the old profile, which seems potentially tricky (and that can always fail for various reasons). Given that, copying over the FHR state during the reset seems like the simplest option.

The question of what data we include in the FHR payload is simpler. What we include in the payload about resets is a separate question - I agree that we can just do something simple for now (e.g. include the timestamp of the latest reset).
Flags: needinfo?(gavin.sharp)
(Assignee)

Comment 14

4 years ago
FTR, and for better or worse, the patch for this is in bug 1063710
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1063710

Updated

4 years ago
Iteration: 35.3 → ---
You need to log in before you can comment on or make changes to this bug.