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

proxylib: Downgrade noisy log msg to debug level #22848

Merged

Conversation

christarazi
Copy link
Member

The log msg doesn't provide any useful / actionable information anyway,
presumably unless you're debugging.

Fixes: 69518a1 ("envoy: API update")

Signed-off-by: Chris Tarazi chris@isovalent.com

The log msg doesn't provide any useful / actionable information anyway,
presumably unless you're debugging.

Fixes: 69518a1 ("envoy: API update")

Signed-off-by: Chris Tarazi <chris@isovalent.com>
@christarazi christarazi added area/proxy Impacts proxy components, including DNS, Kafka, Envoy and/or XDS servers. kind/enhancement This would improve or streamline existing functionality. release-note/misc This PR makes changes that have no direct user impact. labels Dec 22, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot added dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. and removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Dec 22, 2022
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Dec 22, 2022
@christarazi christarazi removed the kind/community-contribution This was a contribution made by a community member. label Dec 22, 2022
@christarazi christarazi marked this pull request as ready for review December 22, 2022 20:31
@christarazi christarazi requested a review from a team as a code owner December 22, 2022 20:31
@@ -183,7 +183,7 @@ func (ins *Instance) PolicyUpdate(resp *envoy_service_discovery.DiscoveryRespons
return fmt.Errorf("NPDS: Policy has no endpoint_ips")
}
for _, ip := range ips {
logrus.Infof("NPDS: Endpoint IP: %s", ip)
logrus.Debugf("NPDS: Endpoint IP: %s", ip)
Copy link
Member

@sayboras sayboras Dec 23, 2022

Choose a reason for hiding this comment

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

this is un-related to this PR, we can just have one single log entry to print all ips together, something like below

logrus.Infof("NPDS: Endpoint IP: %v", strings.Join(ips, ","))

or just

logrus.Debugf("NPDS: Endpoint IP: %v", ips)

@christarazi
Copy link
Member Author

Downgrading a log msg level shouldn't need a full CI run. We have an approving review above, marking ready to merge.

@christarazi christarazi added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Dec 23, 2022
@pchaigno pchaigno merged commit c3e5729 into cilium:master Jan 2, 2023
@pchaigno
Copy link
Member

pchaigno commented Jan 2, 2023

cc @jrajahalme

@christarazi christarazi deleted the pr/christarazi/proxylib-info-log branch January 3, 2023 20:34
@sayboras sayboras added needs-backport/1.12 needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch labels Mar 15, 2023
@sayboras
Copy link
Member

Nominate this PR for 1.13 and 1.12 backport, the main reason is to avoid noisy logs, which might cause issue for users to overlook the actual error.

@NikAleksandrov NikAleksandrov mentioned this pull request Mar 20, 2023
9 tasks
@NikAleksandrov NikAleksandrov mentioned this pull request Mar 23, 2023
29 tasks
@NikAleksandrov NikAleksandrov added backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. and removed needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch labels Mar 23, 2023
@youngnick youngnick added backport-done/1.12 The backport for Cilium 1.12.x for this PR is done. and removed backport-pending/1.12 labels Mar 23, 2023
@jibi jibi added the backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. label Apr 7, 2023
@jibi jibi removed the backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. label Apr 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/proxy Impacts proxy components, including DNS, Kafka, Envoy and/or XDS servers. backport-done/1.12 The backport for Cilium 1.12.x for this PR is done. backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. kind/enhancement This would improve or streamline existing functionality. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
Status: Released
Development

Successfully merging this pull request may close these issues.

None yet

6 participants