mozilla / security-advisor-shield-study

Mozilla Public License 2.0
2 stars 7 forks source link

Fix bug 1317417: Use static study name. #65

Open Osmose opened 7 years ago

Osmose commented 7 years ago

@casebenton r? Testing with about:telemetry I now see a study name in the payload being sent, where previously there was no name in those payloads.

@jtg567 If we fix this and put it up on AMO, I think users currently enrolled in the study will automatically download the update and start sending data with the proper study name. But I don't have a great way of testing the automatic updating without putting it up on AMO. Do you think it's worth uploading this fix to AMO to correct the data being sent in, or is the risk of that update messing with the study make it not worth it?

jtg567 commented 7 years ago

Rather than correct in-flight studies, I think it would be least headache to try to prevent this in future studies, presuming the former would require looking at/parsing two counts (current and corrected) to gauge progress and perform analyses.

Osmose commented 7 years ago

I think https://github.com/mozilla/shield-studies-addon-utils/issues/44 covers that, then. We'll merge this but I won't bother shipping it for now.

groovecoder commented 7 years ago

May not be part of this bug, but when I spot-checked the about:telemetry data on this branch, only the "study_state" : "install|running|user-converted-feature-offer" pings contained the study_name data.

I.e., none of the "eventType" : "signup|panelHide|panelShow|buttonShow" pings had a study_name field.

gregglind commented 7 years ago

(interesting!)