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
Changes to active_users_aggregates query for Mobile #5396
base: main
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
sql_generators/active_users_aggregates_v3/templates/desktop_query.sql
Outdated
Show resolved
Hide resolved
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.
The code looks good. I just have a few comments to address before we can merge.
sql_generators/active_users_aggregates_v3/templates/focus_android_view.sql
Show resolved
Hide resolved
sql_generators/active_users_aggregates_v3/templates/desktop_query.sql
Outdated
Show resolved
Hide resolved
sql_generators/active_users_aggregates_v3/templates/mobile_query.sql
Outdated
Show resolved
Hide resolved
sql_generators/active_users_aggregates_v3/templates/mobile_query.sql
Outdated
Show resolved
Hide resolved
sql_generators/active_users_aggregates_v3/templates/desktop_query.sql
Outdated
Show resolved
Hide resolved
… attribution only when neccesary to improve performance. Delay update by 1 day to get metrics ping's data.
sql_generators/active_users_aggregates_v3/templates/focus_android_query.sql
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…growth metrics from clients_last_seen_v2, replace language_name with locale and remove search metrics based on sprint decision (see DENG-1989).
…DENG-2989_update_active_users_aggregates # Conflicts: # sql_generators/active_users_aggregates_v3/templates/desktop_query.sql # sql_generators/active_users_aggregates_v3/templates/desktop_schema.yaml
…ics based on sprint decision (see DENG-1989) and calculate the min metrics ping received between the current and next date, given that these pings can arrive in the same or next date as the baseline ping.
…is separated to [PR-5607](#5607).
@bochocki this PR is updated to remove search metrics and get the correct data from the metrics ping. Please give it a another review when you have time. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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.
Just one non-blocking question.
attribution_data.install_source, | ||
attribution_data.adjust_network | ||
unioned.*, | ||
{% if app_name == "fenix" or app_name == "firefox_ios" %} |
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.
Will this account for, e.g. MozillaOnline? I guess we don't care about attribution in that case?
This comment has been minimized.
This comment has been minimized.
Integration report for "Update app_anme in checks for Focus"
|
This PR implements DENG-2989 & DENG-2529 to update the query for active_users_aggregates:
Checklist 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