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

Added SourceContext to the Serilog Logger #69

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@KevivL

KevivL commented Nov 4, 2014

Without this there is no way to capture the name / context (commonly the type name) provided by a facility while wiring a logger.

AFAIK the .ForContext("SourceContext", name); Has the same behavior as .ForContext() in Serilog. Which is how they capture context and which translates into things like Loggername when you use other sinks in Serilog. I've tested this with a Serilog-NLogSink being injected via the castle facility.

If someone has a better idea for the "SourceContext" constant? Maybe a constructor parameter? It's a cross project dependency :(

Added SourceContext to the Serilog Logger
Without this there is no way to capture the name / context (commonly the type name) provided by a facility while wiring a logger.

AFAIK .ForContext("SourceContext", name); Has the same behavior as .ForContext<T>() in Serilog.

If someone has a better idea for the string constant? Maybe a constructor parameter?
@jonorossi

This comment has been minimized.

Member

jonorossi commented Nov 5, 2014

Thanks for logging this and doing the research. I've looked into it and have written a cleaner implementation locally. I haven't pushed it just yet because I've found another bug in the Serilog integration with SerilogFactory modifying the passed in LoggerConfiguration which this will only make worse.

@jonorossi

This comment has been minimized.

Member

jonorossi commented Nov 5, 2014

Just got back to this. My bad, I thought this was going to make things worse without fixing #70 (which I just logged and fixed), but it would have only made it worse if you were using config.Enrich.WithProperty("k", "v").

The SourceContext isn't special for NLog, Serilog uses it for log4net too and it is used by a few other sinks. The existing ForContext methods that take a System.Type use this constant too:

@jonorossi jonorossi closed this in dacaa6f Nov 5, 2014

@jonorossi

This comment has been minimized.

Member

jonorossi commented Nov 5, 2014

I've added unit tests for my changes, but I would appreciate if you could pull my changes, verify things are working for you and I can then push a new release.

@KevivL

This comment has been minimized.

KevivL commented Nov 5, 2014

Thank you for the quick help and digging out that Serilog constant.
Yes, this change works just fine. I didn't even need to use the .Enrich() on Serilog - the logformat is on the underlying sink in my case, and the loggerName property gets correctly set via this.
If you could push this out, that would be great.

@jonorossi

This comment has been minimized.

Member

jonorossi commented Nov 6, 2014

I've just released v3.3.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment