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

Make DPanic an actual level and replace DFatal #175

Closed
jcorbin opened this issue Nov 9, 2016 · 2 comments
Closed

Make DPanic an actual level and replace DFatal #175

jcorbin opened this issue Nov 9, 2016 · 2 comments

Comments

@jcorbin
Copy link
Contributor

jcorbin commented Nov 9, 2016

DFatal not being an explicit log level, but instead an implementation detail of Meta, hurts composability:

@jcorbin
Copy link
Contributor Author

jcorbin commented Nov 9, 2016

My proposal:

  • add DFatalLevel before PanicLevel, or before FatalLevel (putting it after FatalLevel requires more code change, as FatalLevel is assumed maximal; yes that means we're missing a semantic name for "maximal logging level", but that's another issue ;-) )
  • add a Meta.DFatal(log, msg, fields) convenience method that can be used to ease implementation (e.g. func (log TConcreteLogger) DFatal(...) { log.Meta.DFatal(log, ...) }), all it will do is delegate to logger.Error or logger.Fatal depending on Meta.Development

@akshayjshah akshayjshah changed the title Fix DFatal (espec. wrt sampled logging) Make DPanic an actual level and replace DFatal Nov 29, 2016
@akshayjshah
Copy link
Contributor

@jcorbin Plan makes sense, but let's replace DFatal with DPanic. It accomplishes the same blow-up-in-dev goal (and actually dumps a stack trace with the panic), and it fits more neatly into the level hierarchy between ErrorLevel and PanicLevel.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants