Using a proxy

[ssteiner]

I was just discussing with a customer how we're going to switch logging from local to Blob Storage. And he asked 'can it handle our proxy?'.

So I had a quick look at the official doc on how to get the libs to go through a proxy: https://learn.microsoft.com/en-us/samples/azure-samples/azure-cxp-developer-support/csharp-sample-code-to-perform-the-blob-storage-get-service-properties-via-the-proxy/ - it seems we need to set the Transport on the BlobServiceClient. Looking through the source code, I don't see the transport set anywhere. The ServicebusTarget and EventHubTargets seem to support a proxy for WebSockets, but as for the rest of it. So, time for my next contribution?

[snakefoot]

Pull-requests are always welcome.

[ssteiner]

@snakefoot Alright.. I've prepared something. I still have to test it, but here's how I imagined proxy support could look like: master...ssteiner:NLog.Extensions.AzureStorage:Add-Proxy-Support

I opted for a separate class to hold the proxy settings, since there can be many. I like to cover both using the default system proxy, and a manual proxy address, and default credentials, custom credentials or no credentials.

What do you think about this approach? (and I know, the doc still needs to be updated).

Also, I did look through the other targets this time around. QueueStorage, eventGrid and AzureDataTables could use the exact same approach - they all have an options object that takes a Transport
Things are a bit tricker for ServiceBus and EventHub - they already have proxy settings for WebSockets. However, it seems less flexible. (no ability to say 'use the default web proxy' / 'use no proxy', and no ability to specify which credentials to use if any. so I guess once it's PR time, the question to ask is 'should this be unified so that all targets support the same proxy configuration settings.

[snakefoot]

Think it looks good. Some minor comments:

• Maybe change so UseProxy == "1" and UseProxy == "true" both enables proxy-logic.
• Only create override of BlobClientOptions when proxySettings != null

[ssteiner]

Shouldn't I be using bool for UseProxy anyway? Having looked through a bunch of targets, it seems I don't really need to use Layout for configuration variables. E.g. the built-in FileTarget uses int and long for numerical values, rather than Layout. So it would appear to me that I should be using Layout only for things that support variables.

As for the constructor overrides - I didn't want to make the code unnecessary long. But how about this: Since the constructors of BlobServiceClient themselves create a new BlobClientOptions object when none is provided - how about using the constructor override, but leaving options null unless proxySettings != null ? That way, the code remains compact and even though using the constructor override, we end up having the standard (and current) behavior, unless we're actually using a proxy. That saves us from if/else blocks for every auth method before creating the BlobServiceClient

In fact, I implemented this already in my branch and updated the config. If you think it's fine, I'll go ahead and implement for QueueStorage, EventGrid and AzureDataTables targets as well (as far as I can see, they support the same approach).

[ssteiner]

But - just ran into an issue - the current behavior in main is that the system's default proxy is used. So, we don't really need a flag to turn on the use of a proxy, it's already on by default. Rather, I'm going to add a flag to turn off the use of a proxy. If it is not set, and there's a custom proxy address or authentication options, I have to provide the proxy settings.