-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add rpc duration logging for chat_local #6108
base: master
Are you sure you want to change the base?
Conversation
// perfLog logs function call durations in ms. | ||
// example usage: defer h.perfLog("MyFunc", time.Now()) | ||
func (h *chatLocalHandler) perfLog(name string, start time.Time) { | ||
h.G().Log.Debug("ChatPerf %s: %d", name, time.Since(start)/time.Millisecond) |
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.
Is there any use case where start
would not be time.Now
?
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.
it has to be a parameter so that the defer works.
but yes, the internal ones inside NewConversationLocal don't use time.Now().
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.
Could do something like:
func (h *chatLocalHandler) perfLog(name string) func() {
start := time.Now()
return func() {
h.G().Log.Debug("ChatPerf %s: %d", name, time.Since(start)/time.Millisecond)
}
}
...
defer h.perfLog("mike")()
Up to you though.
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.
But yeah, doesn't make sense if this is used inside the function too.
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.
Yeah, that's similar to the Trace func we have, but sometimes we forget to call the function it returns :) ...i.e. compiler doesn't complain about h.perfLog("mike")
.
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.
Yep, true!
What's the status of this PR? |
No description provided.