Fix server.address field in HTTP logs #4142
Conversation
michel-laterman
added
bug
|
This pull request does not have a backport label. Could you fix it @michel-laterman? 🙏
|
|
|
mergify
bot
added
the
backport-8.x
internal/pkg/logger/http.go
Outdated
| if zlog.Info().Enabled() { | ||
| e.Uint64(ECSHTTPRequestBodyBytes, rdCounter.Count()) | ||
| e.Uint64(ECSHTTPResponseBodyBytes, wrCounter.Count()) | ||
| e.Int(ECSHTTPResponseCode, wrCounter.statusCode) | ||
| e.Int64(ECSEventDuration, time.Since(start).Nanoseconds()) | ||
|
|
||
| e.Msgf("%d HTTP Request", wrCounter.statusCode) | ||
| e.Msg("HTTP Request") |
There was a problem hiding this comment.
I've changed the middleware layer to log an info level message for each request if info is enabled.
The previous behaviour was to log an (info level) message if Debug is enabled, or the server responded in a non 2xx status
There was a problem hiding this comment.
Are we sure this is a good idea? How frequent would these logs be in the median case?
There was a problem hiding this comment.
It depends on the number of connected agents.
IIRC agents will long poll 10m by default, so we would expect a log every 10m per agent for checkins + additional entries for each action's acks (when an action or policy change is sent)
There was a problem hiding this comment.
That sounds like potentially quite a bit. Could we rate limit or aggregate these based on return code?
There was a problem hiding this comment.
FYI, I'm fine with the rest of this PR, it's just this change that seems potentially dangerous.
There was a problem hiding this comment.
actually the previous condition is broken; it would only output the (info level) message if debug is enabled.
There was a problem hiding this comment.
i'll revert & fix the condition; then make an issue for sampling logs
EDIT:
issue: #4145
|
michel-laterman
added
backport-8.16
* Fix server.address field in HTTP logs, fix request log condition (cherry picked from commit 4132a50)
* Fix server.address field in HTTP logs, fix request log condition (cherry picked from commit 4132a50)
* Fix server.address field in HTTP logs, fix request log condition (cherry picked from commit 4132a50)
* Fix server.address field in HTTP logs, fix request log condition (cherry picked from commit 4132a50) Co-authored-by: Michel Laterman <[email protected]>
* Fix server.address field in HTTP logs, fix request log condition (cherry picked from commit 4132a50) Co-authored-by: Michel Laterman <[email protected]>
* Fix server.address field in HTTP logs, fix request log condition (cherry picked from commit 4132a50) Co-authored-by: Michel Laterman <[email protected]>
What is the problem this PR solves?
server.addressis empty in HTTP request logs.How does this PR solve the problem?
Populate
server.addressusing the interface associated withhttp.LocalAddrContextKey.How to test this PR locally
start a fleet-server and make an HTTP request.
Design Checklist
Checklist
I have made corresponding changes to the documentationI have made corresponding change to the default configuration files./changelog/fragmentsusing the changelog tool