1450090 - Add study branch to Normandy Client unenroll telemetry event

Status: VERIFIED FIXED

(Firefox :: Normandy Client, enhancement, P1)

Description

[Jared Lockhart [:jlockhart]]

The current Normandy Client enroll telemetry event schema includes the study branch, seen here:

https://github.com/mozilla/normandy/blob/master/recipe-client-addon/lib/TelemetryEvents.jsm#L20


However the 'branch' is not included in the unenroll telemetry event:

https://github.com/mozilla/normandy/blob/master/recipe-client-addon/lib/TelemetryEvents.jsm#L27


This makes it difficult to track unenrollment on a per branch basis, which can occur under certain conditions.  We should add branch to the unenroll event to make it easier to track and assess study health.

Comment 1

[Rob Miller [:rmiller]]

This issue is the last thing blocking us being able to put real-time per-branch enrollment numbers on our experiment dashboards... any chance we can get this bumped up in priority?

Comment 2

[Matt Grimes [:Matt_G]]

Getting our in-flight dashboards up is definitely a P1 as this impacts not only my team, but all of our stakeholders as well. Mythmon - What's the LOE for this and what would we be trading off to make this happen?

Comment 3

[Michael Cooper [:mythmon]]

This is something that should be pretty easy to fix. It is mostly an oversight in the spec of what should be in the events that was reviewed here: https://github.com/mozilla/normandy/issues/1139#issuecomment-345879729. Assuming we have this data at unenrollment time (I think we do), it should just be a matter of adding it to the schema and including the data in the event.

As far as getting this bumped in priority: preference rollout still dominates the engineering time on Normandy, but I'll make this a P2 (will work on next)

Comment 4

[Rob Miller [:rmiller]]

Bump... this is still blocking us in a big way, wondering if we've got a timeframe for getting this implemented?

Comment 5

[Matt Grimes [:Matt_G]]

I know we've (mostly) wrapped work on preference rollout. Mythmon - Do you now have time to pick this up? Expanding our visibility into the Shield ecosystem is extremely important.

Comment 6

[Michael Cooper [:mythmon]]

Attachment: Bug 1450090 - Add study branch to preference study unenroll telemetry event

Review commit: https://reviewboard.mozilla.org/r/244446/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/244446/

Comment 7

[Michael Cooper [:mythmon]]

Thanks for the bump. Now that there is some extra time, this was pretty straight forward.

Comment 8

[:Gijs (he/him)]

Comment on attachment 8976262 [details]
Bug 1450090 - Add study branch to preference study unenroll telemetry event

https://reviewboard.mozilla.org/r/244446/#review250554

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/639903e9e136
Add study branch to preference study unenroll telemetry event r=Gijs

Comment 10

[Michael Cooper [:mythmon]]

Comment on attachment 8976262 [details]
Bug 1450090 - Add study branch to preference study unenroll telemetry event

Approval Request Comment
[Feature/Bug causing the regression]: N/A

[User impact if declined]: 
"This change will allow more detailed monitoring of Shield pref flip study health, which helps prevent malformed studies from running for long periods of time unnoticed.  This can help us catch problems sooner so users aren’t being rapidly enrolled and unenrolled from studies, which can appear confusing to them." -jkerim

[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: No
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Adds existing data to a more convenient place, without affecting behavior.
[String changes made/needed]: None

Comment 11

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/639903e9e136

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8976262 [details]
Bug 1450090 - Add study branch to preference study unenroll telemetry event

Helps us better monitor the health of SHIELD studies and avoid potentially confusing users if they end up in a bad state. Approved for 61.0b7.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/617990cee57d

Comment 14

[Adrian Florinescu [:aflorinescu]]

Verified as fixed on: Windows 8 x64, the branch is now displayed in the telemetry events extras.
- 61.0b7 20180521110957
- 62.0a1 20180521220045 


toolkit.telemetry.cachedClientID = ed26014c-77f4-43f2-a655-e8869ea4334f
11379 	normandy 	enroll 	preference_study 	branches_experiment 	{"experimentType": "exp", "branch": "4"}
307098 	normandy 	unenroll 	preference_study 	branches_experiment 	{"didResetValue": "false", "branch": "4", "reason": "user-preference-changed"} 

toolkit.telemetry.cachedClientID = 22bf489d-16f0-4128-98f7-c90d7b06dce2
1521 	normandy 	unenroll 	preference_study 	branches_experiment 	{"didResetValue": "false", "branch": "1", "reason": "user-preference-changed-sideload"}

general opt-out toolkit.telemetry.cachedClientID = 88f32802-cd87-456b-afe7-e0897d5a0a35
4625 	normandy 	unenroll 	preference_study 	branches_experiment 	{"didResetValue": "true", "branch": "2", "reason": "recipe-not-seen"} 

I will check Redash in 1-2days in order to make sure everything actually gets displayed properly. Adding NI for myself for that.

Comment 15

[Adrian Florinescu [:aflorinescu]]

Telemetry Redash data looks good as well. The study branch is now listed in the telemetry server data, hence marking this issue as verified.

Mike, I see two columns in Redash that are not populated with data: active_experiment_id &	
active_experiment_branch. Expected or a possible bug?

Comment 16

[Michael Cooper [:mythmon]]

I'm not sure what those columns are. If you have the particular query you're looking at, I could check it. I would guess however that users that have just unenrolled from an experiment would probably have no data for active_experiment_* columns.

Comment 17

[Adrian Florinescu [:aflorinescu]]

I don't remember seeing those columns populated anywhere. I was looking at the comment 14 client id's, which have enroll/unenroll rollout events:
https://sql.telemetry.mozilla.org/queries/54055/source

Let me know if you'd like to have a separate bug if you think that those two columns should have any data and we should track it.

Comment 18

[Michael Cooper [:mythmon]]

Looking through that query, all the data I expect to see is represented in some column. I haven't done anything with those columns before, and I can't find any reference to them in mozilla-central. I think it is fine that they are blank.