Skip to content

feat(coderd): add template app usage to insights #9138

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

Merged
merged 11 commits into from
Aug 21, 2023

Conversation

mafredri
Copy link
Member

@mafredri mafredri commented Aug 16, 2023

This PR adds template app usage to insights. Once merged, the dashboard will automatically start showing these.

Fixes #8658

Notable changes:

  • Simplified the generate_series a bit via better sqlc workaround
  • Query indentation was fixed (there was some tab spacing)
  • Changed usage granularity from 5 minutes to 1 minute, I feel 5 minutes may skew the numbers too much for short lived sessions

Notes:

  • Unfortunately I couldn't combine GetTemplateInsights and GetTemplateAppInsights into a single query because of sqlc limitations, and I did not want to trade type-safety for performance

@mafredri mafredri changed the title feat(coderd): report template app usage insights feat(coderd): add template app usage to insights Aug 16, 2023
@mafredri
Copy link
Member Author

Demo:
image

appUsage, err = tx.GetTemplateAppInsights(ctx, database.GetTemplateAppInsightsParams{
StartTime: startTime,
EndTime: endTime,
TemplateIDs: templateIDs,
Copy link
Member

Choose a reason for hiding this comment

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

Hint: while you are writing unit tests, please make sure that cover paths with TemplateIDs and without them (ALL).

Copy link
Member Author

Choose a reason for hiding this comment

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

I will create an improved test-suite as a follow-up to this PR which will perform tests for multiple templates as well.

@mafredri mafredri force-pushed the mafredri/feat-add-app-usage-to-template-insights-2 branch from 7707313 to cacebb9 Compare August 18, 2023 14:38
@mafredri
Copy link
Member Author

I've added basic tests and will improve upon them in a follow up PR. I've also manually confirmed that the query changes produces the same data on dev.coder.com when disregarding the new data.

A funny side-effect of using the 5 minute windows is that when testing, I:

  1. Opened code-server
  2. Refreshed insights, there was immediately 5 minutes code-server usage
  3. Waited 1 minute
  4. Refreshed insights, there was 10 minutes code-server usage because the clock went from :39 to :40

More users will skew nr. 4 even more, so I'm convinced we need to reduce the window whilst maintaining or improving query performance, and perhaps not use one at all for template apps since it's not strictly required.

@mafredri mafredri marked this pull request as ready for review August 18, 2023 17:52
@mtojek mtojek self-requested a review August 21, 2023 08:46
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.

Is there anything left for @BrunoQuaresma to polish in the site area?

@mafredri mafredri merged commit 03453b1 into main Aug 21, 2023
@mafredri mafredri deleted the mafredri/feat-add-app-usage-to-template-insights-2 branch August 21, 2023 12:08
@github-actions github-actions bot locked and limited conversation to collaborators Aug 21, 2023
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.

Track coder_app usage in insights API
3 participants