labstack / gommon

Common packages for Go
MIT License
536 stars 100 forks source link

gommon/log | global caller notification #21

Closed alexandrestein closed 6 years ago

alexandrestein commented 6 years ago

This changes the behavior of global calls like log.Print() for example.

This is what happens when you display the test buffer. In first place this the result of the test using the test function. And then the result when you call the function directly.

=== RUN   TestGlobal
{"time":"2018-04-27T13:01:00.592958006+02:00","level":"-","prefix":"-","file":"log_test.go","line":"20","message":"print"}
{"time":"2018-04-27T13:01:00.59296855+02:00","level":"-","prefix":"-","file":"log_test.go","line":"21","message":"printf"}
{"time":"2018-04-27T13:01:00.592990136+02:00","level":"WARN","prefix":"-","file":"log_test.go","line":"26","message":"warn"}
{"time":"2018-04-27T13:01:00.592999838+02:00","level":"WARN","prefix":"-","file":"log_test.go","line":"27","message":"warnf"}
{"time":"2018-04-27T13:01:00.593007922+02:00","level":"ERROR","prefix":"-","file":"log_test.go","line":"28","message":"error"}
{"time":"2018-04-27T13:01:00.593021023+02:00","level":"ERROR","prefix":"-","file":"log_test.go","line":"29","message":"errorf"}

{"time":"2018-04-27T13:01:00.593067503+02:00","level":"-","prefix":"-","file":"log.go","line":"267","message":"print"}
{"time":"2018-04-27T13:01:00.593078668+02:00","level":"-","prefix":"-","file":"log.go","line":"271","message":"printf"}
{"time":"2018-04-27T13:01:00.593100463+02:00","level":"WARN","prefix":"-","file":"log.go","line":"303","message":"warn"}
{"time":"2018-04-27T13:01:00.593106604+02:00","level":"WARN","prefix":"-","file":"log.go","line":"307","message":"warnf"}
{"time":"2018-04-27T13:01:00.593111993+02:00","level":"ERROR","prefix":"-","file":"log.go","line":"315","message":"error"}
{"time":"2018-04-27T13:01:00.593116528+02:00","level":"ERROR","prefix":"-","file":"log.go","line":"319","message":"errorf"}

--- PASS: TestGlobal (0.00s)

Display the line in log.go is not really useful. :smile: After this change the buffer display this:

=== RUN   TestGlobal
{"time":"2018-04-27T13:04:25.063464991+02:00","level":"-","prefix":"-","file":"log_test.go","line":"53","message":"print"}
{"time":"2018-04-27T13:04:25.06347601+02:00","level":"-","prefix":"-","file":"log_test.go","line":"53","message":"printf"}
{"time":"2018-04-27T13:04:25.063489653+02:00","level":"WARN","prefix":"-","file":"log_test.go","line":"53","message":"warn"}
{"time":"2018-04-27T13:04:25.06349519+02:00","level":"WARN","prefix":"-","file":"log_test.go","line":"53","message":"warnf"}
{"time":"2018-04-27T13:04:25.063503081+02:00","level":"ERROR","prefix":"-","file":"log_test.go","line":"53","message":"error"}
{"time":"2018-04-27T13:04:25.063514465+02:00","level":"ERROR","prefix":"-","file":"log_test.go","line":"53","message":"errorf"}

{"time":"2018-04-27T13:04:25.063554739+02:00","level":"-","prefix":"-","file":"log_test.go","line":"60","message":"print"}
{"time":"2018-04-27T13:04:25.063565835+02:00","level":"-","prefix":"-","file":"log_test.go","line":"61","message":"printf"}
{"time":"2018-04-27T13:04:25.06358589+02:00","level":"WARN","prefix":"-","file":"log_test.go","line":"66","message":"warn"}
{"time":"2018-04-27T13:04:25.063593027+02:00","level":"WARN","prefix":"-","file":"log_test.go","line":"67","message":"warnf"}
{"time":"2018-04-27T13:04:25.063597844+02:00","level":"ERROR","prefix":"-","file":"log_test.go","line":"68","message":"error"}
{"time":"2018-04-27T13:04:25.063602406+02:00","level":"ERROR","prefix":"-","file":"log_test.go","line":"69","message":"errorf"}

--- PASS: TestGlobal (0.00s)

We can argue the way the test is done. Because if the file is updated the lines numbers will probably change and the test will fails. We can maybe move it in its own test file.

coveralls commented 6 years ago

Coverage Status

Coverage increased (+5.6%) to 75.258% when pulling 6bfc963b8d77568395d0bee2c6d0bbee5ce80cb4 on alexandreStein:master into 588f4e8bddc6cb45c27b448e925c7fd6a5545434 on labstack:master.

vishr commented 6 years ago

@alexandreStein I will have a look at it soon. Just trying to understand the problem :)

vishr commented 6 years ago

@alexandreStein I got it covered in https://github.com/labstack/gommon/commit/0a22a0df01a7c84944c607e8a6e91cfe421ea7ed, thanks for your contribution.