-
Notifications
You must be signed in to change notification settings - Fork 936
feat: add connection statistics for workspace agents #6469
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
Changes from all commits
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
8f1f141
fix: don't make session counts cumulative
kylecarbs ddf9841
Add databasefake query for getting agent stats
kylecarbs 28d6db5
Add deployment stats endpoint
kylecarbs 29719a4
The query... works?!?
kylecarbs 09a2dad
Fix aggregation query
kylecarbs 12a52b1
Select from multiple tables instead
kylecarbs a1804a9
Fix continuous stats
kylecarbs 93f013b
Increase period of stat refreshes
kylecarbs 50260c3
Add workspace counts to deployment stats
kylecarbs d1bae99
fmt
kylecarbs 9fe9d4c
Add a slight bit of responsiveness
kylecarbs 00ebe2e
Fix template version editor overflow
kylecarbs cd76533
Add refresh button
kylecarbs 506740b
Fix font family on button
kylecarbs 1924f58
Merge branch 'main' into exportstats
kylecarbs 9f00ac5
Fix latest stat being reported
kylecarbs 4b6992c
Merge branch 'main' into exportstats
kylecarbs 1af9f64
Revert agent conn stats
kylecarbs e3ca39f
Merge branch 'main' into exportstats
kylecarbs 8ad39d6
Fix linting error
kylecarbs 0f06b23
Fix tests
kylecarbs e87ba59
Fix gen
kylecarbs 99d7d1a
Fix migrations
kylecarbs 37ad03f
Block on sending stat updates
kylecarbs 415d8b1
Merge branch 'main' into exportstats
kylecarbs 0037a64
Add test fixtures
kylecarbs 3d70b2a
Merge branch 'main' into exportstats
kylecarbs d708210
Fix response structure
kylecarbs c951d5a
make gen
kylecarbs File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I guess this still confuses me a bit. If Tailscale stats aren't cumulative, isn't the only way this matches lastStat if there was no chatter (tx/rx), the latency and sessions for SSH/Code/JetBrains stayed the same?
Since we're also doing network pings in the latency check, I think there is a non-zero chance for multiple
reportStats
to be running concurrently, essentially competing about Load()/Store() here?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.
Hmm, good points. I'll refactor this.
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.
After looking at this again, it seems like this should be fine.
This will only match if there's no traffic, but that's arguably great because then we aren't spamming the database with nonsense. I don't want to do this in
coderd
, because we'd need to query for the last stat to ensure it's not the same.reportStats
is blocking, and so subsequent agent stat refreshes will wait before running again, so I don't think they'd compete.Let me know if I'm overlooking something or didn't understand properly, I'm sick and my brain is stuffy right now ;p