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

Added async appenders #27

Closed
wants to merge 3 commits into from
Closed

Added async appenders #27

wants to merge 3 commits into from

Conversation

wjdavis5
Copy link

New Appenders

Async implementations of both appenders. The async impl uses a ConcurrentQueue and a dedicated appender thread to perform logging. We found this greatly increased throughput in our production applications by making the calls to log return virtually instantly. The async versions simply extend their sync couterparts, overriding the calls to append.

Updated dependencies

Upgraded to the latest version of all dependencies.

Added Async implementations of both the amqp and the udp appender.
@wjdavis5
Copy link
Author

Hello just wondered if you had any comments on this. Thanks.

@jjchiw
Copy link
Owner

jjchiw commented Nov 19, 2015

Hi, yes I'm planning to merge the request and this(#29) one tonight and remove the Debug.WriteLine (#25)

Run some tests and generate the nuget package.....

Thanks.

@jjchiw
Copy link
Owner

jjchiw commented Nov 20, 2015

I'm testing AsyncGelfUdpAppender and it seems is not logging, I'm using the examples project that are in the repo another thing that I noticed it's that the application it doesn't close. I created a branch (https://github.com/jjchiw/gelf4net/tree/async-appenders) that I will look for it during the day

@dheygere
Copy link

Hello, this is obviously not the same feature as the one in this PR, but I just wanted to let you know that there is another project that lets any appender be Async: https://github.com/cjbhaines/Log4Net.Async

@wjdavis5
Copy link
Author

Hrm. I'll take a look to see whats going on too

@jjchiw
Copy link
Owner

jjchiw commented Nov 23, 2015

@uttg I tested the Async Appender and it works with <param name="IncludeLocationInformation" value="false" /> it seems the message.File = loggingEvent.LocationInformation.FileName; LocalInformation is null...

I'm testing (AsyncForwardingAppender) using SimpleConsoleApplication and it has the same problem that the implementation AsyncGelfUdpAppender it seems it doesn't exit the application.

And I also tested ParallelForwardingAppender and this one it does exit the application.

About the implementation of AsyncGelfUdpAppender and AsyncGelfAmqpAppender I don't know why it's not logging the events. I'll give it a try tomorrow

:)

@jjchiw
Copy link
Owner

jjchiw commented Nov 24, 2015

Ok, I found the error, it seems that if IncludeLocationInformation is set as true the gelfmessage is sent like this

{
  "facility": "RandomPhrases",
  "file": "?",
  "host": "host",
  "level": 7,
  "line": "?",
  "timestamp": "1448360742.04914",
  "version": "1.0",
  "LoggerName": "SimpleConsoleApplication.Program",
  "full_message": "[3] Program - Randomizer Sentence: yay",
  "short_message": "[3] Program - Randomizer Sentence: yay",
  "_app": "RandomSentence",
  "_version": "1.0",
  "_Environment": "Dev",
  "_Level": "DEBUG"
}

The line property with the character ? it's what graylog doesn't like MapperParsingException[failed to parse [line]]; nested: NumberFormatException[For input string: "?"];

Ok, so, what do you think?

  • Add in the README.md that the Async~Appenders doesn't log the file an line and IncludeLocationInformation should be set as false (default)

The issue that when we use Async~Appenders and the Console (SimpleConsoleApplication) doesn't close, will check it later

@wjdavis5
Copy link
Author

Could you send me the example app you are using to test with that isnt closing? We are using this in production today and not having this issue.

Thanks,

@jjchiw
Copy link
Owner

jjchiw commented Nov 30, 2015

I'm using SimpleConsoleApplication that exists in the repository https://github.com/jjchiw/gelf4net/tree/master/examples

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.

3 participants