serilog / serilog-settings-configuration

A Serilog configuration provider that reads from Microsoft.Extensions.Configuration
Apache License 2.0
444 stars 129 forks source link

IConfigurationReader and ConfigurationReader should be public #399

Open bielu opened 11 months ago

bielu commented 11 months ago

Hi Serilog Team! I noticed both ConfigurationReader and IConfigurationReader have internal access, which means as developer I cannot inherith or decorate it. I have use case where possiblity of decorating would be great, but with current implementation it is impossible.

bartelink commented 11 months ago

In general, Serilog tries to be extremely conservative in what's made public; as you can imagine, it's hard to put the genie back in the bottle given how many things will take a dependency.

As such, it's absolutely critical to provide the maximum possible information as to your use case in order to either a) figure out a different way of doing it b) figure out a good way of exposing something that meets your need

In other words, a case needs to be made beyond "that thing, I want it, please", as you can be sure that anything that's left private/internal is for a reason, and the general idea is that the publicly exposed interface is designed to afford the access needed for the bulk of cases, but trying to avoid:

It's also normally a good idea to post your problem as a stack overflow question first, showing what you're trying to do - what you've tried etc. This can allow others to provide solutions you may not have considered.

Other things that would be useful in making such a case is a minimal implementation showing the hacking/cloning you needed to do as a workaround; often people who have tried to achieve similar things may be able to provide useful suggestions (same rule applies for stack overflow - the more you context you can provide, the better the chance of someone having useful suggestions).

bielu commented 11 months ago

I am looking into way of reusing IConfigurationReader so I can detect if specific sink was setup, if not inject it with presetup values. Easiest way would be to decorate IConfigurationReader and detect what was configured, but as mentioned in opening comment, there is no way as it is internal. The change which I would affect void ApplySinks(LoggerSinkConfiguration loggerSinkConfiguration); so not seeing it any other option than overriding it. I am currently investigating if it is possible, if class has to stay internal it is fine I will just work around it differently 😓

bartelink commented 11 months ago

I think that helps (I personally have never used/examined this package so won't be offering an answer or suggestion)

Hopefully a watcher that does know this package well will be able to work off what you've provided.

In the meantime, posting on stack overflow will definitely get more eyes than you'll get by relying on watchers of this repo...

0xced commented 11 months ago

I think you might want to have a look at https://github.com/serilog/serilog/issues/1929 which is discussing a very similar issue.