- Notifications
You must be signed in to change notification settings - Fork1k
chore: switch ssh session stats based on experiment#13637
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
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.
LGTM 👍
} | ||
return"" | ||
returncodersdk.UsageAppNameSSH |
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.
Letting the zero value mean 'disabled' feels more idiomatic to me, but 'disabled' is way more explicit and better from a user perspective.
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 considered having the default value on the flag bessh
and disable reporting if it's manually set to empty string but I wasn't sure if that was explicit enough behavior. I'm thinking in the extensions where we doCODER_SSH_USAGE_APP_NAME="" coder ssh ...
I'm worried there may be cases where the empty string get's passed to the CLI as "unset" instead.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
This changes
coderd
to only allow a single endpoint to write ssh session data to the DB at a time. Both the CLI and the agent will always report stats, but the agent endpoint will silently drop session stats while the experimentworkspace-usage
is enabled. This prevents us from double-counting stats during the migration period.We will need to give end users sufficient time to update their CLI and extensions before moving this behavior to GA otherwise outdated clients will have their data dropped until they upgrade.