- Notifications
You must be signed in to change notification settings - Fork1k
chore: add profiling labels for pprof analysis#19232
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
PProf labels segment the code into groups for determing the sourceof cpu/memory profiles. Since the web server and background jobsshare a lot of the same code (eg wsbuilder), it helps to knowif the load is user induced, or background job based.
Emyrk commentedAug 7, 2025 • 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.
I'm sure the |
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.
Only nits, this looks rad
enterprise/coderd/coderd.go Outdated
| api.AGPL.PrebuildsReconciler.Store(&reconciler) | ||
| goreconciler.Run(context.Background()) | ||
| // TODO: Should this context be the app context? |
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.
What is the "app" referring to 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.
api.ctx.
I just noticed the prebuild routine does not get the app context shutdown.
| } | ||
| const ( | ||
| ServiceTerraformProvisioner="terraform-provisioner" |
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.
Follow-up: would be cool to label pubsub as well.
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.
Which pubsub? The database pubsub listeners?
coder/coderd/database/pubsub/pubsub.go
Line 246 incb1ec21
| func (p*PGPubsub)subscribeQueue(eventstring,newQ*msgQueue) (cancelfunc(),errerror) { |
Uh oh!
There was an error while loading.Please reload this page.
| r.Use( | ||
| // TODO: @emyrk Should we standardize these in some other package? | ||
| httpmw.Recover(s.Logger), | ||
| httpmw.WithProfilingLabels, |
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.
Worth discriminating on ws vs https traffic?
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.
Oh that is a really good idea. They should have that upgrade header iirc. Will add
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.
request_type: Total 730.0ms 550.0ms (75.34%): http 180.0ms (24.66%): websocketservice: Total 810.0ms 730.0ms (90.12%): http-api 80.0ms ( 9.88%): terraform-provisioner8ba8b4f intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
PProf labels segment the code into groups for determing the source of cpu/memory profiles. Since the web server and background jobs share a lot of the same code (eg wsbuilder), it helps to know if the load is user induced, or background job based.
Example output: