Really controversial topic: completely remove panic/fatal?

[bufdev]

I'm sure this won't happen pre-V1, but controversial opinion: I think people use logger.Panic/Fatal way too much, and it breaks execution flow of a program, it's harder to reason about, and now with zap especially, it's weird that there are situations where you can log at panic level but not actually panic. I think if you really want to panic or os.Exit(1), you should do it manually (and rarely, basically in the main function only).

Since zap will be used a lot with services, who should almost never panic anyways, might it encourage good behavior to completely remove this ability, and if someone really wants to panic or exit, they can do it manually?

[akshayjshah]

While I agree that services should use Panic and Fatal sparingly, it's important to support them natively - without this support, service owners not only need to call panic or os.Exit themselves, they also need to sync any open file descriptors, flush Sentry reporting buffers, etc., or they'll lose the reason for the crash. That's difficult to do from many locations in the code without passing a number of file-descriptor-like structs (which zap models as WriteSyncers) around with the loggers.

Unless the service owner is also doing evil things with recover, I don't find that Panic and Fatal make programs more difficult to reason about; it just means that sometimes there's a hard crash.

[jcorbin]

If it helps, we're planning to drop support to "log at panic level without panicing" (likewise for fatal) before 1.0 (WIP in #201).

[rogpeppe]

FWIW I agree with the sentiment of this issue. I don't think that any service should ever call exit, and panic should be reserved for truly exceptional cases. By putting Panic into the logger, ISTM that we're pretending that the only panics that happen are those that are called explicitly, whereas of course panics can happen for other reasons too - I don't think there's a particularly good rationale for saying that one kind of panic should be treated differently from another.

[bufdev]

Ya, I like having at least a "style guide" ban on doing Fatal/Panic. I see a lot of golang developers using Fatal/Panic freely (especially new gophers) without thinking about the implications - I'd do as far as to wish that the stdlib log did not split out into Print/Fatal/Panic method types.

[akshayjshah]

Again, I'm generally sympathetic to this strain of thought. However, I feel strongly that there's an important place for at least panic-level logging in the main method of server processes: if the initial configuration is nonsensical, I'd much prefer a hard crash that signals the scheduling and deployment system to halt rollout. I don't think that a blanket ban on panic- and fatal-level logging is nuanced enough to actually capture best practice here.

It's also strictly worse to force developers who do want to panic or exit to do so manually, since they'll often lose the critical log messages immediately preceding the crash. Integration with the logging library allows us to flush file handles, network buffers, and the like before we actually crash. @peter-edge, this has been a long-running pain point with our internal use of logrus.

In sum: yes, these methods should be used very sparingly, and only in exceptional circumstances. When they are necessary, though, it's better to have them correctly integrated.