sentry-java icon indicating copy to clipboard operation
sentry-java copied to clipboard

add property to specify the loggers where the SentryAppender is attached

Open marbon87 opened this issue 3 years ago • 4 comments

:scroll: Description

This pull-request adds the property sentry.loggers, which can be configured with a list of loggers, where the sentry appender is registered and defaults to the ROOT logger.

:bulb: Motivation and Context

The current implementation of the spring-boot-starter-sentry registers the sentry appender only to the root logger. If the logback configuration contains loggers, that are not additive, the log messages from children of the non addtive logger are not sent to sentry.

:green_heart: How did you test it?

Added a new test to SentryLogbackAppenderAutoConfigurationTest

:pencil: Checklist

  • [x] I reviewed the submitted code
  • [x] I added tests to verify the changes
  • [ ] I updated the docs if needed
  • [x] No breaking changes

:crystal_ball: Next steps

marbon87 avatar Jul 14 '22 12:07 marbon87

@marbon87 thank you for opening this PR. Can you provide some more details on how you configure your logback loggers and appenders.

As an alternative to this PR developers can also simply add the SentryAppender to their logback.xml and then add appender-refs to all the loggers.

adinauer avatar Jul 21 '22 12:07 adinauer

@adinauer the loggers are configured like this:

	<root level="ERROR">
		<appender-ref ref="APP_SYSTEM_OUT" />
		<appender-ref ref="SPRINGLOG" />
	</root>
	
	<logger name="de" level="OFF" additivity="false" />

	<logger name="de.foo.bar" level="WARN">
		<appender-ref ref="APP_INFO"/>
		<appender-ref ref="APP_WARNING"/>
		<appender-ref ref="APP_ERROR"/>
		<appender-ref ref="GELF"/>
		<appender-ref ref="SPRINGLOG" />
	</logger>

If i manually define the SentryAppender in logback.xml and use appender-ref for all loggers, i cannot use the comfortable configuration through spring boot by setting properties like sentry.context-tags.

marbon87 avatar Jul 22 '22 08:07 marbon87

@adinauer, maybe you missed this PR somehow. Do we need more information from @marbon87, or can we review this PR?

philipphofmann avatar Aug 29 '22 13:08 philipphofmann

Nothing's missing as far as I know, I just didn't get around to doing the review yet - sorry.

adinauer avatar Aug 29 '22 14:08 adinauer

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.02 :tada:

Comparison is base (3a20d40) 81.14% compared to head (1fd0fcb) 81.17%.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2173      +/-   ##
============================================
+ Coverage     81.14%   81.17%   +0.02%     
- Complexity     4096     4102       +6     
============================================
  Files           333      333              
  Lines         15193    15213      +20     
  Branches       1980     1980              
============================================
+ Hits          12329    12349      +20     
  Misses         2084     2084              
  Partials        780      780              
Impacted Files Coverage Δ
.../spring/boot/jakarta/SentryLogbackInitializer.java 96.87% <100.00%> (+0.72%) :arrow_up:
...o/sentry/spring/boot/jakarta/SentryProperties.java 80.00% <100.00%> (+2.22%) :arrow_up:
...o/sentry/spring/boot/SentryLogbackInitializer.java 96.87% <100.00%> (+0.72%) :arrow_up:
...n/java/io/sentry/spring/boot/SentryProperties.java 77.41% <100.00%> (+3.34%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov-commenter avatar Oct 11 '22 09:10 codecov-commenter

Sorry @adinauer, i overlooked your feedback. Your changes look good to me.

marbon87 avatar Feb 22 '23 07:02 marbon87

Any updates on this?

marbon87 avatar Mar 13 '23 09:03 marbon87