Add NLog SequenceId to ECS documents #392
Conversation
| @@ -539,6 +539,7 @@ private Event GetEvent(LogEventInfo logEventInfo) | |||
| ? long.Parse(eventSeverity) | |||
| : GetSysLogSeverity(logEventInfo.Level), | |||
| Timezone = TimeZoneInfo.Local.StandardName, | |||
| Sequence = logEventInfo.SequenceID | |||
There was a problem hiding this comment.
Notice that NLog LogEventInfo.SequenceID is an integer. Thus if exceeding more than 2,147,483,647 LogEvents, then you will get bad behavior (imagine a service running for several days). See also: NLog/NLog#4159
There was a problem hiding this comment.
yeah I noticed it was int 😢 I assumed NLog had a way of dealing with overflows.
But reading NLog/NLog#4159 SequenceId is actually lazy now and only increments if a layout is requesting it correct?
What happens with folks using SequenceId today, most likely overflowing? Meaning it effectively can't be used.
Whats your guidance @snakefoot keep it in so we can move along when NLog fixes this?
Or keep it out till then?
There was a problem hiding this comment.
I was thinking to obsolete NLog LogEventInfo.SequenceID and instead rely on LayoutRenderer like ${counter}.
I would probably implement EcsLayout SequenceId like ProcessId has been implemented (maybe unassigned by default). By using a Layout, then people can do whatever they want.
There was a problem hiding this comment.
Alternative just have a private long-field, and do Interlocked.Increment in the NLog EcsLayout for every LogEvent (Skip the overhead of NLog Layout)
But I don't know how Elastic Sequence are expected to be filled out when, when having single instance restarting frequently (starts counting from Sequence=0), or having multiple process-instances on the same machine.
There was a problem hiding this comment.
Okay in that case will close this! Thanks @snakefoot
No description provided.