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

Log remaining instruments via Metrics Platform #4117

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

cjming
Copy link
Member

@cjming cjming commented Aug 7, 2023

After verifying data coming thru in a recent pre-prod beta release, this PR proposes to submit events for the remaining Android instruments (currently submitted via Modern Event Platform) via Metrics Platform.

The following instruments are included for Metrics Platform integration:

  • AppearanceSettingInteractionEvent
  • AppSessionEvent
  • BreadcrumbLogEvent
  • CreateAccountEvent
  • CustomizeToolbarEvent
  • DailyStatsEvent
  • EditHistoryInteractionEvent
  • InstallReferrerEvent
  • NotificationInteractionEvent
  • UserContributionEvent

Some new MP classes aim to consolidate a few of the above instruments.
If it is preferred to keep naming conventions 1:1, I can adjust accordingly.

Note: these changes are purely additive - no changes should occur with current event logging.

Bug: T330355

@cjming cjming added the WIP Work in progress label Aug 7, 2023
@cjming cjming marked this pull request as draft August 7, 2023 19:01
@cjming cjming marked this pull request as ready for review August 7, 2023 19:34
@cjming
Copy link
Member Author

cjming commented Aug 7, 2023

hi @dbrant @sharvaniharan @cooltey -- Q re: 2 instruments that have config but code does not seem to exist:

  • image_recommendation_interaction
  • article_page_scroll_interaction

I didn't include the two above in this PR even tho their corresponding tables do exist in Hive (23 rows in android_image_recommendation_interaction and 0 rows in android_article_page_scroll_interaction).

Just flagging this for your team -- should their configs be removed?

@cjming cjming removed the WIP Work in progress label Aug 7, 2023
@cjming
Copy link
Member Author

cjming commented Aug 8, 2023

also, apologies that this is a rather big change set -- as I was making changes, it seemed like some of the same files were being touched so i thought doing it in one go might be easier to reconcile

if it's helpful, i can break it up into smaller PRs

@dbrant dbrant added the HOLD Will review as soon as other more pressing PRs are merged/released. label Aug 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
HOLD Will review as soon as other more pressing PRs are merged/released.
Development

Successfully merging this pull request may close these issues.

3 participants