[System.ClientModel] Add Logging

[m-redding]

Adds logging to System.ClientModel. ClientLoggingPolicy logs basic request and response information and provides opt-in functionality to log request and response content as well. This PR also adds logging to ClientRetryPolicy, to log each retry.

Logs are written to the ILoggerFactory if one is provided. If one is not provided, logs are written to Event Source to provide default logging functionality. Logs are not written to Event Source when an ILoggerFactory is not provided.

[azure-sdk]

API change check

APIView has identified API level changes in this PR and created following API reviews.

System.ClientModel

[m-redding]

TODO - how far to go with validation...

[m-redding]

What about if logging is disabled but a message logging policy implementation is provided?

[jsquire]

We need to allow these to be specified regardless of whether or not logging is enabled. There's a legitimate scenario where logging is controlled by configuration and may be off in local or staging environments but enabled in production.

The expectation is that the same exact code gets promoted locally, then to staging, and ultimately to production with no changes. Validation is performed in each environment and having to make code changes to add a logging implementation only in production where we want logging would be a blocker and policy violation. This is prevalent in enterprise environments and particularly strict in industries such as Pharm and Finance.

[m-redding]

@jsquire thanks for that scenario, that's good to understand better. I wonder if this scenario is a reason to change the precedence rules I have currently set. With the current implementation, ClientPipelineOptions.ClientLoggingOptions.EnableLogging is the highest priority, the ClientPipeline.Create method will not add a logging policy if EnableLogging is false. E.g. this:

ClientLoggingOptions loggingOptions = new() { EnableLogging = false };
ClientPipelineOptions options = new()
{
    MessageLoggingPolicy = MyCustomPolicy(loggingOptions),
    ClientLoggingOptions = loggingOptions
};

will not add MyCustomPolicy to the pipeline. Although for the scenario you mentioned that should be fine.

There is a potential for confusion no matter what with the behavior is, so these exceptions were meant to help alleviate that, but I agree that no exception should be thrown in the scenario you said so I will not add that.

[jsquire]

It would be nice to be able to have config update in real-time, but I don't think that's essential. Mr. Richter, for example, feels strongly that's a bad thing - though it is reasonably common in my experience.

I suppose it depends on how IConfiguration handles an update. If I can update the config file and see that change in the config system, then I think we'll want to always instantiate the logging infrastructure and not write logs if disabled.

Or, react to the config change and update the pipeline with logging infrastructure to react to a config change - but I don't think that's possible right now.

[jsquire]

outside the scope of this PR, but I think it's wild that we depend on immutability but I could extend the type and refuse to call the base class, thus rendering Freeze unable to, you know, freeze things.

[m-redding]

Yeah that's a good point