Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: add deployment-wide DAU chart #5810

Merged
merged 27 commits into from
Jan 26, 2023
Merged

feat: add deployment-wide DAU chart #5810

merged 27 commits into from
Jan 26, 2023

Conversation

presleyp
Copy link
Contributor

Closes #5751

  • adds a Metrics page to Deployment Settings
  • puts deployment-wide DAUs on that page

@presleyp presleyp requested a review from a team as a code owner January 20, 2023 18:23
@presleyp presleyp requested review from code-asher and removed request for a team January 20, 2023 18:23
@presleyp presleyp changed the title 5751/deployment dau/presleyp feat: add deployment-wide DAU chart Jan 20, 2023
@presleyp presleyp requested a review from ammario January 20, 2023 18:44
code-asher
code-asher approved these changes Jan 20, 2023
View reviewed changes
Copy link
Member

@code-asher code-asher left a comment •
edited

Choose a reason for hiding this comment

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

Looks great! The only comment I have is that it seems when there is no activity we hide the chart which makes sense to me on the templates page but I found it a little surprising to see a blank page on the deployment-wide metrics page so maybe there we could have a "no activity" message or something.

@presleyp
Copy link
Contributor Author

Looks great! The only comment I have is that it seems when there is no activity we hide the chart which makes sense to me on the templates page but I found it a little surprising to see a blank page on the deployment-wide metrics page so maybe there we could have a "no activity" message or something.

So glad you caught that! Ammar suggested maybe putting this in General for now since we only have one chart and I was torn but that decides it for me.

ammario
ammario previously requested changes Jan 20, 2023
View reviewed changes
Copy link
Member

@ammario ammario left a comment

Choose a reason for hiding this comment

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

overall, looks good

coderd/insights_test.go Outdated Show resolved Hide resolved
coderd/insights_test.go Outdated Show resolved Hide resolved
coderd/insights_test.go Outdated Show resolved Hide resolved
coderd/insights_test.go Outdated Show resolved Hide resolved
coderd/metricscache/metricscache.go Outdated Show resolved Hide resolved
coderd/metricscache/metricscache.go Outdated Show resolved Hide resolved
presleyp and others added 2 commits January 23, 2023 09:46
@presleyp @ammario
Update coderd/metricscache/metricscache.go
99bbf4f 
Co-authored-by: Ammar Bandukwala <ammar@ammar.io>
@presleyp
Go fixes
88ecbae
@code-asher
Copy link
Member

👍 for the new location!

@presleyp presleyp requested a review from ammario January 23, 2023 17:09
@BrunoQuaresma BrunoQuaresma self-assigned this Jan 26, 2023
@BrunoQuaresma BrunoQuaresma enabled auto-merge (squash) January 26, 2023 01:02
@BrunoQuaresma BrunoQuaresma merged commit 16d8cc4 into main Jan 26, 2023
@BrunoQuaresma BrunoQuaresma deleted the 5751/deployment-dau/presleyp branch January 26, 2023 01:03
@github-actions github-actions bot locked and limited conversation to collaborators Jan 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@code-asher code-asher code-asher approved these changes

@ammario ammario Awaiting requested review from ammario

Assignees

@BrunoQuaresma BrunoQuaresma

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Metric: Deployment-wide daily active users
4 participants
@presleyp @code-asher @ammario @BrunoQuaresma