Skip to content

config: change log level to debug #3738

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.

Sign up for GitHub

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

Merged
htuch merged 1 commit into from
Jun 27, 2018
Merged

config: change log level to debug #3738

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion source/common/config/grpc_mux_subscription_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class GrpcMuxSubscriptionImpl : public Subscription<ResourceType>,
// TODO(htuch): Less fragile signal that this is failure vs. reject.
if (e == nullptr) {
stats_.update_failure_.inc();
ENVOY_LOG(warn, "gRPC update for {} failed", type_url_);
ENVOY_LOG(debug, "gRPC update for {} failed", type_url_);
Copy link
Member

@htuch htuch Jun 27, 2018

Choose a reason for hiding this comment

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

Can you distinguish between the very first update and alter updates? In particular, I think we should warn when a management server is unreachable etc. but not during normal disconnects (e.g. gRPC load shed).

Copy link
Contributor Author

@ramaraochavali ramaraochavali Jun 27, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

If it is not unreachable during Envoy startup, we get the following warning already
[2018-06-27 13:47:24.884][27][warning][upstream] source/common/config/grpc_mux_impl.cc:235] gRPC config stream closed: 14, upstream connect error or disconnect/reset before headers

Is this sufficient? or are you thinking of some thing else?

If we do not use ADS, individual xDS apis may be still logging this for first unreachability also?

Copy link
Contributor Author

@ramaraochavali ramaraochavali Jun 27, 2018

Choose a reason for hiding this comment

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

Ok. So do you want this to be warn till at least one successful update has happened or one update attempt? Based on that, I think we can evaluate the corresponding stat and distinguish between debug and warn.

Copy link
Member

@htuch htuch Jun 27, 2018

Choose a reason for hiding this comment

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

OK, seems fine then if we are already observing a connection level warning.

} else {
stats_.update_rejected_.inc();
ENVOY_LOG(warn, "gRPC config for {} rejected: {}", type_url_, e->what());
Expand Down