HTTP Appender

[johncmckim]

We have applications deployed in networks outside the network the Graylog instance is deployed into. We also don't have control over the firewall rules. The only real option seems to be logging via HTTP(s) as these ports are guaranteed to be open.

Are there any plans to add a GelfHTTPAppender? Do you see any issues with this?

[jjchiw]

Hi, There were no plans in doing a GelfHttpAppender, but I think it sound really well, I was thinking about that when graylog announced the HttpInput (I believe it was two years ago) http://docs.graylog.org/en/1.2/pages/sending_data.html#gelf-via-http but didn't have the time and I didn't need it so I let it pass by. But it sound really interesting.

I'll try to make a first version during the day. I'm thinking in using the source code of https://github.com/statianzo/PostLog or inherit from https://logging.apache.org/log4net/release/config-examples.html RemotingAppender

[jjchiw]

Hi. Well I didn't extend RemotingAppender or used https://github.com/statianzo/PostLog just went with the simple WebClient https://github.com/jjchiw/gelf4net/tree/httpappender here is the code https://github.com/jjchiw/gelf4net/blob/httpappender/src/Gelf4net/Appender/GelfHttpAppender.cs

I didn't want to use RestSharp or HttpClient libraries, because didn't want to add more dependencies, amqp and json.net are already embedded in the library...

So, what do you think about the implementation with WebClient? I had a "weird" problem related to this http://haacked.com/archive/2004/05/15/http-web-request-expect-100-continue.aspx/

Anyway. well, i'll wait to release this appender.....

[johncmckim]

@jjchiw how does the WebClient performance compare against the HttpClient? Furthermore, WebClient is not thread safe, if there is only one appender created, this appender is likely to have deadlocks.

I also note that you have wrapped _webClient.UploadStringAsync(_baseUrl, payload); in a try catch but the method is not awaited. As a result, exceptions thrown by that method will not be caught.

If I was implementing this I would use HttpClient it is thread safe for some methods. If you didn't want to add another dependency you could structure you project to have multiple nuget packages with a shared core library. You could then release a gelf4net.Http package that has the HTTP Client dependencies without adding those dependencies to the main nuget package.

[jjchiw]

Hi!

I found this thread in StackOverflow about WebClient and HttpClient it seems WebClient is faster thant HttpClient, the only difference I found relevant of WebClient and HttpClient in this scenario is that gelf4net could be used in WinRT apps but It couldn't be used in < net 4.0

_webClient.UploadStringAsync(_baseUrl, payload); can not be awaited the one that should be awaited is UploadStringTaskAsync the try catch block is needed because UploadStringAsync throws exceptions

About that WebClient is not thread safe, I don't see anything about that in the msdn just found this

Thread Safety

Any public static (Shared in Visual Basic) members of this type are thread safe. Any instance members are not guaranteed to be thread safe. 

That it's the same as System.Net.HttpClient

Thread Safety
Any public static (Shared in Visual Basic) members of this type are thread safe. Any instance members are not guaranteed to be thread safe.

This was a first draft, I'll look in create new nuget packages and separate udp and amqp and http.....just have to shared the gelflayout....

[johncmckim]

Ah ok. So the method is Async, but not actually Async (awaitable). That is weird.

From what I have seen of WebClient looking into the source, UploadStringAsync is definitely not thread safe. You can take a look at the source using dotpeek or this link. WebClient assigns instance variables in UploadStringAsync and doesn't perform any locking. I don't see how this can be thread safe.

The latest version of the docs for HttpClient outline which methods are thread safe.

[jjchiw]

I'll change GelfHttpAppender to instantiate a WebClient every time it's called the appender, to keep the back compatibility.

I'll also create a package named Gelf4Net.Core that will only have the Gelf4NetLayout and will create a package named Gelf4Net.HttpAppender that will have the HttpClient as dependency.....

I think It'll be done during the weekend.

:)

[jjchiw]

Ok, I think it's done I separated the projects and Gelf4Net.HttpAppender works with HttpClient.

Can you check it? All the changes are in this branch https://github.com/jjchiw/gelf4net/tree/httpappender

I'll upload the new packages during the week.

:)

[johncmckim]

@jjchiw Looks heaps better to me. I have a couple of comments.

When you create the headers, if (!string.IsNullOrEmpty(User) || !string.IsNullOrEmpty(Password)), I would use an & not an ||. Using an OR, if either the User or Password is null or empty, a malformed header will be sent to the server. I think it's better to send no header than a malformed header.

The more important question is about Task.Factory.StartNew. Is there a reason you chose to use this instead of Task.Run? As a general guide, Task.Run is the better choice for async code as it uses the default Synchonization context and understands async delegates. See stephen cleary's blog post about this.

[jjchiw]

Yes about the string.IsNullorEmpty I was thinking about an empty password, but didn't see that through nor test it, maybe build the the Basic Auth in steps, in order to allow empty passwords, yes I know is pointless, anyway, yes, I would change it for an &&

About Task.Factory.StartNew no there is no reason besides that I had always did it that way, but after reading the post, now I know more :) thanks, I already did the change....

[johncmckim]

Fantastic. Thank you so much for working on this. Do you have a nuget package for this?

As a side note, if you're not doing continuous builds, I could highly recommend AppVeyor.

[jjchiw]

Hi!

I thought that AppVeyor was only paying, but I see there is an option for OpenSource projects, I will check it later, it seems it has a lot of configurations...

Just uploaded the new packages and merge the branch with master

• gelf4net. Contains all the Appenders, the HttpAppender uses WebClient
• gelf4net.UdpAppender. Only UdpAppenders
• gelf4net.AmqpAppender. Only AmqpAppenders
• gelf4net.HttpAppender. Only HttpAppenders (it uses HttpClient)

:)