2041497 - write_in_microsurvey ping instructions are being forwarded in a clunky way

Status: RESOLVED FIXED

(Firefox :: Messaging System, enhancement)

Description

[Shane Hughes [:aminomancer]]

The way we decide whether to record messaging system metrics on the messaging-system ping or the microsurvey ping involves reading the message's write_in_microsurvey property. Right now, that property is being read by the React component and passed all the way through the nested components, where it gets added to the new telemetry event's event_context, which is passed to MultiStageUtils.sendEventTelemetry(), ultimately reaching AboutWelcomeTelemetry.sendTelemetry, which processes it and decides to record the metrics on microsurvey as a result.

This is a pretty clunky and inefficient way of doing things. It didn't occur to me at the time that we could bypass React entirely. The surface code that instantiates the React component is aware of the message definition - it's what is passing the message to the React component via AWGetFeatureConfig(). Likewise, it is what provides the AWSendEventTelemetry method. So it can easily check the write_in_microsurvey property within AWSendEventTelemetry, and insert it into the event_context when applicable. That minimizes the amount of processing for the property.

Comment 1

[Shane Hughes [:aminomancer]]

Attachment: Bug 2041497 - Refactor write_in_microsurvey ping determination. r=#omc-reviewers

Stop passing write_in_microsurvey from AWGetFeatureConfig to the
AWSendEventTelemetry call. Instead, the surface that provides the
AWSendEventTelemetry method will just insert it in the method
definition and pass it straight to AboutWelcomeTelemetry. This means
the React component does not have to be aware of the property at all.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/8df306503503
https://hg.mozilla.org/integration/autoland/rev/15c964221f55
Refactor write_in_microsurvey ping determination. r=omc-reviewers,andhuang

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/9fc89ff94a58
https://hg.mozilla.org/integration/autoland/rev/0cfba7541c69
Revert "Bug 2041497 - Refactor write_in_microsurvey ping determination. r=omc-reviewers,andhuang" for causing bc failures @ browser_aiwindow_smartbar_telemetry

Comment 4

[Sandor Molnar[:smolnar]]

Backed out for causing bc failures @ browser_aiwindow_smartbar_telemetry

Backout link: https://hg.mozilla.org/integration/autoland/rev/0cfba7541c6983a197b31a609ae128a69c5af779

Push with failures

Failures log ->TEST-UNEXPECTED-FAIL | browser/components/aiwindow/ui/test/browser/browser_aiwindow_smartbar_telemetry.js

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/a876520663c6
https://hg.mozilla.org/integration/autoland/rev/422cde25bbf7
Refactor write_in_microsurvey ping determination. r=omc-reviewers,andhuang

Comment 6

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/422cde25bbf7

Comment 7

[echa]

Attachment: Bug 2041497 - Refactor write_in_microsurvey ping determination.

Stop passing write_in_microsurvey from AWGetFeatureConfig to the
AWSendEventTelemetry call. Instead, the surface that provides the
AWSendEventTelemetry method will just insert it in the method
definition and pass it straight to AboutWelcomeTelemetry. This means
the React component does not have to be aware of the property at all.

Original Revision: https://phabricator.services.mozilla.com/D301946

Comment 8

[Donal Meehan [:dmeehan]]

:aminomancer, the uplift request is missing an uplift request form
https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift#Submit_the_Uplift_Request_Form

Comment 9

[Donal Meehan [:dmeehan]]

nvm I see it's part of a stack

Comment 10

[echa]

Attachment: Bug 2041497 - Refactor write_in_microsurvey ping determination.

Stop passing write_in_microsurvey from AWGetFeatureConfig to the
AWSendEventTelemetry call. Instead, the surface that provides the
AWSendEventTelemetry method will just insert it in the method
definition and pass it straight to AboutWelcomeTelemetry. This means
the React component does not have to be aware of the property at all.

Original Revision: https://phabricator.services.mozilla.com/D301946

Comment 11

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/e28f24c0dd3b
https://hg.mozilla.org/releases/mozilla-beta/rev/a110467549e0

Comment 12

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/54b73959f18f
https://hg.mozilla.org/releases/mozilla-release/rev/09c4cf0519e6

Comment 13

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/86f17647c74d
https://hg.mozilla.org/releases/mozilla-release/rev/68c211635162

Comment 14

[Donal Meehan [:dmeehan]]

Backed out of release, since Bug 2036193 had to be backout due to CI failures