[GitHub] log4net issue #35: Wip/app settings

[JJoe2]

Github user JJoe2 commented on the issue:

https://github.com/apache/log4net/pull/35
  
I'll rebase this after #36 and #37 have been processed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] log4net issue #25: API to flush appenders that buffer logging data

[JJoe2]

Github user JJoe2 commented on the issue:

https://github.com/apache/log4net/pull/25
  
Replaced by #37 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] log4net pull request #37: Implement flushing of appenders that buffer data

[JJoe2]

GitHub user JJoe2 opened a pull request:

https://github.com/apache/log4net/pull/37

Implement flushing of appenders that buffer data

Replaces #25 


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/JJoe2/log4net wip/Flush

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/log4net/pull/37.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #37


commit f8eb4b49e3bcc38fdc0e8a2bf36c729eb5e70ea4
Author: JoeJoe 
Date:   2016-10-15T20:22:32Z

Implement flushing of appenders that buffer data




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (LOG4NET-511) API to flush appenders

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/LOG4NET-511?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15578731#comment-15578731
 ] 

ASF GitHub Bot commented on LOG4NET-511:


Github user JJoe2 closed the pull request at:

https://github.com/apache/log4net/pull/25


> API to flush appenders
> --
>
> Key: LOG4NET-511
> URL: https://issues.apache.org/jira/browse/LOG4NET-511
> Project: Log4net
>  Issue Type: Wish
>  Components: Appenders, Core
>Affects Versions: 1.2.15
> Environment: NA
>Reporter: Joe
>Priority: Minor
>
> I would like to see an API that flushes any appenders that have buffered 
> data. E.g. a method LogManager.Flush().  An application might call such a 
> method at regular intervals, e.g. on a Timer.
> A naive implementation with the current log4net would iterate through 
> appenders, looking for those that support flushing, and call the appender's 
> flush method, e.g.:
> foreach (IAppender appender in 
>  LogManager.GetRepository().GetAppenders())
> {
> BufferingAppenderSkeleton bas = appender 
> as BufferingAppenderSkeleton;  
> if (bas != null) bas.Flush();
> }
> But (a) I'm not sure this is thread-safe and (b) there are potentially other 
> appenders that may want to be able to flush data (e.g. a TextWriterAppender 
> with ImmediateFlush = false).
> The request consists of:
> - Add an interface, IFlushableAppender or equivalent, with a single method 
> Flush().
> - Implement this interface in all relevant appenders 
> (BufferingAppenderSkeleton, TextWriterAppender, ...)
> - Add a thread-safe static Flush() method to LogManager.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[GitHub] log4net pull request #25: API to flush appenders that buffer logging data

[JJoe2]

Github user JJoe2 closed the pull request at:

https://github.com/apache/log4net/pull/25


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] log4net pull request #36: Use UTC internally to avoid ambiguous timestamps

[JJoe2]

GitHub user JJoe2 opened a pull request:

https://github.com/apache/log4net/pull/36

Use UTC internally to avoid ambiguous timestamps

Resubmitting replacement for #24 

Rebased to latest trunk and fixed backwards compatibility issue of binary 
serialization of LoggingEvent.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/JJoe2/log4net wip/UseUTC

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/log4net/pull/36.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #36


commit 79a68874f04807f162be7749bd8818120f606be2
Author: JoeJoe 
Date:   2016-10-15T19:11:57Z

Use UTC internally to avoid ambiguous timestamps




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Re: [GitHub] log4net issue #25: API to flush appenders that buffer logging data

[Dominik Psenner]

What do you think of adding a timezone field such that people can calculate
the utc timestamp if they care? Nothing breaks while all usecases are taken
care of.

On 15 Oct 2016 7:23 p.m., "Dominik Psenner"  wrote:

I highly recommend to stay compatible and add a utc timestampas a new field
and deprecate the old field for at least one release cyle.

On 15 Oct 2016 7:20 p.m., "JJoe2"  wrote:

Github user JJoe2 commented on the issue:

https://github.com/apache/log4net/pull/25

Thanks.

I agree that I should have done a better job of keeping these changes
separate but when I did the initial work I was very much a git novice.
I’ll bite the bullet and separate these changes into two branches with
a rebase on the latest trunk.

I take your point about the binary serialization format.  For now I’ll
roll back the change in the serialization / deserialization methods.  Users
of RemotingAppender will therefore still be subject to ambiguous timestamps
but it’s no longer a breaking change.

We could also consider implementing serialization as follows (perhaps
enabled by a configuration setting if preserving backwards compatibility is
important):


info.AddValue("TimeStamp", m_data.TimeStampUtc);
  // Serialize
m_data.TimeStampUtc = info.GetDateTime("TimeStamp").ToUniversalTime();
  // Deserialize

Given that DateTime.ToUniversalTime() is a noop if the Kind property is
already set to Utc, this would give the correct result if both sides have
the new version, and the following result (ignoring .NET 1.x which didn’t
have a Kind property) for mixed versions:


1.   Old client and new server

Old client serializes as local time; New server converts this to UTC
based on the server’s timezone.  Result unchanged as long as client and
server timezones are the same.


2.   New client and old server



New client serializes as UTC.  Old server will have a UTC time in the
TimeStamp, and will probably interpret it as local time.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Re: [GitHub] log4net issue #25: API to flush appenders that buffer logging data

[Dominik Psenner]

I highly recommend to stay compatible and add a utc timestampas a new field
and deprecate the old field for at least one release cyle.

On 15 Oct 2016 7:20 p.m., "JJoe2"  wrote:

Github user JJoe2 commented on the issue:

https://github.com/apache/log4net/pull/25

Thanks.

I agree that I should have done a better job of keeping these changes
separate but when I did the initial work I was very much a git novice.
I’ll bite the bullet and separate these changes into two branches with
a rebase on the latest trunk.

I take your point about the binary serialization format.  For now I’ll
roll back the change in the serialization / deserialization methods.  Users
of RemotingAppender will therefore still be subject to ambiguous timestamps
but it’s no longer a breaking change.

We could also consider implementing serialization as follows (perhaps
enabled by a configuration setting if preserving backwards compatibility is
important):


info.AddValue("TimeStamp", m_data.TimeStampUtc);
  // Serialize
m_data.TimeStampUtc = info.GetDateTime("TimeStamp").ToUniversalTime();
  // Deserialize

Given that DateTime.ToUniversalTime() is a noop if the Kind property is
already set to Utc, this would give the correct result if both sides have
the new version, and the following result (ignoring .NET 1.x which didn’t
have a Kind property) for mixed versions:


1.   Old client and new server

Old client serializes as local time; New server converts this to UTC
based on the server’s timezone.  Result unchanged as long as client and
server timezones are the same.


2.   New client and old server



New client serializes as UTC.  Old server will have a UTC time in the
TimeStamp, and will probably interpret it as local time.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] log4net issue #25: API to flush appenders that buffer logging data

[JJoe2]

Github user JJoe2 commented on the issue:

https://github.com/apache/log4net/pull/25
  
Thanks.

I agree that I should have done a better job of keeping these changes 
separate but when I did the initial work I was very much a git novice.
I’ll bite the bullet and separate these changes into two branches with a 
rebase on the latest trunk.

I take your point about the binary serialization format.  For now I’ll 
roll back the change in the serialization / deserialization methods.  Users of 
RemotingAppender will therefore still be subject to ambiguous timestamps but 
it’s no longer a breaking change.

We could also consider implementing serialization as follows (perhaps 
enabled by a configuration setting if preserving backwards compatibility is 
important):


info.AddValue("TimeStamp", m_data.TimeStampUtc);  
// Serialize
m_data.TimeStampUtc = info.GetDateTime("TimeStamp").ToUniversalTime();
// Deserialize

Given that DateTime.ToUniversalTime() is a noop if the Kind property is 
already set to Utc, this would give the correct result if both sides have the 
new version, and the following result (ignoring .NET 1.x which didn’t have a 
Kind property) for mixed versions:


1.   Old client and new server

Old client serializes as local time; New server converts this to UTC based 
on the server’s timezone.  Result unchanged as long as client and server 
timezones are the same.


2.   New client and old server



New client serializes as UTC.  Old server will have a UTC time in the 
TimeStamp, and will probably interpret it as local time.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Resolved] (LOG4NET-512) Thread safety issue in Hierarchy.cs

[Stefan Bodewig (JIRA)]

 [ 
https://issues.apache.org/jira/browse/LOG4NET-512?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Stefan Bodewig resolved LOG4NET-512.

   Resolution: Fixed
Fix Version/s: 2.0.6

should be fixed with an adapted version of your patch as svn revision 1765091

Many thanks!

> Thread safety issue in Hierarchy.cs
> ---
>
> Key: LOG4NET-512
> URL: https://issues.apache.org/jira/browse/LOG4NET-512
> Project: Log4net
>  Issue Type: Bug
>  Components: Core
>Affects Versions: 1.2.15
>Reporter: Joe
>Priority: Minor
> Fix For: 2.0.6
>
>
> From inspecting the source code I believe there is a threading bug in the 
> Hierarchy class.  Both of the methods Exists and GetCurrentLoggers access the 
> internal hashtable m_ht without locking.
> Impact is minor as both these methods are probably rarely called by third 
> party code, and since they do not modify state, the worst that can happen is 
> an unexpected exception.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[GitHub] log4net issue #25: API to flush appenders that buffer logging data

[bodewig]

Github user bodewig commented on the issue:

https://github.com/apache/log4net/pull/25
  
@JJoe2 I'm sorry, but now the patch contains all the changes we've made 
since you started your branch. It would be good if you could rebase your branch.

Many thanks for the `w=1` trick, I wasn't aware of it. I do care about 
line-feeds but I care about identifying what has changed more :-) - in svn the 
linefeeds are correct, BTW.

I'd really prefer to keep the Flush and UTC changes separate. That one 
changes binary serialization of `LoggingEvent`s which means people can't use 
different versions of log4net and the remoting sink, for example. Something 
we'd need to document and warn against, if we deem it acceptable.

Actually I would have preferred a separate PR for 57299a4 as well as it is 
a different JIRA issue. I've applied that separately now (and also locked 
`m_ht` in `Clear`) so it should disappear if you rebase your branch.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] log4net issue #25: API to flush appenders that buffer logging data

[JJoe2]

Github user JJoe2 commented on the issue:

https://github.com/apache/log4net/pull/25
  
From your remarks I gather you aren’t too concerned about line ending 
inconsistencies.

And since I did a bit of research today, and discovered that it’s 
possible to ignore whitespace / line endings when reviewing differences on 
github by appending ?w=1 to the url, it doesn’t seem so important to me.

I've now merged with the latest trunk, rolled back the change to 
ILoggerRepository, and also added a timeout to the flush method, so that it 
better handles asynchronous appenders such as RemoteAppender.

It still includes #24 though: is it important to separate these pull 
requests?



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] log4net issue #25: API to flush appenders that buffer logging data

[bodewig]

Github user bodewig commented on the issue:

https://github.com/apache/log4net/pull/25
  
To be honest I have no idea how svn line ends transfer to the git mirror. 
In svn line ends are CRLF on Windows and LF on Unix. What git makes from this 
depends on your `autocrlf` config and probably on your git client as well (I 
recall cygwin's git insisting on LF).

It's probably best to not touch line ends at all. If that's not possible 
then the best approach may be to create separate commits so that we can at 
least identify the relevant changes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] log4net issue #25: API to flush appenders that buffer logging data

[JJoe2]

Github user JJoe2 commented on the issue:

https://github.com/apache/log4net/pull/25
  
Some recent commits have changed line endings from LF to CRLF: some of the 
ones that affect me are listed below.  Would it be possible for you to fix 
these line endings in the trunk?  This will make it easier for me to redo my 
work and eliminate merge conflicts.

Appender/DebugAppender.cs
core/LoggingEvent.cs
Util/LogManager.cs
Util/LogLog.cs
Util/SystemInfo.cs



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---