Skip to content
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

proposal: log: add flag for displaying function name #47230

Closed
matusf opened this issue Jul 15, 2021 · 8 comments
Closed

proposal: log: add flag for displaying function name #47230

matusf opened this issue Jul 15, 2021 · 8 comments

Comments

@matusf
Copy link

matusf commented Jul 15, 2021

Proposal

When one wants to have more detailed information from the log messages, they can add Lshortfile or Llongfile flags. However, during debugging, having a function name in logs would provide much better context for the developer than a line number. When having only a line number, developer needs to search the file from logs to gain a better understanding of the error.

I'd propose to add a flag to the log package for displaying function name (Lfuncname). Or two flags analogical to file flags. One that will strip the packages and one without.

Example

package pkg

func do() {
    logger := log.New(os.Stderr, "", log.Lfuncname | log.LstdFlags | log.Lshortfile)
    logger.Println("message")
}
2021/07/15 21:28:13 log_test.go:176: pkg.do(): message
@gopherbot gopherbot added this to the Proposal milestone Jul 15, 2021
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/335469 mentions this issue: log: add Lshortfunc and Llongfunc flags

@mengzhuo
Copy link
Contributor

Hi, I've add a PoC CL above.

@rsc
Copy link
Contributor

rsc commented Aug 25, 2021

/cc @robpike

@robpike
Copy link
Contributor

robpike commented Aug 25, 2021

That's adding an expensive step, an interesting contrast to the other recent logging proposal that tries to make it cheaper.

From an API perspective, you'll soon want more than two new flags to control which function is logged: the one that called log.Print, its parent, and so on. The flags will expand if this is adopted.

The log package is a fine basic tool, which is what it was meant to do. There are many external logging packages that provide much richer functionality, and that's the right place to look for this feature.

Also, note that it's easy to just put the function name in the call itself, either by hand or automation. The log package itself doing would only add ease, not ability.

@rsc
Copy link
Contributor

rsc commented Sep 1, 2021

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Sep 8, 2021

I am not sure this is worthwhile, since the function name can get quite long, methods have awkward names, closures have no real name, and so on. The file:line is far more precise and useful.


That said, it's not expensive, nor does it add more API complexity than the extra flag bit. Presumably you'd want file/line/function to always be in sync, so there wouldn't be an extra 'which function' knob beyond the existing one that controls file/line (the depth argument to Output). Today the code does:

	_, file, line, ok = runtime.Caller(calldepth)

It would have to use the ignored result (pc) and call runtime.FuncForPC to get the name. The file/line lookup was much more expensive than resolving the pc to the function name.

So it's not expensive. It just may not be terribly useful.

@rsc
Copy link
Contributor

rsc commented Sep 15, 2021

Based on the discussion above, this proposal seems like a likely decline.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Sep 22, 2021

No change in consensus, so declined.
— rsc for the proposal review group

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants