fix(kuma-dp): reduce size of access log address #894
Conversation
When the name of the dataplane is too long an error stop the side-car from starting. This update reduces the size of the address and log an explicit error if the address is still too long. * reduce the size of access log address replacing 'kuma-access-logs' by 'kal' * add explicit error log if the size of the address is still too long Fix kumahq#853
xbauquet
force-pushed
the
fix/kumadp-access-log-address
branch
from
92e3145
to
7cf3fc3
Compare
|
|
||
| // Log an explicit error when access log address is too long | ||
| // see: https://man7.org/linux/man-pages/man7/unix.7.html | ||
| // see: issue #853 |
There was a problem hiding this comment.
Please remove the reference to the issue here. The best way to track this will be through following the git history of this particular piece of code.
|
Great! @xbauquet could you please run |
go.mod
Outdated
| @@ -22,7 +22,7 @@ require ( | |||
| github.com/go-logr/zapr v0.1.0 | |||
| github.com/gogo/protobuf v1.3.1 | |||
| github.com/golang-migrate/migrate/v4 v4.8.0 | |||
| github.com/golang/protobuf v1.3.2 | |||
| github.com/golang/protobuf v1.4.1 | |||
There was a problem hiding this comment.
What is the reason of this change in this PR? AFAIR we had some problems with 1.3.x -> 1.4.x upgrade
There was a problem hiding this comment.
No good reasons, this came with the make check requested by @lobkovilya.
I'm having troubles running make check anyway. It always finishes with make: *** [vet] Error but I don't have any error logs on my terminal.
I'm a bit lost on it as I don't know much about go.
There was a problem hiding this comment.
It seems like it was a problem with the versions of the tools installed on my computer.
Using make dev/tools/all and redoing the make check fixed the problem.
| Please reduce the size of your dataplane name.`, | ||
| dataplane.Name, | ||
| address)) | ||
| } |
There was a problem hiding this comment.
Can we do something more automatic here?
We are not so attached to this name, to be honest. We can just as well either:
- trim the name to 108 chars
- use UUID
As long as we log this path to the file, it's fine.
There was a problem hiding this comment.
I thought about using a UUID but I thought it would complicate debug.
In which file do you want to log the path ? In the normal logs (using logger.Info(...) ) ?
xbauquet
force-pushed
the
fix/kumadp-access-log-address
branch
from
eb1630d
to
09a47a1
Compare
| var address = fmt.Sprintf("/tmp/%s.sock", id) | ||
| logger.Info("Access log for pod", | ||
| "pod", fmt.Sprintf("/tmp/kal-%s-%s.sock", dataplane.Name, dataplane.Mesh), | ||
| "address", fmt.Sprintf("unix://%s", address)) |
There was a problem hiding this comment.
Log is not needed here after all. We log the address on server start
| @@ -33,10 +34,15 @@ func (s *accessLogServer) NeedLeaderElection() bool { | |||
| } | |||
|
|
|||
| func NewAccessLogServer(dataplane kumadp.Dataplane) *accessLogServer { | |||
| id := guuid.New() | |||
There was a problem hiding this comment.
please use core.NewUUID() (from /pkg/core/alias.go)
xbauquet
force-pushed
the
fix/kumadp-access-log-address
branch
2 times, most recently
from
56dd0da
to
db0f859
Compare
xbauquet
force-pushed
the
fix/kumadp-access-log-address
branch
from
db0f859
to
438ed97
Compare
Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
This reverts commit 2bec835. # Conflicts: # app/kuma-dp/pkg/dataplane/accesslogs/server.go Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
When the name of the dataplane is too long an error stop the side-car
from starting. This update reduces the size of the address and log an
explicit error if the address is still too long.
'kal'
Fix #853