Support for custom logger in servers

[tanis2000]

This adds support for a custom logger for both the server4 and server6 packages.

Fixes #370.

[pmazzini]

Please sign the commit. Example:

git commit --amend --signoff
git push --force-with-lease origin server-logger

[pmazzini]

Some of the newly added tests are failing on travis.

https://travis-ci.org/github/insomniacslk/dhcp/jobs/661124671?utm_medium=notification&utm_source=github_status

[tanis2000]

Some of the newly added tests are failing on travis.

But they're not failing on my Linux box, apart from an Errorf issue with my version of go apparently. I'm still running go version go1.12.12 linux/amd64 on that box.

$ go test ./...
ok  	github.com/insomniacslk/dhcp/dhcpv4	0.005s
ok  	github.com/insomniacslk/dhcp/dhcpv4/async	0.203s
ok  	github.com/insomniacslk/dhcp/dhcpv4/bsdp	0.004s
?   	github.com/insomniacslk/dhcp/dhcpv4/client4	[no test files]
# github.com/insomniacslk/dhcp/dhcpv4/nclient4
dhcpv4/nclient4/client.go:195:16: Errorf format %w has unknown verb w
dhcpv4/nclient4/client.go:206:16: Errorf format %w has unknown verb w
dhcpv4/nclient4/client.go:219:16: Errorf format %w has unknown verb w
dhcpv4/nclient4/client.go:359:10: Errorf format %w has unknown verb w
dhcpv4/nclient4/client.go:419:9: Errorf format %w has unknown verb w
dhcpv4/nclient4/client.go:425:9: Errorf format %w has unknown verb w
dhcpv4/nclient4/client.go:438:9: Errorf format %w has unknown verb w
dhcpv4/nclient4/client.go:446:9: Errorf format %w has unknown verb w
dhcpv4/nclient4/client.go:452:9: Errorf format %w has unknown verb w
dhcpv4/nclient4/client.go:508:20: Errorf format %w has unknown verb w
FAIL	github.com/insomniacslk/dhcp/dhcpv4/nclient4 [build failed]
ok  	github.com/insomniacslk/dhcp/dhcpv4/server4	0.004s
ok  	github.com/insomniacslk/dhcp/dhcpv4/ztpv4	0.003s
ok  	github.com/insomniacslk/dhcp/dhcpv6	0.007s
ok  	github.com/insomniacslk/dhcp/dhcpv6/async	0.203s
ok  	github.com/insomniacslk/dhcp/dhcpv6/client6	0.005s
ok  	github.com/insomniacslk/dhcp/dhcpv6/nclient6	4.004s
ok  	github.com/insomniacslk/dhcp/dhcpv6/server6	0.003s
ok  	github.com/insomniacslk/dhcp/dhcpv6/ztpv6	0.003s
?   	github.com/insomniacslk/dhcp/examples/client6	[no test files]
?   	github.com/insomniacslk/dhcp/examples/packetcrafting6	[no test files]
?   	github.com/insomniacslk/dhcp/examples/server6	[no test files]
?   	github.com/insomniacslk/dhcp/iana	[no test files]
ok  	github.com/insomniacslk/dhcp/interfaces	0.002s
ok  	github.com/insomniacslk/dhcp/netboot	0.003s
ok  	github.com/insomniacslk/dhcp/rfc1035label	0.003s

Do you have any clue why that might happen on trevis?

[Natolumin]

You have introduced a race condition in the tests with the package-global variable called in server_test, which is accessed possibly concurrently by multiple goroutines.
I think you meant to use customLogger.Called instead of the global variable

You can probably reproduce locally by running go test with -race

[Natolumin]

Interestingly the tests are still racy. I think Serve() spawns a goroutine for each message ?
You'll need to make the logger in the tests properly goroutine-safe with a lock or atomics (eg. atomic.StoreUint32)

[codecov]

Codecov Report

Merging #371 into master will increase coverage by 0.39%.
The diff coverage is 94.54%.

@@            Coverage Diff             @@
##           master     #371      +/-   ##
==========================================
+ Coverage   70.46%   70.86%   +0.39%     
==========================================
  Files          82       84       +2     
  Lines        4212     4256      +44     
==========================================
+ Hits         2968     3016      +48     
+ Misses       1078     1076       -2     
+ Partials      166      164       -2

Impacted Files	Coverage Δ
dhcpv6/server6/logger.go	100% <100%> (ø)
dhcpv4/server4/logger.go	100% <100%> (ø)
dhcpv4/server4/server.go	75.47% <88.88%> (+12.05%)	⬆️
dhcpv6/server6/server.go	59.42% <94.11%> (+12.05%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eed709d...a642904. Read the comment docs.

[tanis2000]

I think I finally managed to get everything working as expected.

[pmazzini]

I think I finally managed to get everything working as expected.

Thanks! I'll take a look soon.

[pmazzini]

lgtm

[pmazzini]

This is good to be merged once Chris's comment is addressed.

[purpleidea]

This looks like an overly complicated version of #373 and in particular exposes the dhcpv4 internals to the logger... Most people working on a logger might not want to expose that. I'd reconsider, but just my opinion. Thanks either way!

[pmazzini]

lgtm

[pmazzini]

Your version is simpler but this one is consistent with what is already done with the nclient and allows to tune how to log the exchanged packets.

I am open to reconsider the interface though.