From 9a31d1838803791144a2de3389ea268739806e17 Mon Sep 17 00:00:00 2001 From: Petr Heinz Date: Thu, 23 Jul 2026 10:13:21 +0200 Subject: [PATCH 01/10] T-19629 Add failing reproduction test for cyclic log arguments A log argument with a cyclic object graph makes batchToJson() throw, which makes flushLogs() give up on the entire batch. The test asserts that the surrounding log lines still get serialized; it currently fails with JsonMappingException: Infinite recursion (StackOverflowError). Co-Authored-By: Claude Opus 4.8 --- .../LogtailAppenderCyclicArgumentTest.java | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 src/test/java/com/logtail/logback/LogtailAppenderCyclicArgumentTest.java diff --git a/src/test/java/com/logtail/logback/LogtailAppenderCyclicArgumentTest.java b/src/test/java/com/logtail/logback/LogtailAppenderCyclicArgumentTest.java new file mode 100644 index 0000000..1839808 --- /dev/null +++ b/src/test/java/com/logtail/logback/LogtailAppenderCyclicArgumentTest.java @@ -0,0 +1,66 @@ +package com.logtail.logback; + +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.LoggerContext; +import ch.qos.logback.classic.spi.LoggingEvent; +import org.junit.Test; + +import static org.junit.Assert.assertTrue; + +/** + * Reproduction of T-19629 / logtail/logback-logtail#26. + * + * A single log line carrying an argument with a cyclic object graph (e.g. a JDBC Connection, + * which HikariCP logs on every pooled connection creation) makes the Jackson serialization of + * the whole batch throw, so every other log line in that batch is dropped as well. + */ +public class LogtailAppenderCyclicArgumentTest { + + // Stand-in for any cyclic object graph a dependency might log, e.g. PgConnection <-> PgDatabaseMetaData + static class Parent { + Child child; + + public Child getChild() { + return child; + } + } + + static class Child { + Parent parent; + + public Parent getParent() { + return parent; + } + } + + @Test + public void testCyclicArgumentDoesNotDropTheBatch() throws Exception { + Parent parent = new Parent(); + Child child = new Child(); + parent.child = child; + child.parent = parent; // the cycle + + Logger logger = new LoggerContext().getLogger(Logger.ROOT_LOGGER_NAME); + + LogtailAppender appender = new LogtailAppender(); + appender.setAppName("BetterStackTest"); + appender.setSourceToken("dummy-token"); // any non-empty value; no request is made by this test + + appender.append(new LoggingEvent(Logger.FQCN, logger, Level.INFO, "Log line before the cyclic one", + null, new Object[]{})); + // Equivalent to HikariPool's: LOGGER.debug("{} - Added connection {}", poolName, poolEntry.connection); + appender.append(new LoggingEvent(Logger.FQCN, logger, Level.INFO, "Some object graph: {}", + null, new Object[]{parent})); + appender.append(new LoggingEvent(Logger.FQCN, logger, Level.INFO, "Log line after the cyclic one", + null, new Object[]{})); + + // Currently throws JsonMappingException (infinite recursion / nesting depth exceeded), + // which makes flushLogs() give up on the whole batch. + String json = appender.batchToJson(3); + + assertTrue("Log line before the cyclic one must be sent", json.contains("Log line before the cyclic one")); + assertTrue("The cyclic log line itself must be sent", json.contains("Some object graph:")); + assertTrue("Log line after the cyclic one must be sent", json.contains("Log line after the cyclic one")); + } +} From e76f3fd029b22950ce176db1b2f5b73f9cedc2b0 Mon Sep 17 00:00:00 2001 From: Petr Heinz Date: Thu, 23 Jul 2026 18:50:50 +0200 Subject: [PATCH 02/10] T-19629 Keep sending the batch when a log argument can't be serialized batchToJson() now falls back to sanitizing the batch when the ObjectMapper throws: log lines that serialize fine are kept untouched, and only the values Jackson chokes on are replaced with their string representation. A cyclic object graph on one log line - HikariCP logs a pooled JDBC connection on every connection creation - no longer costs the whole batch. The fallback only runs after a failed serialization, so the happy path is unchanged. StackOverflowError is caught alongside Exception because Jackson only wraps it into JsonMappingException for bean serializers. Co-Authored-By: Claude Opus 4.8 --- .../com/logtail/logback/LogtailAppender.java | 76 ++++++++++++++++++- .../LogtailAppenderCyclicArgumentTest.java | 48 ++++++++++-- 2 files changed, 115 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/logtail/logback/LogtailAppender.java b/src/main/java/com/logtail/logback/LogtailAppender.java index 029c4d9..3661bd1 100644 --- a/src/main/java/com/logtail/logback/LogtailAppender.java +++ b/src/main/java/com/logtail/logback/LogtailAppender.java @@ -254,11 +254,79 @@ protected String batchToJson(int flushedSize) throws JsonProcessingException { synchronized (batch) { snapshot = new ArrayList<>(batch.subList(0, flushedSize)); } - return this.dataMapper.writeValueAsString( - snapshot.stream() + List> logLines = snapshot.stream() .map(this::buildPostData) - .collect(Collectors.toList()) - ); + .collect(Collectors.toList()); + + try { + return this.dataMapper.writeValueAsString(logLines); + + } catch (Exception | StackOverflowError e) { + // A single log line with an unserializable argument - e.g. a cyclic object graph such as a + // pooled JDBC connection - must not prevent the rest of the batch from being sent. + logger.warn("Error processing JSON data, replacing unserializable log values : {}", e.getMessage()); + + return this.dataMapper.writeValueAsString(sanitizeLogLines(logLines)); + } + } + + /** + * Replaces values that Jackson cannot serialize with their string representation, so that one bad + * log line doesn't take down the whole batch. Log lines that serialize fine are left untouched. + */ + protected List> sanitizeLogLines(List> logLines) { + List> sanitized = new ArrayList<>(logLines.size()); + + for (Map logLine : logLines) { + sanitized.add(isSerializable(logLine) ? logLine : sanitizeLogLine(logLine)); + } + + return sanitized; + } + + protected Map sanitizeLogLine(Map logLine) { + Map sanitized = new HashMap<>(logLine); + + for (Entry entry : logLine.entrySet()) { + if (!isSerializable(entry.getValue())) { + sanitized.put(entry.getKey(), sanitizeLogValue(entry.getValue())); + } + } + + return sanitized; + } + + protected Object sanitizeLogValue(Object value) { + // Keep the serializable arguments of a log line intact, replace only the offending ones + if (value instanceof Object[]) { + Object[] values = (Object[]) value; + Object[] sanitized = new Object[values.length]; + + for (int i = 0; i < values.length; i++) { + sanitized[i] = isSerializable(values[i]) ? values[i] : toSafeString(values[i]); + } + + return sanitized; + } + + return toSafeString(value); + } + + protected String toSafeString(Object value) { + try { + return String.valueOf(value); + } catch (Exception | StackOverflowError e) { + return value.getClass().getName(); + } + } + + protected boolean isSerializable(Object value) { + try { + this.dataMapper.writeValueAsString(value); + return true; + } catch (Exception | StackOverflowError e) { + return false; + } } protected Map buildPostData(ILoggingEvent event) { diff --git a/src/test/java/com/logtail/logback/LogtailAppenderCyclicArgumentTest.java b/src/test/java/com/logtail/logback/LogtailAppenderCyclicArgumentTest.java index 1839808..cbb75e5 100644 --- a/src/test/java/com/logtail/logback/LogtailAppenderCyclicArgumentTest.java +++ b/src/test/java/com/logtail/logback/LogtailAppenderCyclicArgumentTest.java @@ -6,6 +6,8 @@ import ch.qos.logback.classic.spi.LoggingEvent; import org.junit.Test; +import java.util.Collections; + import static org.junit.Assert.assertTrue; /** @@ -34,6 +36,18 @@ public Parent getParent() { } } + // Stand-in for any argument Jackson chokes on for a reason other than a cycle + static class Unserializable { + public String getValue() { + throw new UnsupportedOperationException("not available"); + } + + @Override + public String toString() { + return "Unserializable value"; + } + } + @Test public void testCyclicArgumentDoesNotDropTheBatch() throws Exception { Parent parent = new Parent(); @@ -52,15 +66,39 @@ public void testCyclicArgumentDoesNotDropTheBatch() throws Exception { // Equivalent to HikariPool's: LOGGER.debug("{} - Added connection {}", poolName, poolEntry.connection); appender.append(new LoggingEvent(Logger.FQCN, logger, Level.INFO, "Some object graph: {}", null, new Object[]{parent})); - appender.append(new LoggingEvent(Logger.FQCN, logger, Level.INFO, "Log line after the cyclic one", - null, new Object[]{})); + appender.append(new LoggingEvent(Logger.FQCN, logger, Level.INFO, "Log line after the cyclic one: {}", + null, new Object[]{Collections.singletonMap("orderId", 42)})); - // Currently throws JsonMappingException (infinite recursion / nesting depth exceeded), - // which makes flushLogs() give up on the whole batch. String json = appender.batchToJson(3); assertTrue("Log line before the cyclic one must be sent", json.contains("Log line before the cyclic one")); assertTrue("The cyclic log line itself must be sent", json.contains("Some object graph:")); - assertTrue("Log line after the cyclic one must be sent", json.contains("Log line after the cyclic one")); + assertTrue("Log line after the cyclic one must be sent", json.contains("Log line after the cyclic one:")); + + assertTrue("The cyclic argument must be sent as its string representation", + json.contains(Parent.class.getName() + "@")); + assertTrue("Serializable arguments of other log lines must stay structured", + json.contains("{\"orderId\":42}")); + } + + @Test + public void testUnserializableArgumentDoesNotDropTheBatch() throws Exception { + Logger logger = new LoggerContext().getLogger(Logger.ROOT_LOGGER_NAME); + + LogtailAppender appender = new LogtailAppender(); + appender.setAppName("BetterStackTest"); + appender.setSourceToken("dummy-token"); + + appender.append(new LoggingEvent(Logger.FQCN, logger, Level.INFO, "Log line with a broken argument: {}", + null, new Object[]{new Unserializable()})); + appender.append(new LoggingEvent(Logger.FQCN, logger, Level.INFO, "Log line after the broken one", + null, new Object[]{})); + + String json = appender.batchToJson(2); + + assertTrue("The broken log line itself must be sent", json.contains("Log line with a broken argument:")); + assertTrue("Log line after the broken one must be sent", json.contains("Log line after the broken one")); + assertTrue("The broken argument must be sent as its string representation", + json.contains("Unserializable value")); } } From cbe51576435091fac389f2ccf77385db22c61256 Mon Sep 17 00:00:00 2001 From: Petr Heinz Date: Mon, 27 Jul 2026 19:46:17 +0200 Subject: [PATCH 03/10] T-19629 Serialize best-effort in a single pass Replaces the sanitize-and-retry fallback: there is now exactly one serialization per batch and one way of representing data. A new BestEffortSerialization Jackson module detects circular references during serialization (identity set of ancestor objects, mirroring logtail-python's frame.py) and replaces the reference back into the graph with "" while the rest of the object stays structured JSON. SLF4J arguments - the only place arbitrary objects enter the payload - are additionally guarded: an argument whose serialization fails for any other reason (typically a getter that throws) is buffered through a TokenBuffer and replaced as a whole with ">". toString() is never called on logged objects and nothing is ever serialized twice. Co-Authored-By: Claude Fable 5 --- .../logback/BestEffortSerialization.java | 172 ++++++++++++++++++ .../com/logtail/logback/LogtailAppender.java | 88 ++------- .../LogtailAppenderCyclicArgumentTest.java | 104 ----------- .../LogtailAppenderSerializationTest.java | 145 +++++++++++++++ 4 files changed, 331 insertions(+), 178 deletions(-) create mode 100644 src/main/java/com/logtail/logback/BestEffortSerialization.java delete mode 100644 src/test/java/com/logtail/logback/LogtailAppenderCyclicArgumentTest.java create mode 100644 src/test/java/com/logtail/logback/LogtailAppenderSerializationTest.java diff --git a/src/main/java/com/logtail/logback/BestEffortSerialization.java b/src/main/java/com/logtail/logback/BestEffortSerialization.java new file mode 100644 index 0000000..5dbdbe0 --- /dev/null +++ b/src/main/java/com/logtail/logback/BestEffortSerialization.java @@ -0,0 +1,172 @@ +package com.logtail.logback; + +import com.fasterxml.jackson.core.JsonGenerator; +import com.fasterxml.jackson.databind.BeanDescription; +import com.fasterxml.jackson.databind.BeanProperty; +import com.fasterxml.jackson.databind.JsonMappingException; +import com.fasterxml.jackson.databind.JsonSerializer; +import com.fasterxml.jackson.databind.SerializationConfig; +import com.fasterxml.jackson.databind.SerializerProvider; +import com.fasterxml.jackson.databind.jsontype.TypeSerializer; +import com.fasterxml.jackson.databind.module.SimpleModule; +import com.fasterxml.jackson.databind.ser.BeanSerializerModifier; +import com.fasterxml.jackson.databind.ser.ContextualSerializer; +import com.fasterxml.jackson.databind.ser.ResolvableSerializer; +import com.fasterxml.jackson.databind.type.ArrayType; +import com.fasterxml.jackson.databind.type.CollectionType; +import com.fasterxml.jackson.databind.type.MapType; +import com.fasterxml.jackson.databind.util.TokenBuffer; + +import java.io.IOException; +import java.util.Collections; +import java.util.IdentityHashMap; +import java.util.Set; + +/** + * Jackson module making serialization of arbitrary logged values best-effort instead of all-or-nothing, + * in a single serialization pass. Values serialize into the same JSON as without this module, except that + * data Jackson cannot handle is omitted and replaced with a marker string: + *
    + *
  • a reference back to an object the serializer is currently inside (a cyclic object graph, e.g. a + * pooled JDBC connection) is replaced with {@code ""} while the rest of the + * object stays intact,
    • + *
  • a value wrapped with {@link #guard(Object)} whose serialization fails for any other reason + * (typically a getter that throws) is replaced as a whole with a marker naming its class.
    • + *
+ */ +public class BestEffortSerialization extends SimpleModule { + + static final String CIRCULAR_REFERENCE_MARKER = ""; + + private static final Object ANCESTORS = new Object(); + + public BestEffortSerialization() { + addSerializer(Guarded.class, new GuardedSerializer()); + setSerializerModifier(new BeanSerializerModifier() { + @Override + public JsonSerializer modifySerializer(SerializationConfig config, BeanDescription beanDesc, JsonSerializer serializer) { + return new CycleGuard(serializer); + } + + @Override + public JsonSerializer modifyArraySerializer(SerializationConfig config, ArrayType valueType, BeanDescription beanDesc, JsonSerializer serializer) { + return new CycleGuard(serializer); + } + + @Override + public JsonSerializer modifyCollectionSerializer(SerializationConfig config, CollectionType valueType, BeanDescription beanDesc, JsonSerializer serializer) { + return new CycleGuard(serializer); + } + + @Override + public JsonSerializer modifyMapSerializer(SerializationConfig config, MapType valueType, BeanDescription beanDesc, JsonSerializer serializer) { + return new CycleGuard(serializer); + } + }); + } + + /** + * Wraps a value of an unknown type so that when its serialization fails, it is replaced with a marker + * string instead of failing the serialization of everything around it. + */ + public static Object guard(Object value) { + if (value == null || value instanceof String || value instanceof Number || value instanceof Boolean) { + return value; + } + return new Guarded(value); + } + + static final class Guarded { + final Object value; + + Guarded(Object value) { + this.value = value; + } + } + + static final class GuardedSerializer extends JsonSerializer { + @Override + public void serialize(Guarded guarded, JsonGenerator gen, SerializerProvider provider) throws IOException { + // Serialized into a buffer first, so a failure halfway through leaves no partial output behind + TokenBuffer buffer = new TokenBuffer(gen.getCodec(), false); + try { + provider.defaultSerializeValue(guarded.value, buffer); + } catch (Exception | StackOverflowError e) { + gen.writeString(""); + return; + } + buffer.serialize(gen); + } + } + + static final class CycleGuard extends JsonSerializer implements ContextualSerializer, ResolvableSerializer { + private final JsonSerializer delegate; + + @SuppressWarnings("unchecked") + CycleGuard(JsonSerializer delegate) { + this.delegate = (JsonSerializer) delegate; + } + + @Override + public void serialize(Object value, JsonGenerator gen, SerializerProvider provider) throws IOException { + if (!enter(provider, value)) { + gen.writeString(CIRCULAR_REFERENCE_MARKER); + return; + } + try { + delegate.serialize(value, gen, provider); + } finally { + leave(provider, value); + } + } + + @Override + public void serializeWithType(Object value, JsonGenerator gen, SerializerProvider provider, TypeSerializer typeSer) throws IOException { + if (!enter(provider, value)) { + gen.writeString(CIRCULAR_REFERENCE_MARKER); + return; + } + try { + delegate.serializeWithType(value, gen, provider, typeSer); + } finally { + leave(provider, value); + } + } + + @Override + public void resolve(SerializerProvider provider) throws JsonMappingException { + if (delegate instanceof ResolvableSerializer) { + ((ResolvableSerializer) delegate).resolve(provider); + } + } + + @Override + public JsonSerializer createContextual(SerializerProvider provider, BeanProperty property) throws JsonMappingException { + if (delegate instanceof ContextualSerializer) { + JsonSerializer contextual = ((ContextualSerializer) delegate).createContextual(provider, property); + if (contextual != delegate) { + return new CycleGuard(contextual); + } + } + return this; + } + } + + private static boolean enter(SerializerProvider provider, Object value) { + Set ancestors = ancestors(provider); + if (ancestors == null) { + ancestors = Collections.newSetFromMap(new IdentityHashMap()); + provider.setAttribute(ANCESTORS, ancestors); + } + return ancestors.add(value); + } + + private static void leave(SerializerProvider provider, Object value) { + ancestors(provider).remove(value); + } + + @SuppressWarnings("unchecked") + private static Set ancestors(SerializerProvider provider) { + return (Set) provider.getAttribute(ANCESTORS); + } +} diff --git a/src/main/java/com/logtail/logback/LogtailAppender.java b/src/main/java/com/logtail/logback/LogtailAppender.java index 3661bd1..d857e83 100644 --- a/src/main/java/com/logtail/logback/LogtailAppender.java +++ b/src/main/java/com/logtail/logback/LogtailAppender.java @@ -77,7 +77,8 @@ public LogtailAppender() { dataMapper = new ObjectMapper() .setSerializationInclusion(JsonInclude.Include.NON_NULL) .setPropertyNamingStrategy(PropertyNamingStrategies.UPPER_CAMEL_CASE) - .disable(SerializationFeature.FAIL_ON_EMPTY_BEANS); + .disable(SerializationFeature.FAIL_ON_EMPTY_BEANS) + .registerModule(new BestEffortSerialization()); scheduledExecutorService = Executors.newSingleThreadScheduledExecutor(threadFactory); scheduledFuture = scheduledExecutorService.scheduleWithFixedDelay(new LogtailSender(), batchInterval, batchInterval, TimeUnit.MILLISECONDS); @@ -254,79 +255,11 @@ protected String batchToJson(int flushedSize) throws JsonProcessingException { synchronized (batch) { snapshot = new ArrayList<>(batch.subList(0, flushedSize)); } - List> logLines = snapshot.stream() + return this.dataMapper.writeValueAsString( + snapshot.stream() .map(this::buildPostData) - .collect(Collectors.toList()); - - try { - return this.dataMapper.writeValueAsString(logLines); - - } catch (Exception | StackOverflowError e) { - // A single log line with an unserializable argument - e.g. a cyclic object graph such as a - // pooled JDBC connection - must not prevent the rest of the batch from being sent. - logger.warn("Error processing JSON data, replacing unserializable log values : {}", e.getMessage()); - - return this.dataMapper.writeValueAsString(sanitizeLogLines(logLines)); - } - } - - /** - * Replaces values that Jackson cannot serialize with their string representation, so that one bad - * log line doesn't take down the whole batch. Log lines that serialize fine are left untouched. - */ - protected List> sanitizeLogLines(List> logLines) { - List> sanitized = new ArrayList<>(logLines.size()); - - for (Map logLine : logLines) { - sanitized.add(isSerializable(logLine) ? logLine : sanitizeLogLine(logLine)); - } - - return sanitized; - } - - protected Map sanitizeLogLine(Map logLine) { - Map sanitized = new HashMap<>(logLine); - - for (Entry entry : logLine.entrySet()) { - if (!isSerializable(entry.getValue())) { - sanitized.put(entry.getKey(), sanitizeLogValue(entry.getValue())); - } - } - - return sanitized; - } - - protected Object sanitizeLogValue(Object value) { - // Keep the serializable arguments of a log line intact, replace only the offending ones - if (value instanceof Object[]) { - Object[] values = (Object[]) value; - Object[] sanitized = new Object[values.length]; - - for (int i = 0; i < values.length; i++) { - sanitized[i] = isSerializable(values[i]) ? values[i] : toSafeString(values[i]); - } - - return sanitized; - } - - return toSafeString(value); - } - - protected String toSafeString(Object value) { - try { - return String.valueOf(value); - } catch (Exception | StackOverflowError e) { - return value.getClass().getName(); - } - } - - protected boolean isSerializable(Object value) { - try { - this.dataMapper.writeValueAsString(value); - return true; - } catch (Exception | StackOverflowError e) { - return false; - } + .collect(Collectors.toList()) + ); } protected Map buildPostData(ILoggingEvent event) { @@ -337,7 +270,7 @@ protected Map buildPostData(ILoggingEvent event) { logLine.put("message", generateLogMessage(event)); logLine.put("meta", generateLogMeta(event)); logLine.put("runtime", generateLogRuntime(event)); - logLine.put("args", event.getArgumentArray()); + logLine.put("args", guardArguments(event.getArgumentArray())); if (event.getThrowableProxy() != null) { logLine.put("throwable", generateLogThrowable(event.getThrowableProxy())); } @@ -345,6 +278,13 @@ protected Map buildPostData(ILoggingEvent event) { return logLine; } + protected Object[] guardArguments(Object[] arguments) { + if (arguments == null) { + return null; + } + return Arrays.stream(arguments).map(BestEffortSerialization::guard).toArray(); + } + protected String generateLogMessage(ILoggingEvent event) { return this.encoder != null ? new String(this.encoder.encode(event)) : event.getFormattedMessage(); } diff --git a/src/test/java/com/logtail/logback/LogtailAppenderCyclicArgumentTest.java b/src/test/java/com/logtail/logback/LogtailAppenderCyclicArgumentTest.java deleted file mode 100644 index cbb75e5..0000000 --- a/src/test/java/com/logtail/logback/LogtailAppenderCyclicArgumentTest.java +++ /dev/null @@ -1,104 +0,0 @@ -package com.logtail.logback; - -import ch.qos.logback.classic.Level; -import ch.qos.logback.classic.Logger; -import ch.qos.logback.classic.LoggerContext; -import ch.qos.logback.classic.spi.LoggingEvent; -import org.junit.Test; - -import java.util.Collections; - -import static org.junit.Assert.assertTrue; - -/** - * Reproduction of T-19629 / logtail/logback-logtail#26. - * - * A single log line carrying an argument with a cyclic object graph (e.g. a JDBC Connection, - * which HikariCP logs on every pooled connection creation) makes the Jackson serialization of - * the whole batch throw, so every other log line in that batch is dropped as well. - */ -public class LogtailAppenderCyclicArgumentTest { - - // Stand-in for any cyclic object graph a dependency might log, e.g. PgConnection <-> PgDatabaseMetaData - static class Parent { - Child child; - - public Child getChild() { - return child; - } - } - - static class Child { - Parent parent; - - public Parent getParent() { - return parent; - } - } - - // Stand-in for any argument Jackson chokes on for a reason other than a cycle - static class Unserializable { - public String getValue() { - throw new UnsupportedOperationException("not available"); - } - - @Override - public String toString() { - return "Unserializable value"; - } - } - - @Test - public void testCyclicArgumentDoesNotDropTheBatch() throws Exception { - Parent parent = new Parent(); - Child child = new Child(); - parent.child = child; - child.parent = parent; // the cycle - - Logger logger = new LoggerContext().getLogger(Logger.ROOT_LOGGER_NAME); - - LogtailAppender appender = new LogtailAppender(); - appender.setAppName("BetterStackTest"); - appender.setSourceToken("dummy-token"); // any non-empty value; no request is made by this test - - appender.append(new LoggingEvent(Logger.FQCN, logger, Level.INFO, "Log line before the cyclic one", - null, new Object[]{})); - // Equivalent to HikariPool's: LOGGER.debug("{} - Added connection {}", poolName, poolEntry.connection); - appender.append(new LoggingEvent(Logger.FQCN, logger, Level.INFO, "Some object graph: {}", - null, new Object[]{parent})); - appender.append(new LoggingEvent(Logger.FQCN, logger, Level.INFO, "Log line after the cyclic one: {}", - null, new Object[]{Collections.singletonMap("orderId", 42)})); - - String json = appender.batchToJson(3); - - assertTrue("Log line before the cyclic one must be sent", json.contains("Log line before the cyclic one")); - assertTrue("The cyclic log line itself must be sent", json.contains("Some object graph:")); - assertTrue("Log line after the cyclic one must be sent", json.contains("Log line after the cyclic one:")); - - assertTrue("The cyclic argument must be sent as its string representation", - json.contains(Parent.class.getName() + "@")); - assertTrue("Serializable arguments of other log lines must stay structured", - json.contains("{\"orderId\":42}")); - } - - @Test - public void testUnserializableArgumentDoesNotDropTheBatch() throws Exception { - Logger logger = new LoggerContext().getLogger(Logger.ROOT_LOGGER_NAME); - - LogtailAppender appender = new LogtailAppender(); - appender.setAppName("BetterStackTest"); - appender.setSourceToken("dummy-token"); - - appender.append(new LoggingEvent(Logger.FQCN, logger, Level.INFO, "Log line with a broken argument: {}", - null, new Object[]{new Unserializable()})); - appender.append(new LoggingEvent(Logger.FQCN, logger, Level.INFO, "Log line after the broken one", - null, new Object[]{})); - - String json = appender.batchToJson(2); - - assertTrue("The broken log line itself must be sent", json.contains("Log line with a broken argument:")); - assertTrue("Log line after the broken one must be sent", json.contains("Log line after the broken one")); - assertTrue("The broken argument must be sent as its string representation", - json.contains("Unserializable value")); - } -} diff --git a/src/test/java/com/logtail/logback/LogtailAppenderSerializationTest.java b/src/test/java/com/logtail/logback/LogtailAppenderSerializationTest.java new file mode 100644 index 0000000..58cbf0a --- /dev/null +++ b/src/test/java/com/logtail/logback/LogtailAppenderSerializationTest.java @@ -0,0 +1,145 @@ +package com.logtail.logback; + +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.LoggerContext; +import ch.qos.logback.classic.spi.LoggingEvent; +import org.junit.Before; +import org.junit.Test; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +/** + * A log argument that Jackson cannot serialize - a cyclic object graph (e.g. a JDBC Connection, which + * HikariCP logs on every pooled connection creation), or a value whose getter throws - must never + * prevent the batch from being sent. The unserializable data itself is omitted and replaced with a + * marker string; everything around it stays structured JSON. + */ +public class LogtailAppenderSerializationTest { + + // Stand-in for any cyclic object graph a dependency might log, e.g. PgConnection <-> PgDatabaseMetaData + static class Parent { + Child child; + + public Child getChild() { + return child; + } + } + + static class Child { + Parent parent; + + public Parent getParent() { + return parent; + } + } + + // Stand-in for any argument Jackson chokes on for a reason other than a cycle + static class Unserializable { + public String getValue() { + throw new UnsupportedOperationException("not available"); + } + } + + private LogtailAppender appender; + private Logger logger; + + @Before + public void setUp() { + logger = new LoggerContext().getLogger(Logger.ROOT_LOGGER_NAME); + + appender = new LogtailAppender(); + appender.setAppName("BetterStackTest"); + appender.setSourceToken("dummy-token"); // any non-empty value; no request is made by these tests + } + + @Test + public void testCyclicArgumentIsOmittedAtTheCycleOnly() throws Exception { + Parent parent = new Parent(); + Child child = new Child(); + parent.child = child; + child.parent = parent; // the cycle + + log("Log line before the cyclic one"); + // Equivalent to HikariPool's: LOGGER.debug("{} - Added connection {}", poolName, poolEntry.connection); + log("Some object graph: {} and a sibling argument: {}", parent, Collections.singletonMap("sibling", true)); + log("Log line after the cyclic one: {}", Collections.singletonMap("orderId", 42)); + + String json = appender.batchToJson(3); + + assertTrue("Log line before the cyclic one must be sent", json.contains("Log line before the cyclic one")); + assertTrue("The cyclic log line itself must be sent", json.contains("Some object graph:")); + assertTrue("Log line after the cyclic one must be sent", json.contains("Log line after the cyclic one:")); + + assertTrue("The cyclic argument must stay structured up to the point where it loops back", + json.contains("{\"Child\":{\"Parent\":\"\"}}")); + assertTrue("Sibling arguments of the cyclic one must stay structured", + json.contains("{\"sibling\":true}")); + assertTrue("Arguments of other log lines must stay structured", + json.contains("{\"orderId\":42}")); + } + + @Test + public void testCyclicContainerArgumentsAreOmittedAtTheCycleOnly() throws Exception { + Map map = new HashMap<>(); + map.put("self", map); + List list = new ArrayList<>(); + list.add(list); + Object[] array = new Object[1]; + array[0] = array; + + log("A cyclic map: {}", map); + log("A cyclic list: {}", list); + log("A cyclic array: {}", new Object[]{array}); + + String json = appender.batchToJson(3); + + assertTrue("The cyclic map must be sent with its cycle omitted", + json.contains("{\"self\":\"\"}")); + assertTrue("The cyclic list and array must be sent with their cycles omitted", + json.contains("[\"\"]")); + assertTrue(json.contains("A cyclic map:")); + assertTrue(json.contains("A cyclic list:")); + assertTrue(json.contains("A cyclic array:")); + } + + @Test + public void testUnserializableArgumentIsOmittedWithAMarker() throws Exception { + log("Log line with a broken argument: {}", new Unserializable()); + log("Log line after the broken one"); + + String json = appender.batchToJson(2); + + assertTrue("The broken log line itself must be sent", json.contains("Log line with a broken argument:")); + assertTrue("Log line after the broken one must be sent", json.contains("Log line after the broken one")); + assertTrue("The broken argument must be replaced with a marker naming its class", + json.contains("\"\"")); + } + + @Test + public void testRepeatedReferencesAreNotMistakenForCycles() throws Exception { + Map shared = Collections.singletonMap("value", 42); + Map argument = new HashMap<>(); + argument.put("first", shared); + argument.put("second", shared); + + log("Same object referenced twice: {}", argument); + + String json = appender.batchToJson(1); + + assertFalse("A repeated reference is not a cycle and must not be omitted", json.contains("omitted")); + assertTrue(json.contains("\"first\":{\"value\":42}")); + assertTrue(json.contains("\"second\":{\"value\":42}")); + } + + private void log(String message, Object... args) { + appender.append(new LoggingEvent(Logger.FQCN, logger, Level.INFO, message, null, args)); + } +} From adc404c884dc3a14261f4863a2c09927f29d8eb5 Mon Sep 17 00:00:00 2001 From: Petr Heinz Date: Mon, 27 Jul 2026 20:54:11 +0200 Subject: [PATCH 04/10] T-19629 Add failing tests for best-effort serialization gaps Three gaps, each with a failing test committed before its fix: - guard() gives every Number instance a free pass, but Number is not final and Jackson stringifies unknown Number subclasses via toString - a broken subclass escapes both guards and drops the whole batch. - CycleGuard does not delegate unwrappingSerializer, so @JsonUnwrapped properties silently serialize nested instead of flattened. - CycleGuard does not delegate isEmpty, so @JsonInclude(NON_EMPTY) properties are emitted even when empty. Co-Authored-By: Claude Fable 5 --- .../logback/BestEffortSerializationTest.java | 59 +++++++++++++++++++ .../LogtailAppenderSerializationTest.java | 42 +++++++++++++ 2 files changed, 101 insertions(+) create mode 100644 src/test/java/com/logtail/logback/BestEffortSerializationTest.java diff --git a/src/test/java/com/logtail/logback/BestEffortSerializationTest.java b/src/test/java/com/logtail/logback/BestEffortSerializationTest.java new file mode 100644 index 0000000..c4681e2 --- /dev/null +++ b/src/test/java/com/logtail/logback/BestEffortSerializationTest.java @@ -0,0 +1,59 @@ +package com.logtail.logback; + +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonUnwrapped; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.Test; + +import java.util.ArrayList; +import java.util.List; + +import static org.junit.Assert.assertEquals; + +/** + * For anything that serializes fine, the module must be invisible: its output has to be + * byte-identical to a plain ObjectMapper, including for Jackson annotations that change + * how a serializer is used ({@code @JsonUnwrapped}) or when a value is included + * ({@code @JsonInclude(NON_EMPTY)}). + */ +public class BestEffortSerializationTest { + + private final ObjectMapper plain = new ObjectMapper(); + private final ObjectMapper bestEffort = new ObjectMapper().registerModule(new BestEffortSerialization()); + + public static class Name { + public String getFirst() { + return "Ada"; + } + + public String getLast() { + return "Lovelace"; + } + } + + public static class UnwrappedName { + @JsonUnwrapped + public Name getName() { + return new Name(); + } + } + + public static class NonEmptyItems { + @JsonInclude(JsonInclude.Include.NON_EMPTY) + public List getItems() { + return new ArrayList<>(); + } + } + + @Test + public void testUnwrappedPropertiesStayFlattened() throws Exception { + assertEquals(plain.writeValueAsString(new UnwrappedName()), + bestEffort.writeValueAsString(new UnwrappedName())); + } + + @Test + public void testEmptyValueInclusionIsRespected() throws Exception { + assertEquals(plain.writeValueAsString(new NonEmptyItems()), + bestEffort.writeValueAsString(new NonEmptyItems())); + } +} diff --git a/src/test/java/com/logtail/logback/LogtailAppenderSerializationTest.java b/src/test/java/com/logtail/logback/LogtailAppenderSerializationTest.java index 58cbf0a..714b5d5 100644 --- a/src/test/java/com/logtail/logback/LogtailAppenderSerializationTest.java +++ b/src/test/java/com/logtail/logback/LogtailAppenderSerializationTest.java @@ -48,6 +48,35 @@ public String getValue() { } } + // Number is not final: a subclass can fail during serialization (Jackson stringifies unknown + // Number types via toString), and scalar-looking arguments must not get a free pass because of it + static class BrokenNumber extends Number { + @Override + public int intValue() { + throw new UnsupportedOperationException("not available"); + } + + @Override + public long longValue() { + throw new UnsupportedOperationException("not available"); + } + + @Override + public float floatValue() { + throw new UnsupportedOperationException("not available"); + } + + @Override + public double doubleValue() { + throw new UnsupportedOperationException("not available"); + } + + @Override + public String toString() { + throw new UnsupportedOperationException("not available"); + } + } + private LogtailAppender appender; private Logger logger; @@ -123,6 +152,19 @@ public void testUnserializableArgumentIsOmittedWithAMarker() throws Exception { json.contains("\"\"")); } + @Test + public void testBrokenScalarLikeArgumentDoesNotDropTheBatch() throws Exception { + log("Log line with a broken number: {}", new BrokenNumber()); + log("Log line after the broken number"); + + String json = appender.batchToJson(2); + + assertTrue("The broken log line itself must be sent", json.contains("Log line with a broken number:")); + assertTrue("Log line after the broken number must be sent", json.contains("Log line after the broken number")); + assertTrue("The broken argument must be replaced with a marker naming its class", + json.contains("\"\"")); + } + @Test public void testRepeatedReferencesAreNotMistakenForCycles() throws Exception { Map shared = Collections.singletonMap("value", 42); From 0ad9a4749a28b695049c42e6aaf1c5686eee806d Mon Sep 17 00:00:00 2001 From: Petr Heinz Date: Mon, 27 Jul 2026 20:54:36 +0200 Subject: [PATCH 05/10] T-19629 Never let a non-final scalar type skip the argument guard Number is not final and Jackson serializes unknown Number subclasses via toString, so one with a throwing toString escaped guard() and threw during batch serialization, dropping the whole batch - the exact bug this branch exists to fix. Only exact final JDK scalar types skip the guard now. Co-Authored-By: Claude Fable 5 --- .../com/logtail/logback/BestEffortSerialization.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/logtail/logback/BestEffortSerialization.java b/src/main/java/com/logtail/logback/BestEffortSerialization.java index 5dbdbe0..3cc5442 100644 --- a/src/main/java/com/logtail/logback/BestEffortSerialization.java +++ b/src/main/java/com/logtail/logback/BestEffortSerialization.java @@ -70,12 +70,21 @@ public JsonSerializer modifyMapSerializer(SerializationConfig config, MapType * string instead of failing the serialization of everything around it. */ public static Object guard(Object value) { - if (value == null || value instanceof String || value instanceof Number || value instanceof Boolean) { + if (value == null || isSafeScalar(value)) { return value; } return new Guarded(value); } + // Only exact final JDK types whose serializers cannot fail may skip the guard - an instanceof check + // is not enough, e.g. a Number subclass can still throw from the toString Jackson serializes it with + private static boolean isSafeScalar(Object value) { + Class type = value.getClass(); + return type == String.class || type == Boolean.class || type == Character.class + || type == Integer.class || type == Long.class || type == Double.class + || type == Float.class || type == Short.class || type == Byte.class; + } + static final class Guarded { final Object value; From c61ad8d8be470433cd8586a14329854b8f3a2825 Mon Sep 17 00:00:00 2001 From: Petr Heinz Date: Mon, 27 Jul 2026 20:55:10 +0200 Subject: [PATCH 06/10] T-19629 Keep @JsonUnwrapped properties flattened under the cycle guard JsonSerializer.unwrappingSerializer returns the serializer itself unless overridden, so the guard wrapper swallowed the delegate's unwrapping variant and @JsonUnwrapped beans serialized nested. The guard now wraps the delegate's unwrapping serializer instead, keeping both the flattened shape and the cycle protection. Co-Authored-By: Claude Fable 5 --- .../com/logtail/logback/BestEffortSerialization.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/main/java/com/logtail/logback/BestEffortSerialization.java b/src/main/java/com/logtail/logback/BestEffortSerialization.java index 3cc5442..d0528bf 100644 --- a/src/main/java/com/logtail/logback/BestEffortSerialization.java +++ b/src/main/java/com/logtail/logback/BestEffortSerialization.java @@ -15,6 +15,7 @@ import com.fasterxml.jackson.databind.type.ArrayType; import com.fasterxml.jackson.databind.type.CollectionType; import com.fasterxml.jackson.databind.type.MapType; +import com.fasterxml.jackson.databind.util.NameTransformer; import com.fasterxml.jackson.databind.util.TokenBuffer; import java.io.IOException; @@ -142,6 +143,16 @@ public void serializeWithType(Object value, JsonGenerator gen, SerializerProvide } } + @Override + public JsonSerializer unwrappingSerializer(NameTransformer transformer) { + return new CycleGuard(delegate.unwrappingSerializer(transformer)); + } + + @Override + public boolean isUnwrappingSerializer() { + return delegate.isUnwrappingSerializer(); + } + @Override public void resolve(SerializerProvider provider) throws JsonMappingException { if (delegate instanceof ResolvableSerializer) { From b564d78982dd4bc9681c9a64b1215e31ce2401e4 Mon Sep 17 00:00:00 2001 From: Petr Heinz Date: Mon, 27 Jul 2026 20:55:31 +0200 Subject: [PATCH 07/10] T-19629 Respect custom empty-value inclusion under the cycle guard The guard wrapper inherited the default isEmpty (null check only), so @JsonInclude(NON_EMPTY) properties were emitted even when empty. Delegated to the wrapped serializer. Co-Authored-By: Claude Fable 5 --- .../java/com/logtail/logback/BestEffortSerialization.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/main/java/com/logtail/logback/BestEffortSerialization.java b/src/main/java/com/logtail/logback/BestEffortSerialization.java index d0528bf..6d14326 100644 --- a/src/main/java/com/logtail/logback/BestEffortSerialization.java +++ b/src/main/java/com/logtail/logback/BestEffortSerialization.java @@ -143,6 +143,11 @@ public void serializeWithType(Object value, JsonGenerator gen, SerializerProvide } } + @Override + public boolean isEmpty(SerializerProvider provider, Object value) { + return delegate.isEmpty(provider, value); + } + @Override public JsonSerializer unwrappingSerializer(NameTransformer transformer) { return new CycleGuard(delegate.unwrappingSerializer(transformer)); From 4104219f6d8f371b25e9008a1a5b5c7c70d3ba35 Mon Sep 17 00:00:00 2001 From: Petr Heinz Date: Tue, 28 Jul 2026 09:57:21 +0200 Subject: [PATCH 08/10] T-19629 Add tests for cycles outside guarded types and polymorphic typing A cyclic JsonNode serializes through its own JsonSerializable path that no serializer-modifier hook wraps - a regression test pins that such cycles fall back to whole-argument omission instead of dropping the batch. The polymorphic typing test fails: GuardedSerializer inherits a serializeWithType that throws, so a mapper with default typing enabled cannot serialize guarded arguments at all. Co-Authored-By: Claude Fable 5 --- .../logback/BestEffortSerializationTest.java | 18 +++++++++++++++++ .../LogtailAppenderSerializationTest.java | 20 +++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/src/test/java/com/logtail/logback/BestEffortSerializationTest.java b/src/test/java/com/logtail/logback/BestEffortSerializationTest.java index c4681e2..d838e7c 100644 --- a/src/test/java/com/logtail/logback/BestEffortSerializationTest.java +++ b/src/test/java/com/logtail/logback/BestEffortSerializationTest.java @@ -3,12 +3,16 @@ import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonUnwrapped; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.jsontype.impl.LaissezFaireSubTypeValidator; import org.junit.Test; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; /** * For anything that serializes fine, the module must be invisible: its output has to be @@ -56,4 +60,18 @@ public void testEmptyValueInclusionIsRespected() throws Exception { assertEquals(plain.writeValueAsString(new NonEmptyItems()), bestEffort.writeValueAsString(new NonEmptyItems())); } + + @Test + public void testGuardedValuesSurvivePolymorphicTyping() throws Exception { + ObjectMapper typed = new ObjectMapper() + .registerModule(new BestEffortSerialization()) + .activateDefaultTyping(LaissezFaireSubTypeValidator.instance, ObjectMapper.DefaultTyping.OBJECT_AND_NON_CONCRETE); + + Map value = new HashMap<>(); + value.put("key", "value"); + + String json = typed.writeValueAsString(new Object[]{BestEffortSerialization.guard(value)}); + + assertTrue("Guarded values must serialize under polymorphic typing", json.contains("\"key\":\"value\"")); + } } diff --git a/src/test/java/com/logtail/logback/LogtailAppenderSerializationTest.java b/src/test/java/com/logtail/logback/LogtailAppenderSerializationTest.java index 714b5d5..ce9fa34 100644 --- a/src/test/java/com/logtail/logback/LogtailAppenderSerializationTest.java +++ b/src/test/java/com/logtail/logback/LogtailAppenderSerializationTest.java @@ -4,6 +4,8 @@ import ch.qos.logback.classic.Logger; import ch.qos.logback.classic.LoggerContext; import ch.qos.logback.classic.spi.LoggingEvent; +import com.fasterxml.jackson.databind.node.JsonNodeFactory; +import com.fasterxml.jackson.databind.node.ObjectNode; import org.junit.Before; import org.junit.Test; @@ -152,6 +154,24 @@ public void testUnserializableArgumentIsOmittedWithAMarker() throws Exception { json.contains("\"\"")); } + @Test + public void testCyclicArgumentOutsideGuardedTypesDoesNotDropTheBatch() throws Exception { + // JsonNode serializes through its own JsonSerializable path, which no serializer-modifier hook + // wraps - the cycle guard cannot see this cycle, so it must fall back to whole-argument omission + ObjectNode node = JsonNodeFactory.instance.objectNode(); + node.put("key", "value"); + node.set("self", node); + + log("A cyclic JsonNode: {}", node); + log("Log line after the cyclic node"); + + String json = appender.batchToJson(2); + + assertTrue("Log line after the cyclic node must be sent", json.contains("Log line after the cyclic node")); + assertTrue("A cycle the cycle guard cannot see must still be omitted as a whole argument", + json.contains("\"\"")); + } + @Test public void testBrokenScalarLikeArgumentDoesNotDropTheBatch() throws Exception { log("Log line with a broken number: {}", new BrokenNumber()); From 164426dc29c575c16b69726c08a310d1d87b1571 Mon Sep 17 00:00:00 2001 From: Petr Heinz Date: Tue, 28 Jul 2026 09:57:50 +0200 Subject: [PATCH 09/10] T-19629 Serialize guarded values under polymorphic typing The guard wrapper is invisible in the output, so serializeWithType has no type id of its own to write - it now defers to the plain serialize, letting the wrapped value's serializer emit its type info inside the buffer instead of inheriting the base implementation that throws. Co-Authored-By: Claude Fable 5 --- .../java/com/logtail/logback/BestEffortSerialization.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/main/java/com/logtail/logback/BestEffortSerialization.java b/src/main/java/com/logtail/logback/BestEffortSerialization.java index 6d14326..820ed9c 100644 --- a/src/main/java/com/logtail/logback/BestEffortSerialization.java +++ b/src/main/java/com/logtail/logback/BestEffortSerialization.java @@ -107,6 +107,13 @@ public void serialize(Guarded guarded, JsonGenerator gen, SerializerProvider pro } buffer.serialize(gen); } + + @Override + public void serializeWithType(Guarded guarded, JsonGenerator gen, SerializerProvider provider, TypeSerializer typeSer) throws IOException { + // The wrapper is invisible in the output, so there is no type id to write for it - the + // wrapped value's own serializer emits its type info inside the buffer + serialize(guarded, gen, provider); + } } static final class CycleGuard extends JsonSerializer implements ContextualSerializer, ResolvableSerializer { From 39381fa064c7bfac884b0d2acb4a841e0585e2c7 Mon Sep 17 00:00:00 2001 From: Petr Heinz Date: Tue, 28 Jul 2026 13:00:33 +0200 Subject: [PATCH 10/10] Bump version to 0.3.6 Co-Authored-By: Claude Fable 5 --- examples/gradle/build.gradle | 2 +- examples/maven/pom.xml | 2 +- pom.xml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/examples/gradle/build.gradle b/examples/gradle/build.gradle index 3713f92..f7266af 100644 --- a/examples/gradle/build.gradle +++ b/examples/gradle/build.gradle @@ -7,7 +7,7 @@ repositories { } dependencies { - implementation 'com.logtail:logback-logtail:0.3.5' + implementation 'com.logtail:logback-logtail:0.3.6' implementation 'ch.qos.logback:logback-classic:1.2.11' implementation 'ch.qos.logback:logback-core:1.2.11' implementation 'com.fasterxml.jackson.core:jackson-databind:2.13.5' diff --git a/examples/maven/pom.xml b/examples/maven/pom.xml index 0a8ee59..48bfaef 100644 --- a/examples/maven/pom.xml +++ b/examples/maven/pom.xml @@ -26,7 +26,7 @@ com.logtail logback-logtail - 0.3.5 + 0.3.6 ch.qos.logback diff --git a/pom.xml b/pom.xml index dffb4e1..e1bda28 100644 --- a/pom.xml +++ b/pom.xml @@ -11,7 +11,7 @@ com.logtail logback-logtail jar - 0.3.5 + 0.3.6 ${project.groupId}:${project.artifactId} Logback Java appender for sending logs to BetterStack.com