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

logger: removed NVLOG compile flag #2886

Merged
merged 2 commits into from
Mar 23, 2018

Conversation

cpakulski
Copy link
Contributor

Description: logger's trace and debug statements were compiled out when NVLOG flag was defined. This was done for performance reasons. #2751 introduced run-time logger's level check before invoking logging routines, but still compiled out trace and debug logging statements. This change, as per #2854, uses run-time logger's level checks for all levels, including trace and debug. NVLOG flag is not used in the code any more.

Risk Level: Low.

Testing: Added unit test to test new macro comparing logging levels.

Docs Changes: Yes - all logger levels can be configured in run-time now: trace, debug, info, warn, error and critical.

Release Notes: Yes - trace and debug log levels are used in all releases.

Added convenience macros to check logger's current severity level before entering
iterative logging.
Added Unit test for new macro ENVOY_LOG_CHECK_LEVEL.

Signed-off-by: Christoph Pakulski <paker8848@gmail.com>
Signed-off-by: Christoph Pakulski <paker8848@gmail.com>
Copy link
Member

@zuercher zuercher left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM.

Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

This is a fantastic cleanup. Thank you. If possible can you add a release note here: https://github.com/envoyproxy/data-plane-api/blob/master/docs/root/intro/version_history.rst?

@mattklein123 mattklein123 merged commit 3b8875f into envoyproxy:master Mar 23, 2018
@cpakulski
Copy link
Contributor Author

Release note added.

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.

3 participants