exceptionless / Exceptionless.Net

Exceptionless clients for the .NET platform
https://exceptionless.com
Other
551 stars 142 forks source link

Update NLog ExceptionlessTarget to use Layout for ApiKey and ServerUrl #297

Closed snakefoot closed 1 year ago

snakefoot commented 1 year ago

Change these lines:

https://github.com/exceptionless/Exceptionless.Net/blob/17841c9b07858cb3aa5f5f4d5cf038afff5a7bf2/src/Platforms/Exceptionless.NLog/ExceptionlessTarget.cs#L12-L13

To these lines:

 public Layout ApiKey { get; set; } 
 public Layout ServerUrl { get; set; } 

Then update InitializeTarget()

        protected override void InitializeTarget() {
            base.InitializeTarget();

            var apiKey = RenderLogEvent(ApiKey, LogEventInfo.CreateNullEvent());
            var serverUrl = RenderLogEvent(ServerUrl, LogEventInfo.CreateNullEvent());

            if (!String.IsNullOrEmpty(apiKey) || !String.IsNullOrEmpty(serverUrl))
                _client = new ExceptionlessClient(config => {
                    if (!String.IsNullOrEmpty(apiKey) && apiKey != "API_KEY_HERE")
                        config.ApiKey = apiKey;
                    if (!String.IsNullOrEmpty(serverUrl))
                        config.ServerUrl = serverUrl;
                    config.UseInMemoryStorage();
                });
        }

This will enable NLog Target Configuration where settings can be retrieved from app.config or appsettings.json (making it easier to have environment-specific setups)

niemyjski commented 1 year ago

@snakefoot can you provide an example of configuration for this? Also is there a recommended best practice for targets?

snakefoot commented 1 year ago

Yes it is recommended to use NLog Layout for target configuration properties. As you can also see from the link I posted.

See also: https://github.com/NLog/NLog/wiki/How-to-write-a-custom-target

snakefoot commented 1 year ago

Example:

    <target xsi:type="Exceptionless, Exceptionless.NLog" name="exceptionless" apiKey="${configsetting:Exceptionless.ApiKey}">
      <field name="host" layout="${machinename}" />
      <field name="process" layout="${processname}" />
    </target>

Then in appsettings.json (where override can done with ex. appsettings.Production.json):

{
    "Exceptionless":{
        "ApiKey":"API_KEY_HERE"
    }
}

See also: https://github.com/NLog/NLog/wiki/ConfigSetting-Layout-Renderer

P.S. You should update the ReadMe.txt and remove this field from the example-config:

<field name="windows-identity" layout="${windows-identity:userName=True:domain=False}" />

As ${windows-identity} requires additional nuget-package with NLog v5, and not supported on all platforms (Ex. Linux). Maybe consider using ${environment-user} instead.

P.P.S. You should probably remove this rogue nlog.config from your source-repository, since not related to ExceptionLess

niemyjski commented 1 year ago

I guess it wouldn't hurt to support this but I am on the fence with your api key example. Users shouldn't set the api key on an nlog target if it matches the default instance as two exceptionless client instances will be created (when the default should be reused).

@snakefoot thanks for this information, would you mind submitting a pr for all this, it would help out a lot.

snakefoot commented 1 year ago

Created #299

niemyjski commented 1 year ago

Awesome Thanks!, I've left some comments on #299