- Notifications
You must be signed in to change notification settings - Fork928
feat(coderd): add parameter insights to template insights#8656
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
Merged
mtojek merged 11 commits intomainfrommafredri/feat-coderd-add-template-variable-usage-to-insightsAug 3, 2023
Merged
feat(coderd): add parameter insights to template insights#8656
mtojek merged 11 commits intomainfrommafredri/feat-coderd-add-template-variable-usage-to-insightsAug 3, 2023
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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
d4ffac5 tof60019bCompare Base automatically changed frommafredri/feat-coderd-add-user-latency-and-insights-endpoints tomainJuly 21, 2023 18:00
258a5d2 to680da6eCompare355c9b5 tobf218e9CompareUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Member
mtojek commentedAug 3, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
This is what I achieved with Click me{"report": {"start_time":"2023-08-01T00:00:00Z","end_time":"2023-08-03T11:00:00Z","template_ids": [],"active_users":0,"apps_usage": [ {"template_ids": [],"type":"builtin","display_name":"Visual Studio Code","slug":"vscode","icon":"/icon/code.svg","seconds":0 }, {"template_ids": [],"type":"builtin","display_name":"JetBrains","slug":"jetbrains","icon":"/icon/intellij.svg","seconds":0 }, {"template_ids": [],"type":"builtin","display_name":"Web Terminal","slug":"reconnecting-pty","icon":"/icon/terminal.svg","seconds":0 }, {"template_ids": [],"type":"builtin","display_name":"SSH","slug":"ssh","icon":"/icon/terminal.svg","seconds":0 } ],"parameters_usage": [ {"template_ids": ["6762af63-c8da-487a-9b05-814b18f5b3a9" ],"display_name":"","name":"Compute instances","values": [ {"value":"3","count":2 } ] }, {"template_ids": ["6762af63-c8da-487a-9b05-814b18f5b3a9" ],"display_name":"","name":"Docker Image","values": [ {"value":"ghcr.io/harrison-ai/coder-dev:base","count":2 } ] }, {"template_ids": ["6762af63-c8da-487a-9b05-814b18f5b3a9" ],"display_name":"Very random string","name":"Optional random string","values": [ {"value":"aaa","count":1 }, {"value":"ffff","count":1 } ] }, {"template_ids": ["6762af63-c8da-487a-9b05-814b18f5b3a9" ],"display_name":"","name":"Region","options": [ {"name":"US Central","description":"Select for central!","value":"us-central1-a","icon":"/icon/goland.svg" }, {"name":"US East","description":"Select for east!","value":"us-east1-a","icon":"/icon/folder.svg" }, {"name":"US West","description":"Select for west!","value":"us-west2-a","icon":"" } ],"values": [ {"value":"us-west2-a","count":2 } ] }, {"template_ids": ["6762af63-c8da-487a-9b05-814b18f5b3a9" ],"display_name":"","name":"Security groups","values": [ {"value":"[\"Web Server Security Group\",\"Database Security Group\",\"Backend Security Group\"]","count":2 } ] }, {"template_ids": ["6762af63-c8da-487a-9b05-814b18f5b3a9" ],"display_name":"Very random string","name":"buggy-1","values": [ {"value":"","count":2 } ] }, {"template_ids": ["6762af63-c8da-487a-9b05-814b18f5b3a9" ],"display_name":"Force rebuild","name":"force-rebuild","values": [ {"value":"false","count":2 } ] }, {"template_ids": ["6762af63-c8da-487a-9b05-814b18f5b3a9" ],"display_name":"Location","name":"location","options": [ {"name":"US (Virginia)","description":"","value":"eastus","icon":"/emojis/1f1fa-1f1f8.png" }, {"name":"US (Virginia) 2","description":"","value":"eastus2","icon":"/emojis/1f1fa-1f1f8.png" }, {"name":"US (Texas)","description":"","value":"southcentralus","icon":"/emojis/1f1fa-1f1f8.png" }, {"name":"US (Washington)","description":"","value":"westus2","icon":"/emojis/1f1fa-1f1f8.png" }, {"name":"US (Arizona)","description":"","value":"westus3","icon":"/emojis/1f1fa-1f1f8.png" }, {"name":"US (Iowa)","description":"","value":"centralus","icon":"/emojis/1f1fa-1f1f8.png" }, {"name":"Canada (Toronto)","description":"","value":"canadacentral","icon":"/emojis/1f1e8-1f1e6.png" }, {"name":"Brazil (Sao Paulo)","description":"","value":"brazilsouth","icon":"/emojis/1f1e7-1f1f7.png" }, {"name":"East Asia (Hong Kong)","description":"","value":"eastasia","icon":"/emojis/1f1f0-1f1f7.png" }, {"name":"Southeast Asia (Singapore)","description":"","value":"southeastasia","icon":"/emojis/1f1f0-1f1f7.png" }, {"name":"Australia (New South Wales)","description":"","value":"australiaeast","icon":"/emojis/1f1e6-1f1fa.png" }, {"name":"China (Hebei)","description":"","value":"chinanorth3","icon":"/emojis/1f1e8-1f1f3.png" }, {"name":"India (Pune)","description":"","value":"centralindia","icon":"/emojis/1f1ee-1f1f3.png" }, {"name":"Japan (Tokyo)","description":"","value":"japaneast","icon":"/emojis/1f1ef-1f1f5.png" }, {"name":"Korea (Seoul)","description":"","value":"koreacentral","icon":"/emojis/1f1f0-1f1f7.png" }, {"name":"Europe (Ireland)","description":"","value":"northeurope","icon":"/emojis/1f1ea-1f1fa.png" }, {"name":"Europe (Netherlands)","description":"","value":"westeurope","icon":"/emojis/1f1ea-1f1fa.png" }, {"name":"France (Paris)","description":"","value":"francecentral","icon":"/emojis/1f1eb-1f1f7.png" }, {"name":"Germany (Frankfurt)","description":"","value":"germanywestcentral","icon":"/emojis/1f1e9-1f1ea.png" }, {"name":"Norway (Oslo)","description":"","value":"norwayeast","icon":"/emojis/1f1f3-1f1f4.png" }, {"name":"Sweden (Gävle)","description":"","value":"swedencentral","icon":"/emojis/1f1f8-1f1ea.png" }, {"name":"Switzerland (Zurich)","description":"","value":"switzerlandnorth","icon":"/emojis/1f1e8-1f1ed.png" }, {"name":"Qatar (Doha)","description":"","value":"qatarcentral","icon":"/emojis/1f1f6-1f1e6.png" }, {"name":"UAE (Dubai)","description":"","value":"uaenorth","icon":"/emojis/1f1e6-1f1ea.png" }, {"name":"South Africa (Johannesburg)","description":"","value":"southafricanorth","icon":"/emojis/1f1ff-1f1e6.png" }, {"name":"UK (London)","description":"","value":"uksouth","icon":"/emojis/1f1ec-1f1e7.png" } ],"values": [ {"value":"eastus","count":2 } ] }, {"template_ids": ["6762af63-c8da-487a-9b05-814b18f5b3a9" ],"display_name":"","name":"mtojek_region","options": [ {"name":"Los Angeles, CA","description":"","value":"Los Angeles, CA","icon":"" }, {"name":"Moncks Corner, SC","description":"","value":"Moncks Corner, SC","icon":"" }, {"name":"Eemshaven, NL","description":"","value":"Eemshaven, NL","icon":"" } ],"values": [ {"value":"Los Angeles, CA","count":2 } ] }, {"template_ids": ["6762af63-c8da-487a-9b05-814b18f5b3a9" ],"display_name":"My Project ID","name":"project_id","values": [ {"value":"12345","count":2 } ] }, {"template_ids": ["6762af63-c8da-487a-9b05-814b18f5b3a9" ],"display_name":"Force devcontainer rebuild","name":"rebuild_devcontainer","values": [ {"value":"false","count":2 } ] }, {"template_ids": ["6762af63-c8da-487a-9b05-814b18f5b3a9" ],"display_name":"Git Repo URL","name":"repo_url","values": [ {"value":"https://github.com/mtojek/coder","count":2 } ] } ] },"interval_reports": []} |
Comment on lines +1231 to +1245
| for _, templateID := range arg.TemplateIDs { | ||
| template, err := q.db.GetTemplateByID(ctx, templateID) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if err := q.authorizeContext(ctx, rbac.ActionUpdate, template); err != nil { | ||
| return nil, err | ||
| } | ||
| } | ||
| if len(arg.TemplateIDs) == 0 { | ||
| if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceTemplate.All()); err != nil { | ||
| return nil, err | ||
| } | ||
| } |
Member
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.
GetTemplatesWithFilter might be more performant here instead of callingGetTemplateByID in a tight loop?
Edit: or you could leverageGetAuthorizedTemplates to check if the caller has the required permissions on the requested templates.
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.
Turns out this wasn't a useful suggestion :(GetTemplatesWithFilter will return an empty slice if you do not have the permissions on any of the supplied TemplateIDs.
| @@ -1227,6 +1227,25 @@ func (q *querier) GetTemplateInsights(ctx context.Context, arg database.GetTempl | |||
| return q.db.GetTemplateInsights(ctx, arg) | |||
| } | |||
| func (q *querier) GetTemplateParameterInsights(ctx context.Context, arg database.GetTemplateParameterInsightsParams) ([]database.GetTemplateParameterInsightsRow, error) { | |||
| for _, templateID := range arg.TemplateIDs { | |||
| template, err := q.db.GetTemplateByID(ctx, templateID) | |||
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.
note: this will generally only be called with a single TemplateID as a parameter, so we hopefully shouldn't see a pathological case of querying hundreds of templates.
johnstcn approved these changesAug 3, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Labels
None yet
3 participants
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.
Uh oh!
There was an error while loading.Please reload this page.
This PR adds support for template parameter insights to the template insights endpoint.
Ref:#8787
Sample output:
{"report": {"start_time":"2023-07-19T00:00:00Z","end_time":"2023-07-21T17:00:00Z","template_ids": [],"active_users":0,"apps_usage": [...],"parameters_usage": [ {"template_ids": ["152bbc74-88ec-4e74-993e-4efe64c8e56f" ],"display_name":"","name":"Region","options": [ {"name":"Pittsburgh","description":"","value":"us-pittsburgh","icon":"/emojis/1f1fa-1f1f8.png" }, {"name":"Helsinki","description":"","value":"eu-helsinki","icon":"/emojis/1f1eb-1f1ee.png" }, {"name":"Sydney","description":"","value":"ap-sydney","icon":"/emojis/1f1e6-1f1fa.png" }, {"name":"São Paulo","description":"","value":"sa-saopaulo","icon":"/emojis/1f1e7-1f1f7.png" } ],"values": [ {"value":"ap-sydney","count":1 } ] }, {"template_ids": ["152bbc74-88ec-4e74-993e-4efe64c8e56f","eca5d911-526f-48ea-9666-bebd90480560" ],"display_name":"Docker image","name":"docker_image","options":null,"values": [ {"value":"alpine:latest","count":1 }, {"value":"codercom/enterprise-base:ubuntu","count":4 } ] }, {"template_ids": ["152bbc74-88ec-4e74-993e-4efe64c8e56f","eca5d911-526f-48ea-9666-bebd90480560" ],"display_name":"dotfiles URI","name":"dotfiles_uri","options":null,"values": [ {"value":"","count":2 }, {"value":"https://github.com/mafredri/dotfiles.git","count":2 }, {"value":"https://github.com/other/dotfiles.git","count":1 } ] } ] },"interval_reports": []}