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 all commits
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
32 changes: 27 additions & 5 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 @@ -12,13 +12,15 @@
// See the License for the specific language governing permissions and
// limitations under the License.

using System;
using System.Diagnostics;
using System.IO;
using Serilog.Core;
using Serilog.Debugging;
using Serilog.Events;
using Serilog.Formatting;
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Linq;

namespace Serilog.Sinks.EventLog
{
Expand All @@ -29,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 @@ -158,7 +160,27 @@ public void Emit(LogEvent logEvent)
payload = payload.Substring(0, MaximumPayloadLengthChars);
}

_log.WriteEntry(payload, type, _eventIdProvider.ComputeEventId(logEvent));
var payloadSize = payload.Length;
var eventInstance = new EventInstance(_eventIdProvider.ComputeEventId(logEvent), 0, type);
var parameters = new List<object>()
{
payload,
};

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());
}

static EventLogEntryType LevelToEventLogEntryType(LogEventLevel logEventLevel)
Expand Down