-
Notifications
You must be signed in to change notification settings - Fork 46
Send LogEvent timestamp as "time" in Http Collector json #15
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
Conversation
Per splunk documentation, the log event time (that will later be parsed as the timestamp of the event by splunk) should be sent in the `time` property of the event metadata envelope and should be formated as epoch <sec>.<ms> Sending the event timestamp in in the pre-defined `time` metadata property is important as it alleviates the event processing demands in splunk. As stated in Splunk Docs: >The HTTP Event Collector endpoint extracts the events from the HTTP >request and parses them before sending them to indexers. Because the >event data format, as described in this topic, is pre-determined, Splunk >Enterprise is able to parse your data quickly, and then sends it to be >indexed. This results in improved data throughput and reduced event >processing time compared to other methods of getting data in. For more information, see Http Collector Protocol documentation (http://dev.splunk.com/view/SP-CAAAE6P)
|
Thanks @pedroreys, I will retro fit to |
|
Awesome @pedroreys! |
|
@pedroreys to clarify, there's no difference in cost if you send "time" or not. We don't perform timestamp extraction when you use HEC's JSON endpoint. The downside of not sending "time" is it won't be recorded in the event and so HEC will record the time IT received the event which may not match when the event was created on the client. So adding "time" is important, but there is no perf impact of not doing so. The statement in the docs is referring to how we differ from Splunk's standard processing pipeline which does do timestamp extraction. |
|
Wow, that was quick :) Thanks for the clarification, @glennblock |
|
No problem. Thank you for the effort! |
| } | ||
| if (!string.IsNullOrWhiteSpace(index)) | ||
| { | ||
| jsonPayLoad = jsonPayLoad + @",""index"":""" + index + @""""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might want to use StringBuilder here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another thing to consider is adding support for batching i.e. sending multiple events in a single request for efficiency. That would require having some sort of Flush() method. See here under Examples for how batching works.
Our logging library handles a lot of this natively (including retries, batching/flush, async), but it does't support DotNet core.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, also it takes a dependency on Newtonsoft.Json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also we have this, could be improved however we do batch if needed. Needs to be revisited with the epoch time allocated to each event.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point on Newtonsoft, my main point for linking to the code was showing how we do it. We basically have an in-memory queue where events are written to which allows us to process things (like batching) very efficiently in an async manner
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are using ConcurrentQueue. I understand it has a some overhead. @glennblock, any chance you guys are going to port to dotnet core?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A chance at some point but there will have to be SIGNIFICANT demand in our
customer base, and well there is not currently ;-)
On Tue, May 24, 2016 at 10:32 PM Matthew Erbs [email protected]
wrote:
In src/Serilog.Sinks.Splunk/Sinks/Splunk/EventCollectorRequest.cs
#15 (comment)
:@@ -32,6 +32,11 @@ internal SplunkEvent(string logEvent, string source, string sourceType, string h
jsonPayLoad = jsonPayLoad + @",""index"":""" + index + @"""";We are using ConcurrentQueue. I understand it has a some overhead.
@glennblock https://github.com/glennblock, any chance you guys are
going to port to dotnet core?—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/serilog/serilog-sinks-splunk/pull/15/files/c965fe65ed3789486169001735cf5b68fa135c6d#r64517345
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will try to fill the void 😀
Epoch change from #15. Added build file.
Per splunk documentation, the log event time (that will later be parsed as
the timestamp of the event by splunk) should be sent in the
timeproperty of the event metadata envelope and should be formated as
epoch .
Sending the event timestamp in in the pre-defined
timemetadata propertyis important as it alleviates the event processing demands in splunk. As stated in Splunk Docs:
For more information, see Http Collector Protocol documentation
(http://dev.splunk.com/view/SP-CAAAE6P)