From abd1ae490b84382a504625a78c0c01553ef4e4a7 Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Thu, 18 Feb 2021 16:25:24 +0100 Subject: [PATCH 1/5] Simplify configuring Logback integration when DSN is not set. When an environment property that is referenced in logback.xml is not set, Logback sets its value to PROPERTY_NAME_IS_UNDEFINED (not a `null` like it could have been expected). This complicates setting up SentryLogback appender configuration where the integration should be disabled in environments that do not have this property. Before this change, users had to do: ${PROPERTY:- } With this change: ${PROPERTY} --- .../io/sentry/logback/SentryAppender.java | 22 ++++++++++++------- .../io/sentry/logback/SentryAppenderTest.kt | 12 ++++++++-- .../src/main/resources/logback.xml | 3 ++- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/sentry-logback/src/main/java/io/sentry/logback/SentryAppender.java b/sentry-logback/src/main/java/io/sentry/logback/SentryAppender.java index b2cfc53ad3..f4563a56b5 100644 --- a/sentry-logback/src/main/java/io/sentry/logback/SentryAppender.java +++ b/sentry-logback/src/main/java/io/sentry/logback/SentryAppender.java @@ -35,14 +35,20 @@ public final class SentryAppender extends UnsynchronizedAppenderBase() @@ -40,7 +41,7 @@ class SentryAppenderTest { whenever(this.transportFactory.create(any(), any())).thenReturn(transport) val appender = SentryAppender() val options = SentryOptions() - options.dsn = "http://key@localhost/proj" + options.dsn = dsn appender.setOptions(options) appender.setMinimumBreadcrumbLevel(minimumBreadcrumbLevel) appender.setMinimumEventLevel(minimumEventLevel) @@ -305,4 +306,11 @@ class SentryAppenderTest { }, anyOrNull()) } } + + @Test + fun `does not initialize Sentry when DSN is null`() { + fixture = Fixture(dsn = "DSN_IS_UNDEFINED", minimumEventLevel = Level.DEBUG) + + assertTrue(fixture.loggerContext.statusManager.copyOfStatusList.none { it.level == Status.WARN }) + } } diff --git a/sentry-samples/sentry-samples-logback/src/main/resources/logback.xml b/sentry-samples/sentry-samples-logback/src/main/resources/logback.xml index ebfa585671..bd3678a9c7 100644 --- a/sentry-samples/sentry-samples-logback/src/main/resources/logback.xml +++ b/sentry-samples/sentry-samples-logback/src/main/resources/logback.xml @@ -8,8 +8,9 @@ + true - https://502f25099c204a2fbf4cb16edc5975d1@o447951.ingest.sentry.io/5428563 + ${SENTRY_XXX} From 5424526069cd9d7b436e4636e32f4b11de923b62 Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Thu, 18 Feb 2021 17:12:08 +0100 Subject: [PATCH 2/5] Changelog. --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 35c959ac08..ecd6d091bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ * Fix: Make the ANR Atomic flags immutable * Enchancement: Integration interface better compatibility with Kotlin null-safety * Enchancement: Simplify Sentry configuration in Spring integration (#1259) +* Enchancement: Simplify configuring Logback integration when environment variable with the DSN is not set (#1271) + Breaking Changes: * Enchancement: SentryExceptionResolver should not send handled errors by default (#1248). From 4cf822a09807a179dd9926296ff18cf50195a2f6 Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Fri, 19 Feb 2021 10:29:16 +0100 Subject: [PATCH 3/5] Revert accidental change. --- .../sentry-samples-logback/src/main/resources/logback.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry-samples/sentry-samples-logback/src/main/resources/logback.xml b/sentry-samples/sentry-samples-logback/src/main/resources/logback.xml index bd3678a9c7..9c97514312 100644 --- a/sentry-samples/sentry-samples-logback/src/main/resources/logback.xml +++ b/sentry-samples/sentry-samples-logback/src/main/resources/logback.xml @@ -10,7 +10,7 @@ true - ${SENTRY_XXX} + https://502f25099c204a2fbf4cb16edc5975d1@o447951.ingest.sentry.io/5428563 From 91cf243bfc4b3a34e4ea5a6f7cc2a1d09aa44ccb Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Fri, 19 Feb 2021 10:32:27 +0100 Subject: [PATCH 4/5] Better test name --- .../src/test/kotlin/io/sentry/logback/SentryAppenderTest.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sentry-logback/src/test/kotlin/io/sentry/logback/SentryAppenderTest.kt b/sentry-logback/src/test/kotlin/io/sentry/logback/SentryAppenderTest.kt index e35be1db10..bc07dd21d8 100644 --- a/sentry-logback/src/test/kotlin/io/sentry/logback/SentryAppenderTest.kt +++ b/sentry-logback/src/test/kotlin/io/sentry/logback/SentryAppenderTest.kt @@ -308,7 +308,8 @@ class SentryAppenderTest { } @Test - fun `does not initialize Sentry when DSN is null`() { + fun `does not initialize Sentry when environment variable with DSN is not set`() { + // environment variables referenced in the logback.xml that are not set in the OS, have value "ENV_NAME_IS_UNDEFINED" fixture = Fixture(dsn = "DSN_IS_UNDEFINED", minimumEventLevel = Level.DEBUG) assertTrue(fixture.loggerContext.statusManager.copyOfStatusList.none { it.level == Status.WARN }) From 7dd20aa75851a13409ab39d74eecea034c9ae5ba Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Mon, 22 Feb 2021 06:04:12 +0100 Subject: [PATCH 5/5] Update sentry-logback/src/main/java/io/sentry/logback/SentryAppender.java Co-authored-by: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com> --- .../src/main/java/io/sentry/logback/SentryAppender.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry-logback/src/main/java/io/sentry/logback/SentryAppender.java b/sentry-logback/src/main/java/io/sentry/logback/SentryAppender.java index f4563a56b5..2716b7c266 100644 --- a/sentry-logback/src/main/java/io/sentry/logback/SentryAppender.java +++ b/sentry-logback/src/main/java/io/sentry/logback/SentryAppender.java @@ -48,7 +48,7 @@ public void start() { } else { options .getLogger() - .log(SentryLevel.DEBUG, "DSN is null. SentryLogbackAppender is not being initialized"); + .log(SentryLevel.WARNING, "DSN is null. SentryAppender is not being initialized"); } } super.start();