From 44d2d921edfdb45647bcfcd70c22696e6556f319 Mon Sep 17 00:00:00 2001 From: dan norwood Date: Thu, 15 Sep 2022 23:16:52 -0700 Subject: [PATCH 1/5] feat(sentry-logback): add encoder for message formatting --- sentry-logback/api/sentry-logback.api | 1 + .../io/sentry/logback/SentryAppender.java | 24 ++++++- .../io/sentry/logback/SentryAppenderTest.kt | 63 ++++++++++++++++++- 3 files changed, 85 insertions(+), 3 deletions(-) diff --git a/sentry-logback/api/sentry-logback.api b/sentry-logback/api/sentry-logback.api index 9b080a6655..5e5ace5cac 100644 --- a/sentry-logback/api/sentry-logback.api +++ b/sentry-logback/api/sentry-logback.api @@ -11,6 +11,7 @@ public class io/sentry/logback/SentryAppender : ch/qos/logback/core/Unsynchroniz protected fun createEvent (Lch/qos/logback/classic/spi/ILoggingEvent;)Lio/sentry/SentryEvent; public fun getMinimumBreadcrumbLevel ()Lch/qos/logback/classic/Level; public fun getMinimumEventLevel ()Lch/qos/logback/classic/Level; + public fun setEncoder (Lch/qos/logback/core/encoder/Encoder;)V public fun setMinimumBreadcrumbLevel (Lch/qos/logback/classic/Level;)V public fun setMinimumEventLevel (Lch/qos/logback/classic/Level;)V public fun setOptions (Lio/sentry/SentryOptions;)V 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 2c8f18e116..dfb7e1fb84 100644 --- a/sentry-logback/src/main/java/io/sentry/logback/SentryAppender.java +++ b/sentry-logback/src/main/java/io/sentry/logback/SentryAppender.java @@ -6,6 +6,7 @@ import ch.qos.logback.classic.Level; import ch.qos.logback.classic.spi.ILoggingEvent; import ch.qos.logback.classic.spi.ThrowableProxy; +import ch.qos.logback.core.encoder.Encoder; import ch.qos.logback.core.UnsynchronizedAppenderBase; import com.jakewharton.nopen.annotation.Open; import io.sentry.Breadcrumb; @@ -20,6 +21,7 @@ import io.sentry.protocol.Message; import io.sentry.protocol.SdkVersion; import io.sentry.util.CollectionUtils; +import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -39,6 +41,7 @@ public class SentryAppender extends UnsynchronizedAppenderBase { private @Nullable ITransportFactory transportFactory; private @NotNull Level minimumBreadcrumbLevel = Level.INFO; private @NotNull Level minimumEventLevel = Level.ERROR; + private @Nullable Encoder encoder; @Override public void start() { @@ -91,7 +94,7 @@ protected void append(@NotNull ILoggingEvent eventObject) { final SentryEvent event = new SentryEvent(DateUtils.getDateTime(loggingEvent.getTimeStamp())); final Message message = new Message(); message.setMessage(loggingEvent.getMessage()); - message.setFormatted(loggingEvent.getFormattedMessage()); + message.setFormatted(formatted(loggingEvent)); message.setParams(toParams(loggingEvent.getArgumentArray())); event.setMessage(message); event.setLogger(loggingEvent.getLoggerName()); @@ -137,6 +140,19 @@ protected void append(@NotNull ILoggingEvent eventObject) { return event; } + private String formatted(@NotNull ILoggingEvent loggingEvent) { + if (encoder != null) { + try { + return new String(encoder.encode(loggingEvent), StandardCharsets.UTF_8); + } catch (final Throwable t) { + // catch exceptions from possibly incorrectly configured encoder + // and fallback to default formatted message + addWarn("Failed to encode logging event", t); + } + } + return loggingEvent.getFormattedMessage(); + } + private @NotNull List toParams(@Nullable Object[] arguments) { if (arguments != null) { return Arrays.stream(arguments) @@ -158,7 +174,7 @@ protected void append(@NotNull ILoggingEvent eventObject) { final Breadcrumb breadcrumb = new Breadcrumb(); breadcrumb.setLevel(formatLevel(loggingEvent.getLevel())); breadcrumb.setCategory(loggingEvent.getLoggerName()); - breadcrumb.setMessage(loggingEvent.getFormattedMessage()); + breadcrumb.setMessage(formatted(loggingEvent)); return breadcrumb; } @@ -222,4 +238,8 @@ public void setMinimumEventLevel(final @Nullable Level minimumEventLevel) { void setTransportFactory(final @Nullable ITransportFactory transportFactory) { this.transportFactory = transportFactory; } + + public void setEncoder(Encoder encoder) { + this.encoder = encoder; + } } 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 11c2afba98..d809f34fdc 100644 --- a/sentry-logback/src/test/kotlin/io/sentry/logback/SentryAppenderTest.kt +++ b/sentry-logback/src/test/kotlin/io/sentry/logback/SentryAppenderTest.kt @@ -2,6 +2,10 @@ package io.sentry.logback import ch.qos.logback.classic.Level import ch.qos.logback.classic.LoggerContext +import ch.qos.logback.classic.encoder.PatternLayoutEncoder +import ch.qos.logback.classic.spi.ILoggingEvent +import ch.qos.logback.core.encoder.Encoder +import ch.qos.logback.core.encoder.EncoderBase import ch.qos.logback.core.status.Status import com.nhaarman.mockitokotlin2.any import com.nhaarman.mockitokotlin2.anyOrNull @@ -31,7 +35,7 @@ import kotlin.test.assertNull import kotlin.test.assertTrue class SentryAppenderTest { - private class Fixture(dsn: String? = "http://key@localhost/proj", minimumBreadcrumbLevel: Level? = null, minimumEventLevel: Level? = null, contextTags: List? = null) { + private class Fixture(dsn: String? = "http://key@localhost/proj", minimumBreadcrumbLevel: Level? = null, minimumEventLevel: Level? = null, contextTags: List? = null, encoder: Encoder? = null) { val logger: Logger = LoggerFactory.getLogger(SentryAppenderTest::class.java) val loggerContext = LoggerFactory.getILoggerFactory() as LoggerContext val transportFactory = mock() @@ -49,10 +53,13 @@ class SentryAppenderTest { appender.setMinimumEventLevel(minimumEventLevel) appender.context = loggerContext appender.setTransportFactory(transportFactory) + encoder?.context = loggerContext + appender.setEncoder(encoder) val rootLogger = loggerContext.getLogger(Logger.ROOT_LOGGER_NAME) rootLogger.level = Level.TRACE rootLogger.addAppender(appender) appender.start() + encoder?.start() loggerContext.start() } } @@ -88,6 +95,7 @@ class SentryAppenderTest { ) } + @Test fun `converts message`() { fixture = Fixture(minimumEventLevel = Level.DEBUG) @@ -106,6 +114,59 @@ class SentryAppenderTest { ) } + @Test + fun `encodes message`() { + var encoder = PatternLayoutEncoder() + encoder.pattern = "encoderadded %msg" + fixture = Fixture(minimumEventLevel = Level.DEBUG, encoder = encoder) + fixture.logger.info("testing encoding"); + + verify(fixture.transport).send( + checkEvent { event -> + assertNotNull(event.message) { message -> + assertEquals("encoderadded testing encoding", message.formatted) + assertEquals("testing encoding", message.message) + } + assertEquals("io.sentry.logback.SentryAppenderTest", event.logger) + }, + anyOrNull() + ) + } + + class ThrowingEncoder: EncoderBase { + constructor(): super() + override fun headerBytes(): ByteArray { + TODO("Not yet implemented") + } + + override fun footerBytes(): ByteArray { + TODO("Not yet implemented") + } + + override fun encode(event: ILoggingEvent?): ByteArray { + TODO("Not yet implemented") + } + } + + @Test + fun `fallsback when encoder throws`() { + var encoder = ThrowingEncoder() + fixture = Fixture(minimumEventLevel = Level.DEBUG, encoder = encoder) + fixture.logger.info("testing when encoder throws"); + + verify(fixture.transport).send( + checkEvent { event -> + assertNotNull(event.message) { message -> + assertEquals("testing when encoder throws", message.formatted) + assertEquals("testing when encoder throws", message.message) + } + assertEquals("io.sentry.logback.SentryAppenderTest", event.logger) + }, + anyOrNull() + ) + } + + @Test fun `event date is in UTC`() { fixture = Fixture(minimumEventLevel = Level.DEBUG) From 63aac53a65325b41371ac7f169c6a603e3eafea9 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Wed, 12 Oct 2022 14:00:42 +0200 Subject: [PATCH 2/5] Format --- .../main/java/io/sentry/logback/SentryAppender.java | 2 +- .../kotlin/io/sentry/logback/SentryAppenderTest.kt | 10 ++++------ 2 files changed, 5 insertions(+), 7 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 dfb7e1fb84..ae476e2036 100644 --- a/sentry-logback/src/main/java/io/sentry/logback/SentryAppender.java +++ b/sentry-logback/src/main/java/io/sentry/logback/SentryAppender.java @@ -6,8 +6,8 @@ import ch.qos.logback.classic.Level; import ch.qos.logback.classic.spi.ILoggingEvent; import ch.qos.logback.classic.spi.ThrowableProxy; -import ch.qos.logback.core.encoder.Encoder; import ch.qos.logback.core.UnsynchronizedAppenderBase; +import ch.qos.logback.core.encoder.Encoder; import com.jakewharton.nopen.annotation.Open; import io.sentry.Breadcrumb; import io.sentry.DateUtils; 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 d809f34fdc..bea1ee4e55 100644 --- a/sentry-logback/src/test/kotlin/io/sentry/logback/SentryAppenderTest.kt +++ b/sentry-logback/src/test/kotlin/io/sentry/logback/SentryAppenderTest.kt @@ -95,7 +95,6 @@ class SentryAppenderTest { ) } - @Test fun `converts message`() { fixture = Fixture(minimumEventLevel = Level.DEBUG) @@ -119,7 +118,7 @@ class SentryAppenderTest { var encoder = PatternLayoutEncoder() encoder.pattern = "encoderadded %msg" fixture = Fixture(minimumEventLevel = Level.DEBUG, encoder = encoder) - fixture.logger.info("testing encoding"); + fixture.logger.info("testing encoding") verify(fixture.transport).send( checkEvent { event -> @@ -133,8 +132,8 @@ class SentryAppenderTest { ) } - class ThrowingEncoder: EncoderBase { - constructor(): super() + class ThrowingEncoder : EncoderBase { + constructor() : super() override fun headerBytes(): ByteArray { TODO("Not yet implemented") } @@ -152,7 +151,7 @@ class SentryAppenderTest { fun `fallsback when encoder throws`() { var encoder = ThrowingEncoder() fixture = Fixture(minimumEventLevel = Level.DEBUG, encoder = encoder) - fixture.logger.info("testing when encoder throws"); + fixture.logger.info("testing when encoder throws") verify(fixture.transport).send( checkEvent { event -> @@ -166,7 +165,6 @@ class SentryAppenderTest { ) } - @Test fun `event date is in UTC`() { fixture = Fixture(minimumEventLevel = Level.DEBUG) From 64f62e44f9710f5e03ca27d37fa797395c611122 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Wed, 12 Oct 2022 14:04:10 +0200 Subject: [PATCH 3/5] Add changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b905a474dd..149f86e41c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Features + +- Add support for using Encoder with logback.SentryAppender ([#2246](https://github.com/getsentry/sentry-java/pull/2246)) + ## 6.5.0 ### Fixes From 7ae503c14fac688ab9a998762d2f12d5a4d84cbe Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Fri, 14 Oct 2022 17:50:11 +0200 Subject: [PATCH 4/5] Update CHANGELOG.md --- CHANGELOG.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 562ec30c5c..f00c2d97b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,17 +2,17 @@ ## Unreleased -### Features - -- Add support for using Encoder with logback.SentryAppender ([#2246](https://github.com/getsentry/sentry-java/pull/2246)) -- Add captureProfile method to hub and client ([#2290](https://github.com/getsentry/sentry-java/pull/2290)) - ### Fixes - Ensure potential callback exceptions are caught #2123 ([#2291](https://github.com/getsentry/sentry-java/pull/2291)) - Remove verbose FrameMetricsAggregator failure logging ([#2293](https://github.com/getsentry/sentry-java/pull/2293)) - Ignore broken regex for tracePropagationTarget ([#2288](https://github.com/getsentry/sentry-java/pull/2288)) +### Features + +- Add support for using Encoder with logback.SentryAppender ([#2246](https://github.com/getsentry/sentry-java/pull/2246)) +- Add captureProfile method to hub and client ([#2290](https://github.com/getsentry/sentry-java/pull/2290)) + ## 6.5.0 ### Fixes From fff5493aaa9da25e747cda4de23c6a659690c64c Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Fri, 14 Oct 2022 18:19:33 +0200 Subject: [PATCH 5/5] Clear statusmanager after tests --- .../src/test/kotlin/io/sentry/logback/SentryAppenderTest.kt | 1 + 1 file changed, 1 insertion(+) 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 bea1ee4e55..339df1e678 100644 --- a/sentry-logback/src/test/kotlin/io/sentry/logback/SentryAppenderTest.kt +++ b/sentry-logback/src/test/kotlin/io/sentry/logback/SentryAppenderTest.kt @@ -68,6 +68,7 @@ class SentryAppenderTest { @AfterTest fun `stop logback`() { + fixture.loggerContext.statusManager.clear() fixture.loggerContext.stop() Sentry.close() }