Skip to content

Commit f6949d1

Browse files
maciejwalkowiakmaciejwalkowiak
authored
Prevent Logback and Log4j2 integrations from re-initializing Sentry when Sentry is already initialized (#994)
1 parent 70e4a31 commit f6949d1

5 files changed

Lines changed: 83 additions & 57 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
# 3.1.1
22

3+
* fix: Prevent Logback and Log4j2 integrations from re-initializing Sentry when Sentry is already initialized
34
* Enhancement: Bind logging related SentryProperties to Slf4j Level instead of Logback to improve Log4j2 compatibility
45
* fix: Make sure HttpServletRequestSentryUserProvider runs by default before custom SentryUserProvider beans
56
* fix: fix setting up Sentry in Spring Webflux annotation by changing the scope of Spring WebMvc related dependencies

‎sentry-log4j2/src/main/java/io/sentry/log4j2/SentryAppender.java‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -95,17 +95,19 @@ public static SentryAppender createAppender(
9595

9696
@Override
9797
public void start() {
98-
try {
99-
Sentry.init(
100-
options -> {
101-
options.setEnableExternalConfiguration(true);
102-
options.setDsn(dsn);
103-
options.setSentryClientName(BuildConfig.SENTRY_LOG4J2_SDK_NAME);
104-
options.setSdkVersion(createSdkVersion(options));
105-
Optional.ofNullable(transport).ifPresent(options::setTransport);
106-
});
107-
} catch (IllegalArgumentException e) {
108-
LOGGER.info("Failed to init Sentry during appender initialization: " + e.getMessage());
98+
if (!Sentry.isEnabled()) {
99+
try {
100+
Sentry.init(
101+
options -> {
102+
options.setEnableExternalConfiguration(true);
103+
options.setDsn(dsn);
104+
options.setSentryClientName(BuildConfig.SENTRY_LOG4J2_SDK_NAME);
105+
options.setSdkVersion(createSdkVersion(options));
106+
Optional.ofNullable(transport).ifPresent(options::setTransport);
107+
});
108+
} catch (IllegalArgumentException e) {
109+
LOGGER.info("Failed to init Sentry during appender initialization: " + e.getMessage());
110+
}
109111
}
110112
super.start();
111113
}

‎sentry-log4j2/src/test/kotlin/io/sentry/log4j2/SentryAppenderTest.kt‎

Lines changed: 38 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package io.sentry.log4j2
33
import com.nhaarman.mockitokotlin2.mock
44
import com.nhaarman.mockitokotlin2.verify
55
import io.sentry.HubAdapter
6+
import io.sentry.Sentry
67
import io.sentry.SentryLevel
78
import io.sentry.test.checkEvent
89
import io.sentry.transport.ITransport
@@ -27,13 +28,12 @@ import org.apache.logging.log4j.spi.ExtendedLogger
2728
import org.awaitility.kotlin.await
2829

2930
class SentryAppenderTest {
30-
private class Fixture {
31-
val transport = mock<ITransport>()
31+
private class Fixture() {
3232
val loggerContext = LogManager.getContext() as LoggerContext
33-
var minimumBreadcrumbLevel: Level? = null
34-
var minimumEventLevel: Level? = null
33+
lateinit var transport: ITransport
3534

36-
fun getSut(): ExtendedLogger {
35+
fun getSut(transport: ITransport = mock(), minimumBreadcrumbLevel: Level? = null, minimumEventLevel: Level? = null): ExtendedLogger {
36+
this.transport = transport
3737
loggerContext.start()
3838
val config: Configuration = loggerContext.configuration
3939
val appender = SentryAppender("sentry", null, "http://key@localhost/proj", minimumBreadcrumbLevel, minimumEventLevel, transport, HubAdapter.getInstance())
@@ -59,17 +59,35 @@ class SentryAppenderTest {
5959
@AfterTest
6060
fun `stop log4j2`() {
6161
fixture.loggerContext.stop()
62+
Sentry.close()
6263
}
6364

6465
@BeforeTest
6566
fun `clear MDC`() {
6667
ThreadContext.clearAll()
6768
}
6869

70+
@Test
71+
fun `does not initialize Sentry if Sentry is already enabled`() {
72+
val transport = mock<ITransport>()
73+
Sentry.init {
74+
it.dsn = "http://key@localhost/proj"
75+
it.environment = "manual-environment"
76+
it.setTransport(transport)
77+
}
78+
val logger = fixture.getSut(transport = transport)
79+
logger.error("testing environment field")
80+
81+
await.untilAsserted {
82+
verify(fixture.transport).send(checkEvent { event ->
83+
assertEquals("manual-environment", event.environment)
84+
})
85+
}
86+
}
87+
6988
@Test
7089
fun `converts message`() {
71-
fixture.minimumEventLevel = Level.DEBUG
72-
val logger = fixture.getSut()
90+
val logger = fixture.getSut(minimumEventLevel = Level.DEBUG)
7391
logger.debug("testing message conversion {}, {}", 1, 2)
7492

7593
await.untilAsserted {
@@ -84,8 +102,7 @@ class SentryAppenderTest {
84102

85103
@Test
86104
fun `event date is in UTC`() {
87-
fixture.minimumEventLevel = Level.DEBUG
88-
val logger = fixture.getSut()
105+
val logger = fixture.getSut(minimumEventLevel = Level.DEBUG)
89106
val utcTime = LocalDateTime.now(ZoneId.of("UTC"))
90107

91108
logger.debug("testing event date")
@@ -104,8 +121,7 @@ class SentryAppenderTest {
104121

105122
@Test
106123
fun `converts trace log level to Sentry level`() {
107-
fixture.minimumEventLevel = Level.TRACE
108-
val logger = fixture.getSut()
124+
val logger = fixture.getSut(minimumEventLevel = Level.TRACE)
109125
logger.trace("testing trace level")
110126

111127
await.untilAsserted {
@@ -117,8 +133,7 @@ class SentryAppenderTest {
117133

118134
@Test
119135
fun `converts debug log level to Sentry level`() {
120-
fixture.minimumEventLevel = Level.DEBUG
121-
val logger = fixture.getSut()
136+
val logger = fixture.getSut(minimumEventLevel = Level.DEBUG)
122137
logger.debug("testing debug level")
123138

124139
await.untilAsserted {
@@ -130,8 +145,7 @@ class SentryAppenderTest {
130145

131146
@Test
132147
fun `converts info log level to Sentry level`() {
133-
fixture.minimumEventLevel = Level.INFO
134-
val logger = fixture.getSut()
148+
val logger = fixture.getSut(minimumEventLevel = Level.INFO)
135149
logger.info("testing info level")
136150

137151
await.untilAsserted {
@@ -143,8 +157,7 @@ class SentryAppenderTest {
143157

144158
@Test
145159
fun `converts warn log level to Sentry level`() {
146-
fixture.minimumEventLevel = Level.WARN
147-
val logger = fixture.getSut()
160+
val logger = fixture.getSut(minimumEventLevel = Level.WARN)
148161
logger.warn("testing warn level")
149162

150163
await.untilAsserted {
@@ -156,8 +169,7 @@ class SentryAppenderTest {
156169

157170
@Test
158171
fun `converts error log level to Sentry level`() {
159-
fixture.minimumEventLevel = Level.ERROR
160-
val logger = fixture.getSut()
172+
val logger = fixture.getSut(minimumEventLevel = Level.ERROR)
161173
logger.error("testing error level")
162174

163175
await.untilAsserted {
@@ -169,8 +181,7 @@ class SentryAppenderTest {
169181

170182
@Test
171183
fun `converts fatal log level to Sentry level`() {
172-
fixture.minimumEventLevel = Level.FATAL
173-
val logger = fixture.getSut()
184+
val logger = fixture.getSut(minimumEventLevel = Level.FATAL)
174185
logger.fatal("testing fatal level")
175186

176187
await.untilAsserted {
@@ -182,8 +193,7 @@ class SentryAppenderTest {
182193

183194
@Test
184195
fun `attaches thread information`() {
185-
fixture.minimumEventLevel = Level.WARN
186-
val logger = fixture.getSut()
196+
val logger = fixture.getSut(minimumEventLevel = Level.WARN)
187197
logger.warn("testing thread information")
188198

189199
await.untilAsserted {
@@ -195,8 +205,7 @@ class SentryAppenderTest {
195205

196206
@Test
197207
fun `sets tags from ThreadContext`() {
198-
fixture.minimumEventLevel = Level.WARN
199-
val logger = fixture.getSut()
208+
val logger = fixture.getSut(minimumEventLevel = Level.WARN)
200209
ThreadContext.put("key", "value")
201210
logger.warn("testing MDC tags")
202211

@@ -209,8 +218,7 @@ class SentryAppenderTest {
209218

210219
@Test
211220
fun `does not create MDC context when no MDC tags are set`() {
212-
fixture.minimumEventLevel = Level.WARN
213-
val logger = fixture.getSut()
221+
val logger = fixture.getSut(minimumEventLevel = Level.WARN)
214222
logger.warn("testing without MDC tags")
215223

216224
await.untilAsserted {
@@ -222,8 +230,7 @@ class SentryAppenderTest {
222230

223231
@Test
224232
fun `sets SDK version`() {
225-
fixture.minimumEventLevel = Level.INFO
226-
val logger = fixture.getSut()
233+
val logger = fixture.getSut(minimumEventLevel = Level.INFO)
227234
logger.info("testing sdk version")
228235

229236
await.untilAsserted {
@@ -241,9 +248,7 @@ class SentryAppenderTest {
241248

242249
@Test
243250
fun `attaches breadcrumbs with level higher than minimumBreadcrumbLevel`() {
244-
fixture.minimumEventLevel = Level.WARN
245-
fixture.minimumBreadcrumbLevel = Level.DEBUG
246-
val logger = fixture.getSut()
251+
val logger = fixture.getSut(minimumEventLevel = Level.WARN, minimumBreadcrumbLevel = Level.DEBUG)
247252
val utcTime = LocalDateTime.now(ZoneId.of("UTC"))
248253

249254
logger.debug("this should be a breadcrumb #1")
@@ -268,9 +273,7 @@ class SentryAppenderTest {
268273

269274
@Test
270275
fun `does not attach breadcrumbs with level lower than minimumBreadcrumbLevel`() {
271-
fixture.minimumEventLevel = Level.WARN
272-
fixture.minimumBreadcrumbLevel = Level.INFO
273-
val logger = fixture.getSut()
276+
val logger = fixture.getSut(minimumEventLevel = Level.WARN, minimumBreadcrumbLevel = Level.INFO)
274277

275278
logger.debug("this should NOT be a breadcrumb")
276279
logger.info("this should be a breadcrumb")

‎sentry-logback/src/main/java/io/sentry/logback/SentryAppender.java‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,16 +35,17 @@ public final class SentryAppender extends UnsynchronizedAppenderBase<ILoggingEve
3535

3636
@Override
3737
public void start() {
38-
options.setEnableExternalConfiguration(true);
39-
options.setSentryClientName(BuildConfig.SENTRY_LOGBACK_SDK_NAME);
40-
options.setSdkVersion(createSdkVersion(options));
41-
Optional.ofNullable(transport).ifPresent(options::setTransport);
42-
try {
43-
Sentry.init(options);
44-
} catch (IllegalArgumentException e) {
45-
addWarn("Failed to init Sentry during appender initialization: " + e.getMessage());
38+
if (!Sentry.isEnabled()) {
39+
options.setEnableExternalConfiguration(true);
40+
options.setSentryClientName(BuildConfig.SENTRY_LOGBACK_SDK_NAME);
41+
options.setSdkVersion(createSdkVersion(options));
42+
Optional.ofNullable(transport).ifPresent(options::setTransport);
43+
try {
44+
Sentry.init(options);
45+
} catch (IllegalArgumentException e) {
46+
addWarn("Failed to init Sentry during appender initialization: " + e.getMessage());
47+
}
4648
}
47-
4849
super.start();
4950
}
5051

‎sentry-logback/src/test/kotlin/io/sentry/logback/SentryAppenderTest.kt‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import com.nhaarman.mockitokotlin2.any
66
import com.nhaarman.mockitokotlin2.mock
77
import com.nhaarman.mockitokotlin2.verify
88
import com.nhaarman.mockitokotlin2.whenever
9+
import io.sentry.Sentry
910
import io.sentry.SentryLevel
1011
import io.sentry.SentryOptions
1112
import io.sentry.test.checkEvent
@@ -27,8 +28,7 @@ import org.slf4j.LoggerFactory
2728
import org.slf4j.MDC
2829

2930
class SentryAppenderTest {
30-
private class Fixture(minimumBreadcrumbLevel: Level? = null, minimumEventLevel: Level? = null) {
31-
val transport = mock<ITransport>()
31+
private class Fixture(minimumBreadcrumbLevel: Level? = null, minimumEventLevel: Level? = null, val transport: ITransport = mock<ITransport>()) {
3232
val logger: Logger = LoggerFactory.getLogger(SentryAppenderTest::class.java)
3333
val loggerContext = LoggerFactory.getILoggerFactory() as LoggerContext
3434

@@ -55,13 +55,32 @@ class SentryAppenderTest {
5555
@AfterTest
5656
fun `stop logback`() {
5757
fixture.loggerContext.stop()
58+
Sentry.close()
5859
}
5960

6061
@BeforeTest
6162
fun `clear MDC`() {
6263
MDC.clear()
6364
}
6465

66+
@Test
67+
fun `does not initialize Sentry if Sentry is already enabled`() {
68+
val transport = mock<ITransport>()
69+
Sentry.init {
70+
it.dsn = "http://key@localhost/proj"
71+
it.environment = "manual-environment"
72+
it.setTransport(transport)
73+
}
74+
fixture = Fixture(transport = transport)
75+
fixture.logger.error("testing environment field")
76+
77+
await.untilAsserted {
78+
verify(fixture.transport).send(checkEvent { event ->
79+
assertEquals("manual-environment", event.environment)
80+
})
81+
}
82+
}
83+
6584
@Test
6685
fun `converts message`() {
6786
fixture = Fixture(minimumEventLevel = Level.DEBUG)

0 commit comments

Comments
 (0)