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.

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

Decide if Collector should output logs to Windows Events system on Windows #3241

Closed
tigrannajaryan opened this issue May 20, 2021 · 6 comments
Labels
discussion-needed Community discussion needed os:windows Windows specific issues

Comments

@tigrannajaryan
Copy link
Member

tigrannajaryan commented May 20, 2021

Apparently, we send our logs to Event Viewer on Windows, which I had no idea about.

Do we consider this to be a best practice? I think vast majority of app I saw on Windows simply log to a text file. Are we sure we are not doing a weird thing here by logging to Event Viewer?

I understand it may be an official best practice recommended by Microsoft, but the tradition usually trumps official and I have yet to see any Windows app not written by Microsoft that puts its own logs in the Windows Event System).

Continuing the discussion that started here #3147 (review)

@james-bebbington
Copy link
Member

My 2c: logging to Event Viewer is pretty common on Windows from what I've seen, especially for background service type apps (e.g. an agent).

I think supporting both logging to Event Viewer or a log file via flag options would be a good compromise. It should be relatively straightforward to implement both. Then it doesn't really matter too much what you make the default (just make sure any changes to the default behaviour are announced as breaking changes ofc).

@bogdandrutu
Copy link
Member

@victlu @reyang @lmolkova can you help us with this? This is a continuation of the #3147

@reyang
Copy link
Member

reyang commented Nov 12, 2021

This is interesting. I think the more common scenario is to have an agent/collector collect logs from Windows Event Log and convert them to other formats. And here we are asking for the opposite.

May I understand more about the use case @james-bebbington? E.g. when would you use Event Log than a file? Do you have another agent that listens on Event Log and trigger some activity based on certain criteria (e.g. if the Collector is not sending heartbeat, restart the process), or EventViewer is the only use case? Do you prefer Event Log due to the better persistency story (e.g. in case of accidental Collector crash, the unflushed logs might be lost if we're using a buffered file)?

The tenancy model is another thing to be considered - e.g. if multiple Collector instances are running on the same machine - some of them are inside the same NT user session, some of them are running in separate user sessions, do they log to the same stream or each session has its own stream or each instance has its own stream?

@NassimBtk
Copy link

NassimBtk commented Jan 13, 2022

Hello,
I have ran the collector as Windows Service and when enabling the debug level through service.telemetry.logs.level, all the logs were absorbed by the Windows Event Log. Identifying a failure or an unexpected behavior of the collector factories (exporters, receivers, extensions and processors) is really hard as we need to import, filter and convert the events as everything is mixed. If we run several instances of the collector on the same host, things become more complicated.

The zap core is currently managed in /service/collector_windows.go, it is the place where we wrap the windows event log core. I think, it would be better to make the collector_windows.go handler a bit more flexible so that the user can choose between:

  • Event Log routing
  • Log File routing
  • Both

Regarding the Log File option, it would be nice to roll the files, I mean how many log files are kept in the output directory and how to handle the removal of the old files, we can imagine several options:

  • The log file could be named with the timestamp and a rolling strategy is executed at the start-up of the collector to keep a a maximum number of files.
  • The log files could be just named as otel.log, when the collector starts, it renames the old otel.log to otel~1.log then creates a new otel.log, same as on the previous solution, we could define a max number of log files which could be 3 or 4 log files, or maybe the number could be provided by the user.
  • Another option consists in zipping the old files when a maximum size is reached then we define a strategy to remove the file based on the last modified time, I don't know if zap provides a mechanism to handle this...

Hope this can help.

@pjanotti
Copy link
Contributor

pjanotti commented Feb 5, 2024

It is standard practice for Windows services or server applications to write to the Windows event log. However, typically that doesn't apply to verbose logs, when the logging is at high volume, e.g.: an HTTP server like IIS, then typically the application/service has some dedicated logging outside Windows Event Log. Having configuration options suggested on previous comments by @james-bebbington here and @NassimBtk here seem desirable - see also #4086.

That said the collector have been shipped defaulting to Windows Event Log when running as a service for quite some time and that default behavior should be preserved barring a really compelling reason to change the default.

@open-telemetry/collector-triagers and @open-telemetry/collector-maintainers I suggest that we close the current issue, mark #5300 as duplicate of #4086 and follow-up on the later one.

@atoulme
Copy link
Contributor

atoulme commented Mar 15, 2024

Closing per Paulo's comment.

@atoulme atoulme closed this as completed Mar 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion-needed Community discussion needed os:windows Windows specific issues
Projects
None yet
Development

No branches or pull requests

8 participants