(2.11.x) [log] Add TraceHeaders option #6638
Conversation
|
I like this, but defer for 2.11.x? |
|
@ripienaar agreed. will update the title to indicate |
bruth
changed the title
|
@bruth Would it be possible to include a reproducible test plan in the PR description? It might be clear to experienced reviewers, but for those of us still ramping up, having explicit commands and expected output would help us understand the behavior change. It would also make it easier to communicate updates to customers and save us some cycles. Thanks! |
|
Thanks @kozlovic, will review shortly! Yes @alexbozhenko I will add a test for this, so the expected behavior is clear. |
bruth
force-pushed
the
trace-log-headers
branch
from
752259f to
783929d
Compare
|
@kozlovic I went with the second approach you suggested. I am going to add a test to exercise this. |
| @@ -1917,9 +1917,9 @@ func (c *client) traceMsg(msg []byte) { | |||
|
|
|||
| if maxTrace > 0 && l > maxTrace { | |||
| tm := fmt.Sprintf("%q", msg[:maxTrace]) | |||
| c.Tracef("<<- MSG_PAYLOAD: [\"%s...\"]", tm[1:maxTrace+1]) | |||
| c.Tracef("<<- MSG_PAYLOAD: [\"%s...\"]", tm[1:len(tm)-1]) | |||
There was a problem hiding this comment.
Changed this since when converting to the quoted form, \r\n is escaped and thus four characters.
server/client_test.go
Outdated
| Desc: "payload only", | ||
| Msg: []byte(`test\r\n`), | ||
| Hdr: 0, | ||
| Wanted: "", |
There was a problem hiding this comment.
Derek would say to replace "" with _EMPTY (applies to all in this test)
|
It is going to be more helpful for investigations if we trace the headers when they are delivered as well with an option, that is the current limitation of tracing (since both payloads and headers are not shown on the receiving side). |
I am not opposed, but to be clear, this is net new? We can do this in a follow-up PR. Edit: How does this overlap with the debug message tracing functionality? |
|
Please rebase on top of latest main and will queue up for merge, thanks! |
bruth
force-pushed
the
trace-log-headers
branch
2 times, most recently
from
6aca199 to
804e81b
Compare
When true, trace logs will no longer emit the payload of the message, only the headers if these are present. An additional change, is not emitting the MSG_PAYLOAD log line if the payload is empty. Signed-off-by: Byron Ruth <[email protected]>
bruth
force-pushed
the
trace-log-headers
branch
from
804e81b to
4799ca8
Compare
When true, trace logs will no longer emit the payload of the message, only the headers if these are present.
An additional change, is not emitting the MSG_PAYLOAD log line if the payload is empty.