c0shea / Seq.Client.EventLog

Writes Windows Event Log entries to Seq
MIT License
33 stars 13 forks source link

Debuggability enhancments #2

Closed nblumhardt closed 5 years ago

nblumhardt commented 5 years ago

Hi! Apologies for the large, unsolicited pull request.

I'm helping a customer figure out why this application no longer functions for them with Seq 5. We don't have much to go on, so I've made some changes on my fork to allow the app to be run as a console application, and to report any internal errors/exceptions.

When debugging in VS, I start it with the EventLogListeners.json file as a command-line argument:

image

The Environment.UserInteractive check then causes it to run as a console app, like this:

image

This is my first cut and I need to do some more testing before I'd suggest it's ready to review, but I thought I'd send a heads-up in case you have suggestions, would prefer to do things differently, or have other ideas I could implement (I'm not deeply attached to any of these changes).

Also completely happy just to maintain this as a personal fork, if it's not the direction you want to take things :-)

All the best, and thanks for publishing a much-needed project!

c0shea commented 5 years ago

No worries. This is a great idea. Thanks for the help and accolades!

What are your thoughts on also logging to a file when running as a service in case there is a problem?

I originally wrote and tested this on Windows 7. I wonder if something changed in Windows 10 where I can't access the Setup log with System.Diagnostics.EventLog anymore. I found in the .NET Framework source that internally it's trying to enumerate the registry key HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Services\EventLog\System. When I look there on my Windows 10 PC and 2016 Datacenter Server, I don't see Setup. That could be the reason but I'm not certain.

nblumhardt commented 5 years ago

Thanks for the reply; I'm not sure where the differences might creep in between Win 7 and 10, but it sounds possible things will have changed, here. It's been a long time since I spent any time with EventLog! :-)

Thanks for the tweaks and fixes; I'll loop back around to this. If I can find a way to reliably/safely write a log file, I think it'd be a worthwhile change and can work this into the PR 👍

c0shea commented 5 years ago

Sounds great, thanks. I'll keep poking around and see if I can figure out why it can't read the setup log for me, either. Strange.

nblumhardt commented 5 years ago

Pushed some more commits, and re-tested running as a service:

The service now logs to a set of size-restricted files in C:\Windows\SysWOW64\config\systemprofile\AppData\Local\Seq.Client.EventLog - example:

2018-11-25 17:02:35.811 +10:00 [INF] Running as service
2018-11-25 17:02:35.841 +10:00 [INF] Loading listener configuration from C:\Development\nblumhardt\Seq.Client.EventLog\src\Seq.Client.EventLog\bin\Debug\EventLogListeners.json
2018-11-25 17:02:35.863 +10:00 [INF] Starting listener for Application on .
2018-11-25 17:02:35.868 +10:00 [INF] Processing 25211 retroactive entries in Application
2018-11-25 17:02:35.869 +10:00 [INF] Starting listener for Security on .
2018-11-25 17:02:35.873 +10:00 [INF] Processing 26514 retroactive entries in Security
2018-11-25 17:02:35.874 +10:00 [INF] Starting listener for Setup on .
2018-11-25 17:02:35.875 +10:00 [ERR] Failed to start listener for Setup on .
System.InvalidOperationException: The event log 'Setup' on computer '.' does not exist.
   at System.Diagnostics.EventLogInternal.OpenForRead(String currentMachineName)
   at System.Diagnostics.EventLogInternal.get_EntryCount()
   at System.Diagnostics.EventLogInternal.StartListening(String currentMachineName, String currentLogName)
   at System.Diagnostics.EventLogInternal.StartRaisingEvents(String currentMachineName, String currentLogName)
   at System.Diagnostics.EventLogInternal.set_EnableRaisingEvents(Boolean value)
   at System.Diagnostics.EventLog.set_EnableRaisingEvents(Boolean value)
   at Seq.Client.EventLog.EventLogListener.Start() in C:\Development\nblumhardt\Seq.Client.EventLog\src\Seq.Client.EventLog\EventLogListener.cs:line 49
2018-11-25 17:02:35.877 +10:00 [INF] Starting listener for System on .
2018-11-25 17:02:35.875 +10:00 [ERR] Failed to send retroactive entries in Setup on .
System.InvalidOperationException: The event log 'Setup' on computer '.' does not exist.
   at System.Diagnostics.EventLogInternal.OpenForRead(String currentMachineName)
   at System.Diagnostics.EventLogInternal.get_EntryCount()
   at System.Diagnostics.EventLogEntryCollection.get_Count()
   at Seq.Client.EventLog.EventLogListener.SendRetroactiveEntries() in C:\Development\nblumhardt\Seq.Client.EventLog\src\Seq.Client.EventLog\EventLogListener.cs:line 100
2018-11-25 17:02:35.878 +10:00 [INF] Processing 46941 retroactive entries in System

Retroactive entry processing now opens its own copy of the event log - it turns out EventLog is not thread safe (or at least doesn't appear to be). Was hitting some rather strange behavior and deadlocks trying to shut down/exit the process before this change.

Finally, I pulled the HTTP code out into its own class, and made HttpClient singleton as suggested in #1.

All seems good to go! Let me know if there's anything that needs attention.

c0shea commented 5 years ago

Looks great! Thanks for the help 👍