jjchiw / gelf4net

GELF log4net Appender - graylog2
MIT License
63 stars 59 forks source link

HttpAppender (HttpClient) usage and NuGet package #32

Closed ssoubra closed 8 years ago

ssoubra commented 8 years ago

It appears as though the NuGet package https://www.nuget.org/packages/Gelf4Net.HttpAppender/ has the HttpAppender as an internal class, yet in the source code it's public class.

Also, with this assembly, the namespace and assembly name have changed in subtle ways, how would you recommend configuring this appender?

jjchiw commented 8 years ago

I’ll check later the nuget package.

The way to configure the appender is shown here https://github.com/jjchiw/gelf4net/tree/master/examples/SimpleConsoleApplicationHttpPackage

If you are using the package gelf4net this has a httpappender but it uses webclient instead of httpclient and it should be configured like this https://github.com/jjchiw/gelf4net/tree/master/examples/SimpleConsoleApplicationCorePackage

ssoubra commented 8 years ago

For configuration, there were small changes that were required, essentially because the assembly names have changed, here's what I used:

<appender name="GelfHttpAppender" type="gelf4net.Appender.GelfHttpAppender, Gelf4Net.HttpAppender">

and

<layout type="gelf4net.Layout.GelfLayout, Gelf4Net.Core">

Also found an interesting problem, that messages weren't being received properly until the following line was added into the GelfHttpAppender's ActivateOptions method:

_httpClient.DefaultRequestHeaders.ExpectContinue = false;

Not sure what your thoughts are regarding that.

Finally, if you manage to take a look at the NuGet package, could you please include the Gelf4Net.Core.dll as well

jjchiw commented 8 years ago

Hi!

I'll update the samples project and the documentation to change the log4net configuration.

Here is the thread were it was discussed to separate the Adapters https://github.com/jjchiw/gelf4net/issues/30

I think the Gelf4Net.Core.dll is not needed because it's already embedded in Gelf4Net.HttpAppender.dll

Instad of use Gelf4net.Core you should use Gelf4Net.HttpAppender

<layout type="gelf4net.Layout.GelfLayout, Gelf4Net.HttpAppender">

About the ExpectContinue = false I thought that was only needed when we used WebClient, I'll run some tests tomorrow while I'm updating the documentation and the samples to check about this "flag"

:)

ssoubra commented 8 years ago

Ok thanks a lot for that.

Are you sure about not needing Gelf4Net.Core.dll? I'm not seeing any of those classes in the Gelf4Net.HttpAppender.dll assembly?

jjchiw commented 8 years ago

Yes, I'm sure GelfLayout is in in the assembly Gelf4Net.HttpAppender.dll, I just downloaded the package an opened it with ILSpy

capture

ssoubra commented 8 years ago

Ok, I think I worked out why we're not seeing the same thing.

In the downloaded assembly from the NuGet package GelfHttpAppender is an internal class.

Because of that, and ExpectContinue, I had to build from the source, and the one built from the source files, only contains GelfHttpAppender

gelfhttpappender

jjchiw commented 8 years ago

Sorry it took me to long to check this, the problem was that ILMerge "internalize" all the classes aside of the First Assembly, so when I was merging Gelf4Net.Core with Gelf4Net.Appender.HttpAppender the Gelf4Net.Appender.HttpAppender classes became internal.

And now it should be fixed

ssoubra commented 8 years ago

Ok great. Thanks for that.

Before I try and update, did you manage to consider the GelfHttpAppender's ActivateOptions method as well:

_httpClient.DefaultRequestHeaders.ExpectContinue = false;

We're appear to be needing this line.

jjchiw commented 8 years ago

I added the line in the version 1.0.0.2