serilog-contrib / serilog-sinks-splunk

A Serilog sink that writes to Splunk
https://splunk.com
Apache License 2.0
46 stars 47 forks source link

Update build and test infrastructure; move to standalone JSON formatter #27

Closed nblumhardt closed 8 years ago

nblumhardt commented 8 years ago

Breaks the dependency on Serilog's JsonFormatter virtual extension points for: https://github.com/serilog/serilog/pull/819

Adds a basic test project and updates to the latest build scripts. Tests are a bit light but at least from this point we can start accumulating them. Un-tested with Splunk but based heavily the current production version of the equivalent Seq JSON frmatter, which has been free of any regressions after moving to the same model.

I took the opportunity to scrap several constructor parameters on SplunkJsonFormatter that are unlikely to be used, but were previously accepted by the base class (omitEnclosingObject, closingDelimiter).

I also removed rendering of the Renderings JSON property, which while it was emitted by the early version of the sink, is very unlikely to be used in Splunk and incurs a substantial size overhead per event if format specifiers are used in message templates.

This may be a breaking change for some consumers, if they were constructing SplunkFormatter manually, but I'm not sure how often that API is used.

On any/all of the above points we could revert to the old behavior, just thought I'd put this particular set of options out there. Let me know your thoughts.

Cheers!

merbla commented 8 years ago

Looks good, I am digging a little further as I am getting an issue returned from the HTTP endpoint. All event seem to be present.

I have the #28 change that is required to reproduce.

docker run --hostname splunk -p 8000:8000 -p 8088:8088 -d --env SPLUNK_START_ARGS="--accept-license" outcoldman/splunk:6.4.2
[08:04:26 INF] Running vanilla without extended endpoint loop 14
2016-07-29T22:04:27.0551070Z A status code of BadRequest was received when attempting to send to http://localhost:8088.  The event has been discarded and will not be placed back in the queue.
merbla commented 8 years ago

@nblumhardt got it I think.... will try to get a PR to yours

Without Message Template

{
    "Timestamp": "2016-07-30T09:33:13.8527820+10:00",
    "Level": "Information",
    "RenderedMessage": "Running no template loop 0",
    "Properties": {
        "Counter": 0,
        "Serilog.Sinks.Splunk.Sample": "ViaEventCollector",
        "Serilog.Sinks.Splunk.Sample.TestType": "No Templates"
    }
}

With Message Template (note the extra trailing quotes on message template)

{
    "Timestamp": "2016-07-30T09:33:13.8454590+10:00",
    "Level": "Information",
    "MessageTemplate": "Running host override loop {Counter}"",
    "RenderedMessage": "Running host override loop 14",
    "Properties": {
        "Counter": 14,
        "Serilog.Sinks.Splunk.Sample": "ViaEventCollector",
        "Serilog.Sinks.Splunk.Sample.TestType": "Host Override"
    }
}
nblumhardt commented 8 years ago

Think I got 'em. Thanks!

nblumhardt commented 8 years ago

Hey @merbla, just had a quick shot at merging all JSON formatting into the one class as we discussed today.

I'm not 100% sure it all makes sense - this will change the shape of the payloads sent by TCP and UDP to also include the "event" and "time" properties - but thought I'd get your feedback on it before I make the wrong assumption about what should be done here.

I think I have the invalid JSON and newline issues dealt with but I haven't yet piped anything into an actual Splunk instance :-)

merbla commented 8 years ago

Thanks champ