Closed Bug 1121001 Opened 9 years ago Closed 9 years ago

FHR data migration: org.mozilla.uitour.treatment

Categories

(Firefox Health Report Graveyard :: Client: Desktop, defect)

defect
Not set
normal

Tracking

(firefox40 fixed, firefox41 fixed)

RESOLVED FIXED
Firefox 41
Tracking Status
firefox40 --- fixed
firefox41 --- fixed

People

(Reporter: gfritzsche, Assigned: benjamin)

References

Details

(Whiteboard: [uplift])

Attachments

(2 files)

We need to migrate the UITour FHR provider.
Blocks: 1121006
No longer blocks: 1121006
Blocks: 1137160
No longer blocks: 1120356
Felipe, i see that you wrote that provider.
We are unclear on what the exact goals are for this provider.
As we don't have an exact equivalent to "daily text" in Telemetry now, we need to understand this better.
Can you expand or do you have an idea on how this would port over to Telemetry?
Flags: needinfo?(felipc)
I recommend putting this as an entirely separate ping type, including the profile ID. The goal of this is to be able to record actions taken during the UI tour or firstrun experience so that we can measure their effectiveness later, and also avoid repeating those treatments in self-support.
Flags: needinfo?(felipc)
In that case this is probably best done after the retention logic changes etc. and can wait a little.
Attached patch 1121001-uitourSplinter Review
Assignee: nobody → benjamin
Status: NEW → ASSIGNED
Attachment #8598808 - Flags: review?(gfritzsche)
Comment on attachment 8598808 [details] [diff] [review]
1121001-uitour

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

Is there any existing testing that we can adopt to this?

::: browser/components/uitour/UITour.jsm
@@ +1928,5 @@
>   * Public API to be called by the UITour code
>   */
>  const UITourHealthReport = {
>    recordTreatmentTag: function(tag, value) {
> +  TelemetryPing.send("uitour-tag", { version: 1 },

Note: This is now TelemetryController.submitExternalPing().

@@ +1934,5 @@
> +      retentionDays: 180,
> +      addClientId: true,
> +      addEnvironment: true,
> +      tagName: tag,
> +      tagValue: value,

I assume tagName & tagValue go in the payload above?
Attachment #8598808 - Flags: review?(gfritzsche)
Bug 1121001 - Record a uitour ping when a tag is set, r?gfritzsche
Attachment #8620979 - Flags: review?(gfritzsche)
Comment on attachment 8620979 [details]
MozReview Request: Bug 1121001 - Record a uitour ping when a tag is set, r?gfritzsche

https://reviewboard.mozilla.org/r/10871/#review9725

I assume recordTreatmentTag() is expected to be called max. once per browser session? Let's ship it if so.
If this can be called more often we might generate a lot of small pings.

::: toolkit/components/telemetry/docs/uitour-ping.rst:19
(Diff revision 1)
> +      }

Nit: We should add `clientId: ...,` and some `// ...` hinting at the additional fields in the top-level ping format.

::: browser/components/uitour/UITour.jsm:1989
(Diff revision 1)
> +      retentionDays: 180,

We removed `retentionDays` from the options, you should remove it here.
Attachment #8620979 - Flags: review?(gfritzsche) → review+
Blocks: 1174876
https://hg.mozilla.org/mozilla-central/rev/c418add0c07f
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Whiteboard: [uplift]
Comment on attachment 8620979 [details]
MozReview Request: Bug 1121001 - Record a uitour ping when a tag is set, r?gfritzsche

Approval Request Comment
[Feature/regressing bug #]:
Unified Telemetry, https://wiki.mozilla.org/Unified_Telemetry
This is part of the first (main) batch of uplifts to 40 to enable shipping on that train, see bug 1120356, comment 2.
[User impact if declined]:
Data & measurement insight projects delayed or blocked with direct impact on projects depending on this.
[Describe test coverage new/current, TreeHerder]:
We have good automation coverage of the feature.
We also had manual tests of the main tasks as well as confirmation of correct behavior on Nightly for the patches here.
[Risks and why]:
Low-risk - these patches are rather isolated to Telemetry and have been on Nightly for a while with no bad reports.
We intend to track on-going data quality and other issues during the 40 aurora & beta and flip the new behavior off when it doesn't meet the requirements.
[String/UUID change made/needed]:
The only string changes were to the about:telemetry page.
We decided that we can live with missing translations on that page for a cycle as that page is not exactly user-facing.
Attachment #8620979 - Flags: approval-mozilla-aurora?
Comment on attachment 8620979 [details]
MozReview Request: Bug 1121001 - Record a uitour ping when a tag is set, r?gfritzsche

Unified telemetry is an important new feature. It is blocking some other projects. Taking it.
Attachment #8620979 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Product: Firefox Health Report → Firefox Health Report Graveyard
You need to log in before you can comment on or make changes to this bug.