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

File overwritten #11

Closed
sBoff opened this issue Feb 27, 2018 · 10 comments
Closed

File overwritten #11

sBoff opened this issue Feb 27, 2018 · 10 comments
Labels

Comments

@sBoff
Copy link

sBoff commented Feb 27, 2018

When using the logging with the following config, it doesn't produce the expected results as the file gets overwritten multiple times during startup. This is due to the "appendToFile" value being false. Setting the value to "true" obviously resolves the issue however, the file is no longer overwritten on application startup as required.

I assume as each logger is created, it deletes the existing file and creates a new one.

<appender name="FileAppenderXml" type="log4net.Appender.FileAppender"> <file type="log4net.Util.PatternString" value="Log\ClientApp.xml"/> <appendToFile value="false"/> <layout type="log4net.Layout.XmlLayoutSchemaLog4j"> <locationInfo value="true"/> </layout> <param name="Encoding" value="utf-8" /> </appender>

In essence, any logging that is done prior to the last instance of the logger object being created is deleted. The expected behavior is that all the logging is recorded however, on application startup the existing file is delete (once, if present).

@huorswords
Copy link
Owner

Hello @sBoff ,

Thank you very much by your feedback.

This seems an issue coming from log4net library itself, as the appender implementation is not a part of Microsoft.Extensions.Logging.Log4Net.AspNetCore nuget.

I'm sorry, my fault, but I don't know how to help you on that.

Can you raise the issue on the corresponding project instead?

Regards.

@sBoff
Copy link
Author

sBoff commented Feb 27, 2018

Hi @huorswords,

The log4net implementation is fine. It works as expected in other applications. It is the implementation of Log4NetLogger that is causing the problem - specifically this line in the Log4NetLogger constructor:

log4net.Config.XmlConfigurator.Configure(loggerRepository, xmlElement);

I don't believe XmlConfigurator.Configure should not be called for each logger instance that is created. In a typical application, this method is called once at startup. When new loggers are created, this method is not called again. It is this method that causes the log file to be deleted/recreated and hence, wipes out any log entries created before the last instance of Log4NetLogger is created.

I'll put forth a pull request soon.

@huorswords huorswords reopened this Feb 27, 2018
@huorswords
Copy link
Owner

Hi again,

Cool, thank you very much! I will wait for the pull request then.

Regards

@sBoff
Copy link
Author

sBoff commented Feb 27, 2018

@huorswords, I've been trying to do a pull request for the last hour with little success. I can connect to github and authenticate but it always returns the following when I try to push.

ERROR: Permission to huorswords/Microsoft.Extensions.Logging.Log4Net.AspNetCore.git denied to sBoff.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

Are there some permissions you need to grant me, or something obvious you know I would be doing wrong?

Regards.

sBoff pushed a commit to sBoff/Microsoft.Extensions.Logging.Log4Net.AspNetCore that referenced this issue Feb 27, 2018
sBoff added a commit to sBoff/Microsoft.Extensions.Logging.Log4Net.AspNetCore that referenced this issue Feb 27, 2018
huorswords#11 - Bug fix to address mutiple calls to XmlConfigurator.Configure
@sBoff
Copy link
Author

sBoff commented Feb 27, 2018

I ended up having to fork this repository, create the fix, and then do a pull request from my fork to this repo. Is that the normal way? I would have expected to be able to create a branch and do a pull request directly from this repo?

@huorswords
Copy link
Owner

Hello @sBoff ,

Yes, I think that Fork flow is the default flow on Github repos by default. I didn't change permissions from the beginning, and also I do not include yet the Contributing document on the repo.

My fault, sorry.

Thank you very much by your contribution.

@huorswords huorswords removed the wontfix label Mar 1, 2018
@sBoff
Copy link
Author

sBoff commented Mar 1, 2018

Thanks @huorswords, I didn't realize forking was the default flow, all sorted now 👍

huorswords added a commit that referenced this issue Mar 2, 2018
#11 - Bug fix to address mutiple calls to XmlConfigurator.Configure
@huorswords
Copy link
Owner

Merged!

Thank you very much. I will schedule the master release as soon as possible.

Thank you very much again, @sBoff .

@huorswords
Copy link
Owner

Hi @sBoff ,

Just to let you know. Your changes are available on the 2.0.3 version of the nuget package.

Thank you. Regards,

@sBoff
Copy link
Author

sBoff commented Mar 4, 2018

@huorswords, amazing, thanks very much :)

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

No branches or pull requests

2 participants