Prepend windowsEventLogCore instead of appending #5297
Comments
|
I have opened a draft PR that shows the change I would like to make. I will be running a manual test first thing Monday. |
|
I finally successfully ran a manual test that confirmed my suspicion and confirmed that the fix in the PR mentioned above works. |
|
Fix is merged, thanks! |
|
This regressed in #9726. |
mx-psi
pushed a commit
that referenced
this issue
Sep 16, 2024
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description In the CollectorSettings LoggingOptions, when the options are `zap.WrapCore`, the final core wrapped will end up running first. For this reason, the windowsEventLogCore must be the first option in the list of user supplied options, to ensure user supplied options are run before the core that writes to the Windows Event Log. Prior art: I fixed this before in #5298. <!-- Issue number if applicable --> #### Link to tracking issue #5297 <!--Describe what testing was performed and which tests were added.--> #### Testing Testing was manual by running the Collector as a service after this change. I tried to add a unit test but could not find an effective way to do that due to all the type indirection done by the `zap` package. I was hoping to find an easy way to tell that the `windowsEventLogCore` is the first core wrapped in the options but I could not find a way to pull that off. Open to ideas if anyone can find a way.
|
With #11051 merged, this can be closed again. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Is your feature request related to a problem? Please describe.
When creating a new Windows service, a
zapcore for Windows Event Log output is created and appended to theCollectorSettings.LoggingOptionsthat were passed in by the user. Unfortunately, this means thewindowsEventLogCoreis the first in line of zap cores to be executed. This leads to the LoggingOptions passed in not being applied to the Windows Event Log output.Describe the solution you'd like
It would be preferable if the
windowsEventLogCorewas instead prepended to theLoggingOptionsin newWithWindowsEventLogCore so that the options the user passed in will wrap the event log core, instead of the event log core wrapping the user's options.The text was updated successfully, but these errors were encountered: