Read from IConfiguration without SectionKey

[krafs]

I'm using Serilog.Settings.Configuration to configure Serilog, and I came across an issue. If I have an IConfiguration that consists of the Serilog-section, how can I configure Serilog with it? ConfigurationLoggerConfigurationExtensions.ConfigurationSection only takes an IConfigurationSection, whilst I have an IConfiguration. And even if I did have a section - it's marked as obsolete. ConfigurationLoggerConfigurationExtensions.Configuration takes an IConfiguration, but it requires an IConfiguration that is one step back, so it can use the sectionKey to get into it.

Now, I understand that you can't just add an overload to the abovementioned method, because an overload without the sectionkey already defaults to the key being "Serilog". Or is there perhaps some other way that I've missed? Apart from "just provide an IConfiguration that isn't already zoomed in to the Serilog section" :)

[nblumhardt]

I had a quick look and can't spot a way to achieve this currently. Whipping up your own IConfiguration implementation that wraps the one you're holding could be a quick unblocker, though.

Longer term we might look at accepting a null section name - but, it's a bit of a niche case, so not entirely likely we'd add this.

Any more info about why/how you're hitting this requirement? Thanks!

[krafs]

I've inherited a code base where IConfigurations are distributed already-zoomed-into. Yeah, I've considered making my own IConfiguration-wrapper for stepping back a step into the outer config section. Definitely possible, but would prefer a native solution. It's probably also possible to get ahold of a different IConfiguration in my specific case.

Still, is there a reason I'm not able to give Serilog exactly what it needs? It doesn't feel right to provide it with the entire outer configuration block. And if there are reasons, why is that the default?

[nblumhardt]

@krafs thanks for the reply. Perhaps you're right - it could just be historical accident at this point.

If you're interested in investigating further and putting a PR together, it seems reasonable to take a closer look 👍

[skomis-mm]

@krafs , @nblumhardt the reason why IConfigurationSection parameters were obsoleted is to support IConfiguration sinks arguments, such as Serilog.Sinks.MSSqlServer's (in order to get root ConnectionStrings section etc). See #148 and related PRs/issues.

This is why root configuration is needed, so it is probably "by design" issue.

[nblumhardt]

Thanks for the note @skomis-mm!

I think where this gets confusing is that IConfigurationSection implements IConfiguration anyway, so the API we have here doesn't really enforce the kind of usage that the MSSQL sink requires (one can still pass a nested IConfiguration to ReadFrom.Configuration(), as long as it has a sub-key for Serilog, but it doesn't have to be the root).

Unless I'm misunderstanding the situation, I think the desire to pass both the root configuration and the one to read from as the same argument might be problematic - going back in time, would we consider separating them to allow something like this?

    .ReadFrom.Configuration(configuration.GetSection("MySerilog"), root: configuration)

where the second root parameter would be optional and really only required for MSSQL. 🤔

[skomis-mm]

I think where this gets confusing is that IConfigurationSection implements IConfiguration anyway, so the API we have here doesn't really enforce the kind of usage that the MSSQL sink requires (one can still pass a nested IConfiguration to ReadFrom.Configuration()

Indeed.. 🤔

I beleive the intent was to have overloads of .ReadFrom.Configuration(..) accepting only IConfigurationRoot type. But IConfigurationRoot is not UX friendly because of need to cast to that type in asp.net/generic host scenarios (HostBuilderContext/WebHostBuilderContext expose IConfiguration type). Because of this IConfiguration is used like a root configuration.

.ReadFrom.Configuration(configuration.GetSection("MySerilog"), root: configuration)

Looks good to me. 👍 Probably move it to .ReadFrom.ConfigurationSection(..) overloads? And since this is a new overload, we can accept IConfiguration as root parameter and add runtime check if it is really IConfiguratonRoot.

Thoughts?

[nblumhardt]

Seems like a reasonable set of trade-offs 👍

[krafs]

I feel like the best fit would have been as an overload for .ReadFrom.Configuration, but seeing as the preferred signature would look something like .ReadFrom.Configuration(IConfiguration serilogConfig, IConfiguration root = null) that won't work, as it would conflict with existing overloads.

If we do add it to .ReadFrom.ConfigurationSection, how about using IConfigurationinstead of IConfigurationSection? So a signature like e.g. .ReadFrom.ConfigurationSection(IConfiguration configuration)?

[nblumhardt]

I'm not 100% sure it would actually conflict - if we just add the IConfiguration root = null argument to the existing method, the calls would look like:

    // Section name defaults to "Serilog", argument is assumed to be at the root, everything
    // works as it does today
    .ReadFrom.Configuration(configuration)

    // Passed-in configuration is the Serilog section, root not available
    .ReadFrom.Configuration(configuration, sectionName: null)

    // Passed-in configuration is the Serilog section, root supplied for MSSQL and any other
    // sinks that need it (probably quite uncommon)
    .ReadFrom.Configuration(configuration, sectionName: null, root: ...)

Passing a null sectionName is slightly strange but not unprecedented 🤔

[krafs]

Ah, right. Today, all overloads throw ArgumentNullException if the provided sectionName is null. But changing null to instead indicate that the given IConfiguration is the serilog section would work. As you say - slightly strange, but seems perfectly fine given the circumstances :)

I was going to suggest making sectionName nullable, but seems like the project doesn't have nullable reference types enabled anyway, so I guess that's not an issue.

[skomis-mm]

👍 I'll come up with something based on your suggestions