-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Make HttpLoggingPolicy log level configurable #44115
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
base: main
Are you sure you want to change the base?
Conversation
- Add logging_level parameter to HttpLoggingPolicy.__init__ with default logging.INFO - Replace hardcoded logging.INFO checks and logger.info() calls with configurable level - Add tests for custom log level in both sync and async test suites - Maintain backward compatibility with default INFO level
|
Thank you for your contribution @Nikhil172913832! We will review the pull request and get back to you soon. |
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.
Pull Request Overview
This PR makes the HttpLoggingPolicy log level configurable by adding a logging_level parameter to the constructor. This allows users to control the log level (e.g., DEBUG, WARNING) instead of being restricted to the hardcoded INFO level, addressing needs for flexibility in centralized logging systems.
Key Changes:
- Added
logging_levelparameter toHttpLoggingPolicy.__init__with a default value oflogging.INFO - Updated all logging calls throughout the policy to use the configurable level
- Added test coverage in both sync and async test suites to verify custom log level functionality
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| sdk/core/azure-core/azure/core/pipeline/policies/_universal.py | Added logging_level parameter to constructor and replaced all hardcoded logging.INFO references with the configurable level |
| sdk/core/azure-core/tests/test_http_logging_policy.py | Added test to verify HttpLoggingPolicy respects custom DEBUG log level |
| sdk/core/azure-core/tests/async_tests/test_http_logging_policy_async.py | Added async test to verify HttpLoggingPolicy respects custom DEBUG log level |
Co-authored-by: Copilot <[email protected]>
API Change CheckAPIView identified API level changes in this PR and created the following API reviews |
656737e to
7df4d7a
Compare
7df4d7a to
cb385b5
Compare
pvaneck
left a comment
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.
Thanks for the contribution! Overall, the implementation looks good. Since an end user would be passing this kwarg through their SDK clients (e.g. BlobServiceClient, etc.), I wonder if we should make this a bit more specific like http_logging_level.
Would like to get others' opinions on this addition and naming. cc @annatisch @kashifkhan @johanste
Resolves #43955
Changes
logging_levelparameter toHttpLoggingPolicy.__init__with defaultlogging.INFOlogging.INFOchecks andlogger.info()calls with configurable levelThis change allows users to configure the logging level when creating
HttpLoggingPolicy, addressing the need for flexibility in centralized logging systems (e.g., Grafana/Loki) where users may prefer DEBUG or other log levels instead of the hardcoded INFO level.Usage:
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines