Make logging requests configurable#51
Conversation
ShimmerGlass
left a comment
ShimmerGlass
left a comment
There was a problem hiding this comment.
Thanks for the PR, I've left a few comments.
gdubicki
force-pushed
the
log-requests
branch
2 times, most recently
from
e7496dc to
5aa567e
Compare
main.go
Outdated
| fi, err := os.Stat(logAddress) | ||
| if err != nil { | ||
| // maybe it's a syslog host[:port] | ||
| if len(strings.Split(logAddress, ":")) == 1 { |
There was a problem hiding this comment.
Use net.SplitHostPort() here instead to properly handle IPv6
There was a problem hiding this comment.
Thanks! But it turned out to be complicated to check if the error out of net.SplitHostPort() is "missing port" so I did something else, I think that it's even more elegant. :) Can you please check this code now?
I also added one more case that HAproxy supports - file descriptor/stdout/stderr.
There was a problem hiding this comment.
Is the current validation code ok for you, @ShimmerGlass ?
gdubicki
force-pushed
the
log-requests
branch
2 times, most recently
from
5c55e55 to
8076e1d
Compare
not requiring the app logging level to be set to TRACE and with configurable log address (stdout/stderr/syslog etc.)
|
I think that in the long term work on making the |
| } | ||
| } else { | ||
| if fi.Mode()&os.ModeSocket == 0 { | ||
| return errors.New(fmt.Sprintf("%s is a file but not a socket", logAddress)) |
There was a problem hiding this comment.
You can use fmt.Errorf() to directly create an error with formatting
| // allowed values taken from https://cbonte.github.io/haproxy-dconv/2.0/configuration.html#4.2-log | ||
| fi, err := os.Stat(logAddress) | ||
| if err != nil { | ||
| match, err := regexp.Match(`(fd@<[0-9]+>|stdout|stderr)`, []byte(logAddress)) |
There was a problem hiding this comment.
Please use regex.MatchString to remove the type casting
|
@gdubicki if you'd still like to incorporate the suggestions by Thorleon we could proceed with merging this. |
4 participants
...not requiring the app logging level to be set to TRACE and with configurable log address - syslog socket.
I think that this fixes #45.
As this is my first contribution and one of the first golang programming attempts please do not hesitate to write me everything that I should fix before this can be merged - I will do by best to apply all the comments. :)