Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IConfigurationReader and ConfigurationReader should be public #399

Open
bielu opened this issue Sep 22, 2023 · 4 comments
Open

IConfigurationReader and ConfigurationReader should be public #399

bielu opened this issue Sep 22, 2023 · 4 comments

Comments

@bielu
Copy link

bielu commented Sep 22, 2023

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
Copy link
Member

bartelink commented Sep 22, 2023

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:

  • making the exposed API more complex than it needs to be
  • painting the implementation into a corner by not being able to change it in the future without breaking users

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
Copy link
Author

bielu commented Sep 22, 2023

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
Copy link
Member

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
Copy link
Member

0xced commented Sep 22, 2023

I think you might want to have a look at serilog/serilog#1929 which is discussing a very similar issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants