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

Common Logging 3 Basic Support #8

Merged
merged 1 commit into from Jan 13, 2015

Conversation

richardlawley
Copy link
Contributor

I can't find much in the way of documentation about the changes to Common.Logging 3, presumably documentation of the new release is forthcoming at some point. From what I can see, the only significant change is the introduction of Global and Thread variables context, which to my understanding are incompatible with Serilog's way of doing things (pushing onto a stack).

Hence this change simply includes a copy of the changes made to AbstractLogger. The contexts will be ignored.

Fixes #7

@Jaben
Copy link
Member

Jaben commented Jan 13, 2015

The issue is that Serilog supports "Global" and "Thread" scoped variables. This is a quick fix -- but it doesn't support that feature.

Jaben added a commit that referenced this pull request Jan 13, 2015
Common Logging 3 Basic Support -- I'll merge this for now and open an issue to iterate on the variables.
@Jaben Jaben merged commit 9851a57 into ChangemakerStudios:master Jan 13, 2015
@richardlawley
Copy link
Contributor Author

Serilog supports these in a different way which is incompatible (in my opinion) with the Common Logging implementation. For example, to add this information in Serilog:

using (LogContext.PushProperty("A", 1))
{
    log.Information("Carries property A = 1");
}

To do this in Common.Logging...

ILog log = LogManager.GetLogger(...);
log.GlobalVariables.Set("A", 1);
log.Information("Carries property A = 1");
log.GlobalVariables.Remove("A");

I don't think it's going to be easy to map these together?

@Jaben
Copy link
Member

Jaben commented Jan 13, 2015

I agree with you -- it's rather unfortunate leaky abstraction by Common.Logging API. Not easy to map. But that's why I didn't want to commit right now... would rather circle back and do it right. That might even include a pull request to Serilog to get the current enrichers, etc.

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

Successfully merging this pull request may close these issues.

Update to Common.Logging 3.0
2 participants