Skip to content

LG-5477 Log total users counts w/breakdowns to analytics#5744

Merged
stevegsa merged 18 commits intomainfrom
steveu-reports-to-analytics
Jan 6, 2022
Merged

LG-5477 Log total users counts w/breakdowns to analytics#5744
stevegsa merged 18 commits intomainfrom
steveu-reports-to-analytics

Conversation

@stevegsa
Copy link
Contributor

@stevegsa stevegsa commented Dec 21, 2021

Why: Log all JSON reports to analytics for time based reporting and visualization

@stevegsa stevegsa marked this pull request as ready for review December 21, 2021 23:18
track_report_data_events(user_counts)
results = save_report(REPORT_NAME, user_counts.to_json, extension: 'json')

track_global_user_total_events
Copy link
Contributor Author

@stevegsa stevegsa Dec 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can piggy back it here...it could be it's own job

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the queries are completely independent of the ones for the s3 report right? seems better to be in its own job IMO

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my thinking on this was that these should eventually go into a report and logically this might be the one

track_report_data_events(user_counts)
results = save_report(REPORT_NAME, user_counts.to_json, extension: 'json')

track_global_user_total_events
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the queries are completely independent of the ones for the s3 report right? seems better to be in its own job IMO

@stevegsa stevegsa requested a review from zachmargolis January 6, 2022 02:53
@stevegsa stevegsa changed the title LG-5477 Log JSON reports to analytics LG-5477 Log total users counts w/breakdowns to analytics Jan 6, 2022
Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@stevegsa stevegsa merged commit 47b63ef into main Jan 6, 2022
@stevegsa stevegsa deleted the steveu-reports-to-analytics branch January 6, 2022 14:53
end

def reports_logger
@reports_log ||= ActiveSupport::Logger.new(Rails.root.join('log', 'reports.log'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fails with the changes in #5761, causing a build failure on main at the moment.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants