Extend support for static accessor to any type (not only interfaces or abstract classes)

[ghost]

A few questions before you begin:

Is this an issue related to the Serilog core project or one of the sinks or community projects. This issue list is intended for Serilog core issues. If this issue relates to a sink or related project, please log on the related repository. Please use Gitter chat and Stack Overflow for discussions and questons.

Does this issue relate to a new feature or an existing bug?

• Bug
• New Feature

What version of Serilog is affected? Please list the related NuGet package.
N/A. This is a new feature.

Please describe the current behavior?
Given a Serilog setup like the following:

new LoggerConfiguration()
                .ReadFrom.AppSettings()
                .CreateLogger()

And app.config:

<?xml version="1.0" encoding="utf-8"?>
<configuration>
  <appSettings>
    ...
    <add key="serilog:using:Rollbar" value="Serilog.Sinks.RollbarCom" />
    <add key="serilog:write-to:Rollbar.accessToken" value=".... a token ..." />
    <add key="serilog:write-to:Rollbar.environment" value="oh123" />
    <add key="serilog:write-to:Rollbar.restrictedToMinimumLevel" value="4" />
    <add key="serilog:write-to:Rollbar.transform" value="ExtremeValueInc.RollbarUtils::Transform" />
    <add key="serilog:selflog.path" value="" />
  </appSettings>

On startup, Serilog will fail to convert the string "ExtremeValueInc.RollbarUtils::Transform" into type Action<Payload>. The SettingValueConversions class is unable to find a suitable method for converting this value and defaults to Convert.ChangeType(value, toType)

Please describe the expected behavior?
The expected behavior is Serilog should widen the following conditional to allow Delegates and Actions types to be retrieved from properties and fields:
https://github.com/serilog/serilog/blob/dev/src/Serilog/Settings/KeyValuePairs/SettingValueConversions.cs#L61
The code that flows within that if statement would appear to already be ready to handle Delegate and Action types.

To be honest, it would appear to me that the statement condition is not appropriate for many conditions. If we have a value that has been found to map to a method/field within a class in a namespace, we should invoke it as long as the return type matches that of the app.config appsettings value?

This is to say, if TryParseStaticMemberAccessor successfully parses the value, and the return type of the found method matches the toType, we should invoke it.

[tsimbalar]

Hi Brandon,

when implementing the static accessor features we kind of locked it down on purpose to avoid possible side effects, see conversation here : #1064 (comment) .

I think this would offer a workaround for cases where the configuration cannot easily be expressed via the appSettings xml.

@serilog/reviewers-core any opinion on this one ?

Also, there are also discussions about how to support Delegates in some cases here : #1072

[ghost]

Hi Thibaud,

In the meantime, we cloned the extension methods used by the app.config so that we can configure Rollbar as we need, I look forward to seeing where those other conversations land.

[nblumhardt]

Any later thoughts on this? It seems on the surface that it would be a reasonable addition, especially in the context of things like Destructure.ByTransforming(), and has the pleasant advantage of not requiring any kind of crazy lambda syntax nested inside XML :-)

I guess the question is how much effort is required and how robust the results might be.

[tsimbalar]

I think it's fair to relax a bit the initial rule, and allow access to static properties and fields of any type as long as it matches the config method signature.

The general idea being that given a configuration that accepts a parameter named foo of type Foo , and a class with a public static property or field MyCustomFoo in class MyNamespace.MyClass in assembly MyAssembly, the following config should work :

<?xml version="1.0" encoding="utf-8"?>
<configuration>
  <appSettings>
    ...
    <add key="serilog:write-to:Method.foo" value="MyNamespace.MyClass::MyCustomFoo, MyAssembly" />
  </appSettings>

Related to : #1057

If someone feels like issuing a PR, that would be great :)

[nblumhardt]

I'm working through open issues and closing most of those that have no activity in the past 12 months as stale. This helps us track and prioritize the issues that have the most impact.

If you believe that this issue still needs attention, please feel free to comment here. Thanks!