textlogger: optionally turn off header#430
Conversation
k8s-ci-robot
added
needs-triage
k8s-ci-robot
added
approved
|
/assign @harshanarayana |
|
/triage accepted |
k8s-ci-robot
added
triage/accepted
|
/assign |
nojnhuh
left a comment
nojnhuh
left a comment
There was a problem hiding this comment.
/lgtm
Left a couple non-blocking comments.
/hold In case you want to make any changes here still.
| // Determine caller. | ||
| // +1 for this frame, +1 for Info/Error. |
There was a problem hiding this comment.
nit: should we keep this comment immediately above l.callDepth + 2?
| ) | ||
| logger := textlogger.NewLogger(config) | ||
|
|
||
| logger.Error(errors.New("fake error"), "Something broke", "id", 42) |
There was a problem hiding this comment.
Is it worth also checking that these other methods don't somehow lose the withHeader? Maybe in a separate test if this is too busy for an example?
logger.WithName("name").WithValues("key", "value").WithCallDepth(0).Info("Still no header")There was a problem hiding this comment.
A few more lines seem fine.
While looking at this again I started wondering why I was redirecting to a buffer - no good reason, so now I am using os.Stdout directly, which saves some lines again.
There was a problem hiding this comment.
Actually, it's because WithHeader was meant to be used in combination with buffer as a general-purpose "structured" -> "string" converter.
I've added a comment to the docs and kept the example with bytes.Buffer.
k8s-ci-robot
added
the
do-not-merge/hold
k8s-ci-robot
added
the
lgtm
This is useful for converting structured log parameters (e.g. error + message + key/value pairs from the Kubernetes ErrorHandler API) into a single string.
pohly
force-pushed
the
textlogger-optional-header
branch
from
1d414ff to
36bc4ff
Compare
k8s-ci-robot
removed
the
lgtm
k8s-ci-robot
added
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: nojnhuh, pohly The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
6 participants
What this PR does / why we need it:
Turning off the header is useful for converting structured log parameters (e.g. error + message + key/value pairs from the Kubernetes ErrorHandler API) into a single string.
See
Which issue(s) this PR fixes:
Fixes #429
Special notes for your reviewer:
Needed for https://github.com/kubernetes/kubernetes/pull/135419/files#r2576981852
Release note: