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

Fix double logging to AWS when using AppConfigAWSCredentials #200

Closed
wants to merge 1 commit into from

Conversation

lanierhall
Copy link

@lanierhall lanierhall commented Apr 14, 2022

Issue #115, Issue #199
Description of changes:
Prevent AWSLoggerCore from starting up multiple Monitors through the StartMonitor() method.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@normj
Copy link
Member

normj commented Apr 15, 2022

Thanks for the PR. I appreciate it works for your use case and probably most use cases but there are use cases where it is valid to have multiple AWSLoggerCore in action. For example with this log4net config I have an appender that gets all messages and another appender that gets just error logs from a specific logger.

<?xml version="1.0" encoding="utf-8" ?>
<log4net>

	<appender name="Console" type="log4net.Appender.ConsoleAppender">

		<!-- A1 uses PatternLayout -->
		<layout type="log4net.Layout.PatternLayout">
			<conversionPattern value="%-4timestamp [%thread] %-5level %logger %ndc - %message%newline" />
		</layout>
	</appender>
	<appender name="CloudwatchDebug" type="AWS.Logger.Log4net.AWSAppender,AWS.Logger.Log4net">
		<LogGroup>SystemDeveloperDebug</LogGroup>
		<Region>us-west-2</Region>
		<layout type="log4net.Layout.PatternLayout">
			<conversionPattern value="%property{log4net:HostName} [%thread] %-5level %logger - %message%newline"/>
		</layout>
		<LibraryLogFileName>c:\logs\awslog.txt</LibraryLogFileName>
	</appender>

	<appender name="CloudwatchError" type="AWS.Logger.Log4net.AWSAppender,AWS.Logger.Log4net">
		<LogGroup>SystemDeveloperError</LogGroup>
		<Region>us-west-2</Region>
		<layout type="log4net.Layout.PatternLayout">
			<conversionPattern value="%property{log4net:HostName} [%thread] %-5level %logger - %message%newline"/>
		</layout>
		<LibraryLogFileName>c:\logs\awslog.txt</LibraryLogFileName>
	</appender>

	<root>
		<level value="DEBUG" />
		<appender-ref ref="Console" />
		<appender-ref ref="CloudwatchDebug" />
	</root>

	<logger name="LoggingAppSettingstEST.Program">
		<level value="ERROR" />
		<appender-ref ref="Console" />
		<appender-ref ref="CloudwatchError" />
	</logger>
</log4net>

I'm sure there are also some plugin scenarios where there could be some process that have some plugins using log4net and others using NLog and only the first one would work if we took this PR.

I'm going to close this PR since it would break some use cases and move the discussion about what to do with the original in the corresponding issue.

@normj normj closed this Apr 15, 2022
@GStoynev
Copy link

GStoynev commented May 5, 2022

@normj, is there a different approach you can recommend to work around the double logging while a more appropriate solution is worked out?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants