Skip to content

Improve logging#392

Merged
jimmyjames merged 4 commits intomasterfrom
improve-logging
Jan 20, 2022
Merged

Improve logging#392
jimmyjames merged 4 commits intomasterfrom
improve-logging

Conversation

@jimmyjames
Copy link
Copy Markdown
Contributor

@jimmyjames jimmyjames commented Jan 12, 2022

Currently, logging of HTTP request/response information is only enabled through the setLoggingEnabled(boolean enabled) method on the ManagementAPI and AuthAPI API clients. While this does allow for enabling or disabling logging (disabled by default), as discussed in #389 the configurability of the logging can be improved.

This PR adds a new configuration point to our existing HttpOptions config object. It does so by exposing a new method, HttpOptions#setLoggingOptions(LoggingOptions loggingOptions).

The LoggingOptions class exposes the ability to customize the logging level to any of OkHttp's supported levels, as well as the ability to specify headers that should be redacted from the logs (it does not expose any of the OkHttp functionality publicly).

A follow-up PR will be made to deprecate the existing setLoggingEnabled in favor of this new customization point.

@jimmyjames jimmyjames requested a review from a team as a code owner January 12, 2022 13:29
@jimmyjames jimmyjames added this to the v1-Next milestone Jan 12, 2022
logging.redactHeader(header);
}
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, unfortunately this (and all other networking client config) is copy-paste between the Auth and Management API clients :(. Given the current package structure, extracting it to a common ancestor or utility class would require exposing public methods and dependencies (e.g., OkHttp) that we do not wish to do.

poovamraj
poovamraj previously approved these changes Jan 14, 2022
Copy link
Copy Markdown
Contributor

@poovamraj poovamraj left a comment

Choose a reason for hiding this comment

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

👍

@jimmyjames jimmyjames merged commit c189dbc into master Jan 20, 2022
@jimmyjames jimmyjames deleted the improve-logging branch January 20, 2022 17:47
@jimmyjames jimmyjames modified the milestones: v1-Next, 1.37.0 Jan 20, 2022
@jimmyjames jimmyjames mentioned this pull request Jan 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants