Skip to content

fix(coderd): use insights for DAUs, simplify metricscache #12775

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

Conversation

mafredri
Copy link
Member

@mafredri mafredri commented Mar 27, 2024

This PR simplifies the DAU logic by relying on rolled up insights giving us great query performance and removing the need for caching each individual timezone.

For this fix, I opted not to refactor the WebUI requests to use start_time=...&end_time=... like template insights requests do. Instead I retrofitted this logic on the /daus API endpoints using the TZ offset.

I also opted to always return 60 days (2 months), just because the way this is used it doesn't make sense to return more.

In the future, we can request this data from /insights/templates?section=interval&start_time=&end_time&interval=day instead, this also takes care of the 60 days hardcoding.

Fixes #12134
Fixes https://github.com/coder/customers/issues/384
Refs #12122

Copy link
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @mafredri and the rest of your teammates on Graphite Graphite

@mafredri mafredri force-pushed the mafredri/03-27-fix_coderd_use_insights_for_daus_simplify_metricscache branch from 8770c0f to 2b3b5da Compare March 27, 2024 13:48
@mafredri mafredri marked this pull request as ready for review March 27, 2024 13:50
@mafredri mafredri requested review from Emyrk, coadler and mtojek March 27, 2024 13:51
Copy link
Member

@kylecarbs kylecarbs left a comment

Choose a reason for hiding this comment

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

Code deletion 🤤🤤🤤

@f0ssel
Copy link
Contributor

f0ssel commented Mar 27, 2024

This was an area I spotted as a rough patch in our code, very happy to see significant deletions here!

Copy link
Member

@mtojek mtojek left a comment

Choose a reason for hiding this comment

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

Nice!

Copy link
Member

@Emyrk Emyrk left a comment

Choose a reason for hiding this comment

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

❤️

@mafredri mafredri merged commit 421bf7e into main Mar 27, 2024
32 checks passed
@mafredri mafredri deleted the mafredri/03-27-fix_coderd_use_insights_for_daus_simplify_metricscache branch March 27, 2024 16:10
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

A slow SQL query is causing high database load
5 participants