Alternate, map-based sugared logger

[akshayjshah]

This PR is a sibling to #185; only one of the two should be merged. It explores a different sugaring API, using map[string]interface{} to pass fields and string to pass messages.

This avoids allocating to pass strings as interface{} (for both the message and the field keys). In our BenchmarkZapSugarAddingFields benchmark, this saves roughly 1 microsecond and 20 small allocations at each log site. However, it actually allocates more memory overall, increases verbosity at log sites, decreases flexibility, and necessitates at least seven additional methods on the sugared logger.

IMO, this isn't a good tradeoff.

BenchmarkLogrusAddingFields-4                         	  200000	      9601 ns/op	    5727 B/op	      77 allocs/op
BenchmarkLogrusWithAccumulatedContext-4               	  200000	      7622 ns/op	    3919 B/op	      61 allocs/op
BenchmarkLogrusWithoutFields-4                        	 1000000	      2222 ns/op	    1321 B/op	      25 allocs/op

BenchmarkZapAddingFields-4                            	 1000000	      1260 ns/op	     768 B/op	       4 allocs/op
BenchmarkZapWithAccumulatedContext-4                  	 5000000	       325 ns/op	      64 B/op	       2 allocs/op
BenchmarkZapWithoutFields-4                           	 5000000	       339 ns/op	      64 B/op	       2 allocs/op

BenchmarkZapSugarAddingFields-4                       	  500000	      2924 ns/op	    1729 B/op	      15 allocs/op
BenchmarkZapSugarWithAccumulatedContext-4             	 5000000	       370 ns/op	      64 B/op	       2 allocs/op
BenchmarkZapSugarWithoutFields-4                      	 5000000	       378 ns/op	      64 B/op	       2 allocs/op

[mention-bot]

@akshayjshah, thanks for your PR! By analyzing the history of the files in this pull request, we identified @jcorbin, @prashantv and @pravj to be potential reviewers.

[prashantv]

That is a lot of methods, I don't think we want that large of an API.

How about an approach that is more similar to log15, where you always take varargs, and the user can choose to pass a single Ctx map, or they can pass a bunch of field/values. Then the API for each level should only need:

Panic(msg string, ctx... interface{})
Panicf(fmt string, args... interface{})

[jcorbin]

I agree that this doesn't seem worth it to me, but the biggest problem I see is that the caller skip isn't right:

• Debug calls log calls Check.Write, needing +1 skip
• DebugWith calls log ... needing +1 skip
• DebugF calls Debug calls log ... needing +2 skip

We currently don't have support for dynamically varied caller skip, and cannot without moving caller annotation out of logger.check (e.g. defer it until Check.Write time, when we have access to the []Field).

[akshayjshah]

Fixed the caller skip - totally an oversight on my part. Don't think that this requires a facility for dynamic skips; the current situation is, IMO, simple and quite tolerable.