(In reply to :Grisha Kruglov from bug 1377288 comment #14) > A thought: you might want to account for ability to configure the A-S panel. > Perhaps having an ability to segment analytics (and dashboards) by which > portions of the UI are enabled would be helpful? I'm not sure if the > telemetry part of that was done along with other AS Settings work. Good idea! I filed this bug. > Also, what does desktop do to account for such user customizations? It looks like desktop has bit-packed field "user_prefs" in their telemetry (see the description towards the bottom of the page): https://github.com/mozilla/activity-stream/blob/master/docs/v2-system-addon/data_dictionary.md --- Adding to the current iteration because it sounds useful.

(In reply to Michael Comella (:mcomella) from comment #0) > It looks like desktop has bit-packed field "user_prefs" in their telemetry Which I assume is sent for every UI event.

Actually, I asked Maria the priority on Slack: I'll change the flags once I get a response.

And please update the docs too! http://searchfox.org/mozilla-central/source/mobile/android/docs/activitystreamtelemetry.rst

This should be easy to do as far as adding stuff to the telemetry bundle is concerned. See https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/ActivityStreamPanel.java#88 for how we set a global "is FxA account present" flag. I reckon this will be very similar.

From Maria: Great if it can land, but we won't hold the release for this. (In reply to :Grisha Kruglov from comment #4) > for how we set a global "is FxA account present" flag. I reckon this will be very > similar. Don't forget to update this after changing the settings (after bug 1395792) - perhaps we should put this setGlobal in onResume or similar?

From liuche: we record when the preference changes. --- This will let us know *how often* people toggle these preferences, but will not let us attribute the current preference state to a UI click. From Grisha: if [the customizations are] used enough, without segmenting A-S dashboards become potentially very misleading --- So if this doesn't land, we can at least see how important it is that this does land. :)

[eng triage recommendation] P1: Chenxia feels this is easy to fix.

Updating the bug to make it more clear that this is to help us segment usage based on what is actually enabled (e.g., # of highlight clicks/total clicks of people who have highlights enabled).

Comment on attachment 8910957 [details] Bug 1401404 - Add telemetry for AS content prefs. https://reviewboard.mozilla.org/r/182414/#review187742 Category 2 data. datareview+

Comment on attachment 8910957 [details] Bug 1401404 - Add telemetry for AS content prefs. https://reviewboard.mozilla.org/r/182414/#review187750 I think the bit-packing is not quite right, unless you can explain to me otherwise. :) However, I think it'd be best to be consistent with desktop. ::: mobile/android/base/java/org/mozilla/gecko/activitystream/ActivityStreamTelemetry.java:64 (Diff revision 1) > } > > /** > + * AS_USER_PREFERENCES > + * > + * NB: Additional pref values to be bit-packed should be unique prime numbers. Why do these need to be prime numbers? I'm not sure this is correct. A conventional way to define bit-packing is with bit shift operators: ``` final int showSearch = 1 << 0; // 0001 final int showTopSites = 1 << 1; // 0010 ... ``` e.g. in your example, the bits for 2 (Pocket), 0010, conflict for those in 3 (Visited), 0011 so it'd be impossible to distinguish pocket enabled/disabled when visited is enabled. Caveat: Java ints are signed so you'll have to make sure these values are actually 1, 2, and 4 (I think the signed bit is the first bit, so I think it's fine). Alternatively, just define them as 1, 2, and 4. :) ::: mobile/android/base/java/org/mozilla/gecko/activitystream/ActivityStreamTelemetry.java:66 (Diff revision 1) > /** > + * AS_USER_PREFERENCES > + * > + * NB: Additional pref values to be bit-packed should be unique prime numbers. > + **/ > + private final static int POCKET_ENABLED_VALUE = 2; Can we make these flags consistent with desktop? i.e. ``` showSearch | 1 | | showTopSites | 2 | | showTopStories | 4 | ``` This should make analysis easier because we can use the same-ish code. from https://github.com/mozilla/activity-stream/blob/master/docs/v2-system-addon/data_dictionary.md ::: mobile/android/base/java/org/mozilla/gecko/activitystream/ActivityStreamTelemetry.java:111 (Diff revision 1) > > public static Builder builder() { > return new Builder(); > } > > + nit: ws ::: mobile/android/docs/activitystreamtelemetry.rst:132 (Diff revision 1) > > Full Examples > ============= > Following examples of events are here to provide a better feel for the overall shape of telemetry data being recorded. > > -1) User with an active Firefox Account clicked on a menu item for a third highlight ("visited"): > +1) User with an active Firefox Account clicked on a menu item for a third highlight ("visited") and has preffed on AS top stories, bookmarks, and visited: nit: `prefed`? I just figure since preference has one f, but I'm not sure. :)

Comment on attachment 8910957 [details] Bug 1401404 - Add telemetry for AS content prefs. https://reviewboard.mozilla.org/r/182414/#review187760 lgtm, thanks for the quick turn around! :) ::: mobile/android/base/java/org/mozilla/gecko/activitystream/ActivityStreamTelemetry.java:65 (Diff revision 2) > > /** > + * AS_USER_PREFERENCES > + * > + * NB: Additional pref values should be (unique) powers of 2 > + * We skip 1 and 2 to be consistent with Desktop, because those prefs are not used in Firefox for Android. Oh good call – I actually didn't notice that the prefs we use are different from those in desktop. ::: mobile/android/base/java/org/mozilla/gecko/activitystream/ActivityStreamTelemetry.java:78 (Diff revision 2) > + * > + * @param sharedPreferences SharedPreferences of this profile > + * @return bit-packed value of the user's AS preferences, which is the sum of the values of the enabled preferences. > + */ > + public static int getASUserPreferencesValue(final SharedPreferences sharedPreferences, final Resources res) { > + int bitPackedPrefValue = 0; Here's a wonky thought: we could start at 2 because that's the desktop preference and our top sites are always enabled. :) I don't think we should though because that creates assumptions in the code that'd be easy to mess up in future iterations.

Pushed by michael.l.comella@gmail.com: https://hg.mozilla.org/integration/autoland/rev/9909c8392ccf Add telemetry for AS content prefs. r=francois,mcomella

Comment on attachment 8910957 [details] Bug 1401404 - Add telemetry for AS content prefs. Approval Request Comment [Feature/Bug causing the regression]: Missing telemetry [User impact if declined]: Won't have telemetry about the pref state on activity stream [Is this code covered by automated tests?]: no [Has the fix been verified in Nightly?]: local builds [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, adding telemetry [Why is the change risky/not risky?]: Only adding calls to telemetry [String changes made/needed]:

Comment on attachment 8910957 [details] Bug 1401404 - Add telemetry for AS content prefs. More telemetry, taking it. Should be in 57b3.