-
Notifications
You must be signed in to change notification settings - Fork 98
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
feat(DENG-3462): add activity fields to support KPI metrics to baseline_last_seen views #5434
feat(DENG-3462): add activity fields to support KPI metrics to baseline_last_seen views #5434
Conversation
@kik-kik do all the fields have the columns it is referencing? distribution ID, isp_name, etc? |
sql_generators/glean_usage/templates/baseline_clients_last_seen.view.sql
Outdated
Show resolved
Hide resolved
sql_generators/glean_usage/templates/baseline_clients_last_seen.view.sql
Outdated
Show resolved
Hide resolved
sql_generators/glean_usage/templates/baseline_clients_last_seen.view.sql
Outdated
Show resolved
Hide resolved
sql_generators/glean_usage/templates/baseline_clients_daily.view.sql
Outdated
Show resolved
Hide resolved
sql_generators/glean_usage/templates/baseline_clients_last_seen.view.sql
Outdated
Show resolved
Hide resolved
sql_generators/glean_usage/templates/baseline_clients_last_seen.view.sql
Outdated
Show resolved
Hide resolved
sql_generators/glean_usage/templates/baseline_clients_last_seen.view.sql
Outdated
Show resolved
Hide resolved
sql_generators/glean_usage/templates/baseline_clients_last_seen.view.sql
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comments and the need to account for Desktop, because the Glean template also generates desktop's view.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part of the description is incorrect This is a re-implementation of https://github.com/mozilla/bigquery-etl/pull/5396
.
I'd probably suggest "This PR moves the logic to calculate growth metrics from a single view for all apps to the clients_last_seen` views per app.".
sql_generators/glean_usage/templates/baseline_clients_last_seen.view.sql
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment on adding activity segments.
cc53718
to
819e62a
Compare
This comment has been minimized.
This comment has been minimized.
2650535
to
50e05e0
Compare
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bits28 udf requires the IFNULL validation
ea20b80
to
6d58d90
Compare
This comment has been minimized.
This comment has been minimized.
6d58d90
to
e120625
Compare
This comment has been minimized.
This comment has been minimized.
e120625
to
f206fe9
Compare
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
Blocked by: #5497 |
f206fe9
to
f7bc55d
Compare
This comment has been minimized.
This comment has been minimized.
f7bc55d
to
1e60491
Compare
This comment has been minimized.
This comment has been minimized.
To get this merged we'll likely need to skip stage deploys. Some of the queries that got changed depend on certain views, and some views depend on queries that got changed. Stage deploys can be skipped using the |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only a comment on the products_to_include_extra_activity_fields
sql_generators/glean_usage/templates/baseline_clients_last_seen.view.sql
Outdated
Show resolved
Hide resolved
b5fd34a
to
8a9de08
Compare
This comment has been minimized.
This comment has been minimized.
8a9de08
to
e206a89
Compare
e206a89
to
aef9b25
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blocked pending discussion.
Integration report for "feat: add activity fields to support KPI metrics to last_seen views"
|
Closing in favor of #5594 |
feat(DENG-3462): add activity fields to support KPI metrics to
baseline_last_seen
viewsThis approach aims to add fields to enable us to calculate KPI metrics based on activity. The original proposal was to have a "main" view which would union all the tables and add these field (see: Create telemetry.active_users view ).
This PR takes a slightly different approach and instead introduces those additional fields in the upstream view (baseline_clients_last_seen).
The following fields are being added to support our KPI metric calculations:
activity_segment
- categorise a client based on their activity "levels"is_dau
- sent ping and considered active on current dateis_wau
- sent ping and considered active in the last 7 daysis_mau
- sent ping and considered active in the last 28 daysis_daily_user
- client a baseline sent ping on current dateis_weekly_user
- client a baseline sent ping in the 7 daysis_monthly_user
- client a baseline sent ping in the last 28 daysChecklist for reviewer:
<username>:<branch>
of the fork as parameter. The parameter will also show upin the logs of the
manual-trigger-required-for-fork
CI task together with more detailed instructions.For modifications to schemas in restricted namespaces (see
CODEOWNERS
):┆Issue is synchronized with this Jira Task