JDetmar / NLog.Extensions.AzureStorage

NLog Target for Azure Storage. Uses NLog batch write to optimize writes to Storage.
MIT License
31 stars 19 forks source link

Parameter connectionStringKey not supported on BlobStorageTarget #84

Closed Imar closed 4 years ago

Imar commented 4 years ago

Hi there,

I upgraded to the latest version of the package and now run into issues with the following config:

<target type="AzureBlobStorage" name="..." layout="${longdate} ${callsite} ${level} ${message} ${exception:format=tostring}" connectionStringKey="LogsConnectionString" container="Logs" blobName="...log" />

The code breaks with the following exception:

Parameter connectionStringKey not supported on BlobStorageTarget

I have to use connectionString="..." to get it to work again.

Is that a deliberate choice? Will this be supported again in a future update? We have a number of targets in the config file that all log to Azure, and duplicating the same connection string over and over is a bit of a pain.

Thanks!

Imar

snakefoot commented 4 years ago

The ConnectionStringKey only worked on NetFramework, and the future is NetCore.

You can lookup the ConnectionString from NetFramework-app.config (or web.config) like this:

You can lookup the ConnectionString from NetCore-appsetings.json like this:

Does this solve your issue, or are you storing your ConnectionString somewhere else?

Imar commented 4 years ago

The ConnectionStringKey only worked on NetFramework, and the future is NetCore.

Thanks, that makes sense. It's just that I ended up with broken projects after upgrading ;-)

I am not sure how to use your example to pull in the connection string though. I have Nlog config code in my app's web.config file like this:

<target type="AzureBlobStorage"
        name="Main"
        layout="..."
        connectionString="... connection string here ..."
        container="ApiLogs"
        blobName="Main/${date:universalTime=true:format=yyyy-MM-dd}/${date:universalTime=true:format=HH}.log" />
<target type="AzureBlobStorage"
        name="Performance"
        layout="..."
        connectionString="... connection string here ..."
        container="ApiLogs"
        blobName="Performance/${date:universalTime=true:format=yyyy-MM-dd}.log" />

        ...

and then a bunch more for all the different targets that we have.

Previously I would use connectionStringKey attribute to point to an app setting that held the entire connection string. With that I could set a single settings on an Azure app service and not store the connection string in code or config files. With this recent change, I do have to store the connection string in web.config and not in Azure anymore, unless I miss something and the ${appsetting:item...} syntax is the answer to my problem somehow. Can you elaborate?

In summary, I see two problems with this code for the .NET Framework:

  1. I need to repeat the connection string in my config file for every registered target
  2. I need to store the keys in the config file itself and can't pull them from the app settings on the app service.

I could work around these maybe with custom code looping over the configured targets and/or use Azure Key Vault but both solutions would mean quite a bit of work for something that worked fine out of the box before.

Thanks for looking into this, and for building this project in the first place. We use it a lot and we really like it!

Cheers,,

Imar

snakefoot commented 4 years ago

And why does ${appsetting:item=connectionStrings.MyStorageKey} not work for you ? (Will also load from web.config).

Imar commented 4 years ago

Oh, I see now. I thought layout renderers were for message formatting only; I didn't realize they can also be used to bring in config data for stuff like configured targets.

For anyone else running into this, here's the code I ended up with:

<configuration>
  ...
  <connectionStrings>
    <add name="MyStorageKey" connectionString="DefaultEndpointsProtocol=https;... rest of connection string here" />
  </connectionStrings>
  <nlog ...>
    <targets async="true">
      <target type="AzureBlobStorage"
              name="Main"
              layout="${longdate} ..."
              connectionString="${appsetting:item=connectionStrings.MyStorageKey}"
              container="ApiLogs"
              blobName="Main/${date:universalTime=true:format=yyyy-MM-dd}.log" />
    </targets>
    ...
  </nlog>
</configuration>

Thanks; this was exactly what I was looking for.

Cheers,

Imar