Logging improvements

[JonathanGiles]

The ClientLogger API in azure-core should be improved to enable new features to be supported. The features we want to offer are:

1. Support for logging redaction: The ClientLogger API, as the central location for all logging activities in all Java client libraries, might benefit from having a pluggable API for string redaction, before the message is output into the console, output files, etc. For example, consumers do not want their secret keys, passwords, etc to be published into log files, where they could more easily end up in the wrong hands. One approach we could take would be to have a pluggable API (either an SPI or just a static method on ClientLogger) to register string redactors. Before any logging message is output, the string will run through all redactors. Ideally we could make this more performant by only having redactors execute if the logging message has come from that service in the first place.

2. Custom logging strategies: We should provide a plugin API that allows for client libraries to augment the default logging framework with additional strategies. For example, if a client library wishes to write all WARNING level logging information into a rotating log file, even in the absence of user configuration or opt-in, this strategy should be enabled by a custom plugin that can read all log messages as they flow through the ClientLogger.

To support these requirements, we might need to explore allowing for hierarchical loggers to be defined, so that the pluggable features for redaction and custom logging strategies do not need to be defined against every ClientLogger instance.

[joshfree]

@JonathanGiles is going to investigate this for Preview 4 / Preview 5

[JonathanGiles]

For logging redaction (as required in the general spec), based on the .Net approach, I propose the following:

Where today our client builders offer a httpLogLevelDetail(HttpLogLevelDetail logLevel) method, we would remove this and replace it with a httpOptions(HttpOptions options) method. This would be a fluent API along the following lines:

public class HttpOptions {
    private HttpLogLevelDetail logLevel;
    private List<String> allowedHeaderNames;
    private List<String> allowedQueryParamNames;

   // set / get logLevel

   // set / get / add whitelisted names 
}

We would then update the HttpLoggingPolicy to inspect these two lists and only output white listed values.

[alzimmermsft]

Thoughts on adding a Function that is able to process the response and return a logging level? Storage had functionality in previous clients that would escalate a logging level based on response time.

[srnagar]

For completeness, we should have similar guidelines for AMQP too. Currently, we don't have any logging options made available to users in Event Hubs client builder. @conniey and I will look into the communication metadata that are useful for logging and come up with a proposal for AMQP. (this should be applicable to Service Bus too).

[rickle-msft]

What's the reason for having allowedHeaderNames instead of redactedHeaderNames? It seems like using a whitelist makes logging by default almost useless as everything will be redacted. We'd have to specify dozens of headers to log (probably 95%) instead of specifying that we don't want to publish just the auth header and the sig query parameter. Additionally, if customers have their own custom headers or query parameters, they'd have to go out of their way to make sure their logs are legible.

[rickle-msft]

Additionally, there was a customer issue that requested what Alan mentioned--being able to log based on slow response times.

[JonathanGiles]

@rickle-msft The whitelist approach is the approach set out in the general track two guidelines here for all languages, for both headers and query params. It was designed this way from a security standpoint - it is not possible to feel comfortable as library developers that we won't accidentally leak sensitive information if we went with a blacklist approach.

[rickle-msft]

The customer issue I mentioned is here. Ideally, we'd be able to meet their feature requests.

[alzimmermsft]

Closing this issue as #7636 and #11477 cover point 2 mentioned in the issue and point 1 has be resolved by the introduction of HttpLogOptions which allows configuring which headers and query parameters of a request/response are logged.