1152255 - Telemetry.asyncFetchTelemetryData is called twice (TelemetrySession and TelemetryPing)

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect, P4)

Description

[Alessio Placitelli [:Dexter]]

|Telemetry.asyncFetchTelemetryData| is being called in two different locations:

- https://hg.mozilla.org/mozilla-central/annotate/078128c2600a/toolkit/components/telemetry/TelemetrySession.jsm#l1572
- https://hg.mozilla.org/mozilla-central/annotate/078128c2600a/toolkit/components/telemetry/TelemetryPing.jsm#l812

We should be calling that initialisation function only from one location, removing the other call.

Comment 1

[Georg Fritzsche [:gfritzsche]]

Note that due to the implementation the double call is not a problem right now.

The things collected are session related measurements, so we should probably keep the call in TelemetrySession.jsm only.

Comment 2

[Georg Fritzsche [:gfritzsche]]

ni?dexter for setting it up as a mentored bug.

Comment 3

[Alessio Placitelli [:Dexter]]

We should remove the |Telemetry.asyncFetchTelemetryData| call in TelemetryController.jsm [0] and check that everything works as expected by running xpcshell tests for Telemetry as follows:

./mach xpcshell-test toolkit/components/telemetry

[0] - https://dxr.mozilla.org/mozilla-central/rev/f029ccdee154bdc2b49a1e0fcd5b0fa0397aa97b/toolkit/components/telemetry/TelemetryController.jsm#767

Comment 4

[Georg Fritzsche [:gfritzsche]]

Sanchit, would you be interested in working on this?

Comment 5

[Sanchit Nevgi]

(In reply to Georg Fritzsche [:gfritzsche] from comment #4)
> Sanchit, would you be interested in working on this?

Hi Georg! Sorry for the late response(My exams are going on). I would love to work on this bug. Thanks for the suggestion :)

Comment 6

[Georg Fritzsche [:gfritzsche]]

Great, assigning it to you :)
If you run into any problems, Alessio (Dexter on IRC) and i are happy to help.

Comment 7

[Sanchit Nevgi]

Attachment: Removed Telemetry.asyncFetchTelemetryData call in TelemetryController.jsm

Comment 8

[Sanchit Nevgi]

Hi! I removed the call in TelemetryController.jsm and ran the xpcshell tests. They have all passed. Do I need to make any more changes? Thanks :)

Comment 9

[Alessio Placitelli [:Dexter]]

Comment on attachment 8684553 [details] [diff] [review]
Removed Telemetry.asyncFetchTelemetryData call in TelemetryController.jsm

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

Sanchit, this looks good, thanks!

Comment 10

[Alessio Placitelli [:Dexter]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c00d3f6bdd64

Comment 11

[Alessio Placitelli [:Dexter]]

I've pushed this to the Try servers for you: https://treeherder.mozilla.org/#/jobs?repo=try&revision=70d0eca2c737

We'll land this as soon as the tests are done!

Comment 12

[Alessio Placitelli [:Dexter]]

The tests look good, the failures seem to be unrelated. Marking this for check-in!

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/e402ff476e1b

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/e402ff476e1b