Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(kuma-cp) more descriptive KDS logging #1135

Merged
merged 3 commits into from
Nov 17, 2020
Merged

Conversation

jakubdyszkiewicz
Copy link
Contributor

Summary

A couple of improvements to KDS logging

  • Logs on INFO level should be understandable by first-time users of Kuma. Full XDS lifecycle with words like DiscoveryRequest, ACK, NACK etc. should be hidden on debug level just like it is for data plane exchange
  • I think on INFO users should really care about logs if there are new changes in either control plane.
  • Fixed a problem when disconnecting Remote results in error in logs

Issues resolved

Fix #1081

Documentation

  • No docs, internal change only

Signed-off-by: Jakub Dyszkiewicz <jakub.dyszkiewicz@gmail.com>
@jakubdyszkiewicz jakubdyszkiewicz requested a review from a team as a code owner November 9, 2020 12:32
@@ -47,10 +47,12 @@ func (s *kdsSink) Start(stop <-chan struct{}) (errs error) {

clusterID, rs, err := s.kdsStream.Receive()
if err != nil {
if err == io.EOF {
break
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we break on EOF? I guess s.kdsStream.Receive() on the next iteration will return EOF as well

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There won't be the next iteration without this if, it will use

return errors.Wrap(err, "failed to receive a discovery response")

@jakubdyszkiewicz jakubdyszkiewicz merged commit febf748 into master Nov 17, 2020
@jakubdyszkiewicz jakubdyszkiewicz deleted the chore/kds-logging branch November 17, 2020 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cleaner logs when Remote CP goes down
3 participants