Skip to content
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.

Sign up for GitHub

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

Add event data parameters to event log xml #41

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

Add event data parameters to event log xml #41

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cdcd3cb
Added properties to eventdata
mcollera May 28, 2021
5078178
Fixed payload size
mcollera May 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions src/Serilog.Sinks.EventLog/Sinks/EventLog/EventLogSink.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ namespace Serilog.Sinks.EventLog
public class EventLogSink : ILogEventSink
{
const string ApplicationLogName = "Application";
const int MaximumPayloadLengthChars = 31839;
const int MaximumPayloadLengthChars = 31639;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The EventLog.WriteEntry documentation says

The message string is longer than 31,839 bytes (32,766 bytes on Windows operating systems before Windows Vista).

Is this incorrect?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After running the UsingSuperLongLogMessageWorks test repeatedly while tweaking the MaximumPayloadLengthChars value I figured the actual max length is 31718 on Windows 10. Can you confirm this observation?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@0xced This PR would be a very nice improvement so I jumped in and ran the UsingSuperLongLogMessageWorks test too, Win 11 22H2.

The projects dev branch the actual max length is 31701 for me. On mcolleras PR it only worked up to 31698.
Not sure if this is helpful or just making it worse. :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤦‍♂️ Nobody seems to know what the actual limit is, see also dotnet/dotnet-api-docs#9999 (comment)

Not sure what's the best course of action.

const int MaximumSourceNameLengthChars = 212;
const int SourceMovedEventId = 3;

Expand Down Expand Up @@ -160,12 +160,25 @@ public void Emit(LogEvent logEvent)
payload = payload.Substring(0, MaximumPayloadLengthChars);
}

var payloadSize = payload.Length;
var eventInstance = new EventInstance(_eventIdProvider.ComputeEventId(logEvent), 0, type);
var parameters = new List<object>()
{
payload,
};
parameters.AddRange(logEvent.Properties.Values.Select(x => x.ToString()));

foreach (var property in logEvent.Properties)
{
if (payloadSize >= MaximumPayloadLengthChars)
{
break;
}

var value = property.Value.ToString();
var valueLength = Math.Min(value.Length, MaximumPayloadLengthChars - payloadSize);
parameters.Add(value.Substring(0, valueLength));
payloadSize += valueLength;
}

_log.WriteEvent(eventInstance, parameters.ToArray());
}
Expand Down