🌱 Switch to StartLogging for event debug logs#3451
Conversation
|
Hi @timuthy. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
| e, isEvt := event.(*eventsv1.Event) | ||
| if isEvt { | ||
| p.logger.V(1).Info(e.Note, "type", e.Type, "object", e.Related, "action", e.Action, "reason", e.Reason) | ||
| p.logger.V(1).Info(e.Note, "type", e.Type, "object", e.Regarding, "action", e.Action, "reason", e.Reason) |
There was a problem hiding this comment.
Maybe we should do what was suggested in this issue #3407 instead?
There was a problem hiding this comment.
While logs are not really a stable API I would like to avoid changing the log format twice for folks that rely on the format.
I think we can simply implement half of #3407 here without much effort:
stopWatcher, err := p.broadcaster.StartLogging(p.logger)
if err != nil {
p.logger.Error(err, "error starting event watcher for broadcaster")
}
p.stopWatcherFunc = stopWatcher(Let's also rename stopWatcher and p.stopWatcherFunc to stopLogging and p.stopLoggingFunc)
|
/ok-to-test |
f4f33fa to
0434c92
Compare
Regarding for event recorder debug logStartLogging for event debug logs
0434c92 to
4ab2ca8
Compare
|
Thank you! /lgtm /assign @alvaroaleman |
|
LGTM label has been added. DetailsGit tree hash: 54479dd52e65455a38a69e30a55111f2cc27b72e |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alvaroaleman, timuthy 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 |
Switches from an event watcher to
StartLoggingto log events, as proposed by #3407.Fixes #3407