-
Notifications
You must be signed in to change notification settings - Fork 242
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
fix: [NPM] Reduce/Refactor Noisy NPM Logs #3468
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Overview
This PR aims to reduce the noisy NPM logs by removing numerous informational logging (klog.Infof) statements and refactoring logging in key components.
- Removed debug/info logs from chain management, policy management, and IP set operations.
- Refactored log statements in the dataplane and dpshim code to promote a cleaner production log output.
- Eliminated extraneous log output in auxiliary components including the parser and dirty cache management.
Reviewed Changes
File | Description |
---|---|
npm/pkg/dataplane/policies/chain-management_linux.go | Removed multiple klog.Infof statements in bootup, cleanup, and chain positioning functions to reduce clutter. |
npm/pkg/dataplane/types.go | Eliminated excessive logging in updatePodCache and netPolQueue functions. |
npm/pkg/dataplane/dpshim/dpshim.go | Cleared out informational logs in HydrateClients, deleteIPSet, and IP set processing functions. |
npm/pkg/dataplane/dataplane.go | Removed several log messages in AddPolicy, applyDataPlaneNow, and related dataplane functions. |
npm/pkg/dataplane/ipsets/ipsetmanager_linux.go | Removed verbose logging in reset and destroy operations for IPSets. |
npm/pkg/dataplane/parse/parser.go | Stripped out debugging logs in command execution for iptables parsing. |
npm/pkg/dataplane/ipsets/ipsetmanager.go | Cleared extraneous info logs during IP set reconciliation and deletion. |
npm/pkg/dataplane/dpshim/dirtycache.go | Removed debug log calls in dirty cache clearing and contents printing. |
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you confirm that changing configmap value causes desired effects?
Can you update PR description too? |
/azp run Azure Container Networking PR |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run Azure Container Networking PR |
Azure Pipelines successfully started running 1 pipeline(s). |
This reverts commit 08697eb.
/azp run Azure Container Networking PR |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run NPM Conformance Tests |
/azp run NPM Scale Test |
Azure Pipelines successfully started running 1 pipeline(s). |
1 similar comment
Azure Pipelines successfully started running 1 pipeline(s). |
Reason for Change:
Reduces noisy NPM logs (non-error/warning logs) and adds a loglevel property to the npm configmap which controls if the app insight logs print. This is important as the logs are printed every 5 minutes with the npm heartbeat and can quickly accumulate.
Will implement Zap logging framework in another pr. Commented out non-error/warning logs with a TODO statement that should be marked as debug when Zap is implemented.
NPM logs with loglevel
info
NPM logs with loglevel
debug
Issue Fixed:
Requirements:
Notes:
Resolves customer log storage issues in managed azure-npm.