Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Analytic event for "Lookup Dictionary" (see if we can remove it) #8263

Closed
david-allison opened this issue Mar 18, 2021 · 13 comments · Fixed by #8283
Closed

Add Analytic event for "Lookup Dictionary" (see if we can remove it) #8263

david-allison opened this issue Mar 18, 2021 · 13 comments · Fixed by #8283
Labels
Analytics Good First Issue! Help Wanted Requesting Pull Requests from volunteers
Milestone

Comments

@david-allison
Copy link
Member

david-allison commented Mar 18, 2021

I don't think the feature is used. It's 10 years old, esoteric, hard to access, and probably deprecated by the share/system context menu.

It adds complexity to AbstractFlashcardViewer, and may be better as a separate app to allow system-wide usage, instead of an AnkiDroid specific feature.

I would like an analytic event to see if users still use it, to argue for its removal in 2.16

Likely entry point:

public static boolean lookUp(String text) {
if (!mIsDictionaryAvailable) {
return false;
}
// clear text from leading and closing dots, commas, brackets etc.
text = text.trim().replaceAll("[,;:\\s(\\[)\\].]*$", "").replaceAll("^[,;:\\s(\\[)\\].]*", "");

@david-allison david-allison changed the title Add Analytic event for "Lookup Dictionary" Add Analytic event for "Lookup Dictionary" (can remove it) Mar 18, 2021
@david-allison david-allison changed the title Add Analytic event for "Lookup Dictionary" (can remove it) Add Analytic event for "Lookup Dictionary" (to see if we can remove it) Mar 18, 2021
@david-allison david-allison changed the title Add Analytic event for "Lookup Dictionary" (to see if we can remove it) Add Analytic event for "Lookup Dictionary" (see if we can remove it) Mar 18, 2021
@david-allison david-allison added the Help Wanted Requesting Pull Requests from volunteers label Mar 18, 2021
@david-allison david-allison added this to the 2.15 release milestone Mar 18, 2021
@ritvij14
Copy link
Contributor

ritvij14 commented Mar 18, 2021

I went through the code for Lookup.java. And looking into AbstractFlashcardViewer I understood that this class is being initialized during loading of the collection (when the user starts using the flashcard deck). And Lookup was being used as a dictionary for translation (please correct me if I am wrong in understanding this part).
I also had 2 doubts, one, that this is the same analytic event being used here right?

UsageAnalytics.sendAnalyticsEvent(this.getClass().getSimpleName(), "enabled");

And also, just out of curiosity, how would this whole part function without the Lookup class?

@david-allison
Copy link
Member Author

And also, just out of curiosity, how would this whole part function without the Lookup class?

We'd remove it - I don't think the feature's used at all.


UsageAnalytics.sendAnalyticsEvent(this.getClass().getSimpleName(), "enabled"); 

Pending further explanation by @mikehardy (I'll add this to the Development Guide, or code, as I wasn't aware and learned in passing a few days ago)

The first argument is the category, and we may have a finite number of these(?), so we may want to standardise the input of this value

@ritvij14
Copy link
Contributor

We'd remove it - I don't think the feature's used at all.

UsageAnalytics.sendAnalyticsEvent(this.getClass().getSimpleName(), "enabled"); 

Pending further explanation by @mikehardy (I'll add this to the Development Guide, or code, as I wasn't aware and learned in passing a few days ago)

Ok got it.

The first argument is the category, and we may have a finite number of these(?), so we may want to standardise the input of this value

Oh, you mean in the development guide to AnkiDroid right?

Also, when adding a new analytics event, how do we know that it has been added correctly?

@david-allison
Copy link
Member Author

david-allison commented Mar 18, 2021

Also, when adding a new analytics event, how do we know that it has been added correctly?

We have a dev version of Google Analytics which dev builds send data to - we'll work something out. Typically we can just check logs to see if it works.

All in all, assume the API calls will be stable, and do some final testing afterwards.

Oh, you mean in the development guide to AnkiDroid right?

Yeah - Analytics are an area of the code that I don't have much experience with. Mike's very knowledgeable, and it's best to share this knowledge for myself, and future developers in the guide.

@mikehardy
Copy link
Member

I just checked to see if categories were limited as it was vague in my mind as well. I do not believe there are finite categories https://developers.google.com/analytics/devguides/collection/analyticsjs/field-reference#eventCategory

So then it is just a question on how to log the data so that reports are meaningful.

The only report I can easily create that shows "how much activity total there is" vs "how many times a user did a thing" is the engagement report that shows total screen views plus screens/session (so you can see about how many user sessions there were, easily) with event categories on the right (so you can see how many times a thing happened, and relate it to the number of user sessions)

EngagementReport

Which makes me think using a unique category is correct here, so you can answer the question "how many times is this used vs total app usage"

For that purpose, I think using the Lookup classname is fine. For the value, probably including which dictionary was used would be useful - the idea for the event value is to include any other information that is useful when you have questions about the feature usage. For the widget that's just "enabled" or "disabled" - there is nothing more to know - but the dictionary usage has more information that might be interesting.

@mikehardy
Copy link
Member

If you turn on debugging for analytics, I think it logs out every event it sends? So you will see them going out in the logs - or you should -

Timber.d("sendAnalyticsEvent() category/action/value/label: %s/%s/%s/%s", category, action, value, label);

Trust the logs

@ritvij14
Copy link
Contributor

ritvij14 commented Mar 18, 2021

For the value, probably including which dictionary was used would be useful - the idea for the event value is to include any other information that is useful when you have questions about the feature usage. For the widget that's just "enabled" or "disabled" - there is nothing more to know - but the dictionary usage has more information that might be interesting.

@mikehardy about this, how about we add analytics event to choosing a specific type of dictionary?

switch (mDictionary) {
case DICTIONARY_AEDICT:
mDictionaryAction = "sk.baka.aedict.action.ACTION_SEARCH_EDICT";
mIsDictionaryAvailable = Utils.isIntentAvailable(mContext, mDictionaryAction);
break;

For example, we add separate analytics event triggers in each of the cases of the switch construct and we make events this way ->

  1. aedict for first
  2. web for 2nd, 3rd, and 4th
  3. leo for 5th
  4. colordict for 6th
  5. fora for 7th, and finally
  6. none

This way we can categorize the events for all dictionaries.

@mikehardy
Copy link
Member

@ritvij14 I think you would do it in this switch:

public static boolean lookUp(String text) {

But otherwise, yes so I should expect to see Analytics hits of category "Lookup" with value "aedict" etc

@ritvij14
Copy link
Contributor

ritvij14 commented Mar 18, 2021

@ritvij14 I think you would do it in this switch:

public static boolean lookUp(String text) {

But otherwise, yes so I should expect to see Analytics hits of category "Lookup" with value "aedict" etc

Alright got it. I will get started on this and send a PR soon. One more thing, how should I test out the additions of analytics changes locally?

@mikehardy
Copy link
Member

@ritvij14 watch logcat, and if the analytics logs indicate the event is being logged, it will be logged, you may trust it.

@ritvij14
Copy link
Contributor

@mikehardy for this part I am getting a very small log, and there is no mention of analytics events being logged in here :-

2021-03-20 21:44:43.412 12099-12099/com.ichi2.anki D/ViewRootImpl: [TouchInput][ViewRootImpl] MotionEvent { action=ACTION_DOWN, id[0]=0, pointerCount=1, eventTime=49008305, downTime=49008305 } moveCount:0
2021-03-20 21:44:43.558 12099-12099/com.ichi2.anki D/ViewRootImpl: [TouchInput][ViewRootImpl] MotionEvent { action=ACTION_UP, id[0]=0, pointerCount=1, eventTime=49008453, downTime=49008305 } moveCount:1
2021-03-20 21:44:43.583 12099-12182/com.ichi2.anki D/Surface: Surface::disconnect(this=0x7ea3183000,api=1)
2021-03-20 21:44:43.586 12099-12182/com.ichi2.anki D/OpenGLRenderer: endAllActiveAnimators on 0x7ea311b500 (AlertController$RecycleListView) with handle 0x7e505a83a0
2021-03-20 21:44:43.586 12099-12099/com.ichi2.anki D/View: [Warning] assignParent to null: this = DecorView@63e6db1[Preferences]

There should be some log for sendAnalytics() as well right?

@mikehardy
Copy link
Member

In debug builds I believe you need to go into advanced settings and toggle on debug analytics, can you try that? And if so please add a note about this requirement on #8284 as we try to document this better

@ritvij14
Copy link
Contributor

ritvij14 commented Mar 20, 2021

@mikehardy I got the problem.
Actually, the place where I am adding the analytic actions is wrong. I am adding it in the lookup() function of the Lookup class. But when the person selects a dictionary from the Lookup Dictionary option, it doesn't call this function at all. This function is only called when using the dictionary in AbstractFlashCardViewer.java.

So what I did was, in the Preferences class, under the case of advanced settings, I added an extra onPreferenceChangeListener for dictionary preference. And now, when the user chooses a dictionary, it automatically sends an action to analytics.
Here are 2 examples I tested:-

2021-03-20 22:59:59.164 24539-24539/com.ichi2.anki D/ViewRootImpl: [TouchInput][ViewRootImpl] MotionEvent { action=ACTION_DOWN, id[0]=0, pointerCount=1, eventTime=53501656, downTime=53501656 } moveCount:0
2021-03-20 22:59:59.280 24539-24539/com.ichi2.anki D/ViewRootImpl: [TouchInput][ViewRootImpl] MotionEvent { action=ACTION_UP, id[0]=0, pointerCount=1, eventTime=53501772, downTime=53501656 } moveCount:0
2021-03-20 22:59:59.309 24539-24624/com.ichi2.anki D/Surface: Surface::disconnect(this=0x7eefbc6000,api=1)
2021-03-20 22:59:59.312 24539-24624/com.ichi2.anki D/OpenGLRenderer: endAllActiveAnimators on 0x7e96e3a400 (AlertController$RecycleListView) with handle 0x7ef43e0220
2021-03-20 22:59:59.313 24539-24539/com.ichi2.anki D/View: [Warning] assignParent to null: this = DecorView@9dc712c[Preferences]
2021-03-20 22:59:59.325 24539-24539/com.ichi2.anki D/UsageAnalytics: sendAnalyticsEvent() category/action/value/label: Lookup/colordict/-2147483648/null
2021-03-20 22:59:59.326 24539-24539/com.ichi2.anki D/UsageAnalytics: getOptIn() status: true

I have also added these changes to my PR #8283.

mikehardy pushed a commit that referenced this issue Mar 24, 2021
* added analytic event triggers to Lookup.java fix for #8263
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Analytics Good First Issue! Help Wanted Requesting Pull Requests from volunteers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants