T-19629 Keep sending the batch when a log argument can't be serialized

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'

examples/maven/pom.xml

@@ -26,7 +26,7 @@
         <dependency>
             <groupId>com.logtail</groupId>
             <artifactId>logback-logtail</artifactId>
-            <version>0.3.5</version>
+            <version>0.3.6</version>
         </dependency>
         <dependency>
             <groupId>ch.qos.logback</groupId>

pom.xml

@@ -11,7 +11,7 @@
     <groupId>com.logtail</groupId>
     <artifactId>logback-logtail</artifactId>
     <packaging>jar</packaging>
-    <version>0.3.5</version>
+    <version>0.3.6</version>
 
     <name>${project.groupId}:${project.artifactId}</name>
     <description>Logback Java appender for sending logs to BetterStack.com</description>

src/main/java/com/logtail/logback/BestEffortSerialization.java

@@ -0,0 +1,204 @@
+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.NameTransformer;
+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:
+ * <ul>
+ * <li>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 "<omitted circular reference>"} while the rest of the
+ * object stays intact,</li>
+ * <li>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.</li>
+ * </ul>
+ */
+public class BestEffortSerialization extends SimpleModule {
+
+    static final String CIRCULAR_REFERENCE_MARKER = "<omitted circular reference>";
+
+    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 || 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;
+
+        Guarded(Object value) {
+            this.value = value;
+        }
+    }
+
+    static final class GuardedSerializer extends JsonSerializer<Guarded> {
+        @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("<omitted unserializable " + guarded.value.getClass().getName() + ">");
+                return;
+            }
+            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<Object> implements ContextualSerializer, ResolvableSerializer {
+        private final JsonSerializer<Object> delegate;
+
+        @SuppressWarnings("unchecked")
+        CycleGuard(JsonSerializer<?> delegate) {
+            this.delegate = (JsonSerializer<Object>) 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 boolean isEmpty(SerializerProvider provider, Object value) {
+            return delegate.isEmpty(provider, value);
+        }
+
+        @Override
+        public JsonSerializer<Object> 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) {
+                ((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<Object> ancestors = ancestors(provider);
+        if (ancestors == null) {
+            ancestors = Collections.newSetFromMap(new IdentityHashMap<Object, Boolean>());
+            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<Object> ancestors(SerializerProvider provider) {
+        return (Set<Object>) provider.getAttribute(ANCESTORS);
+    }
+}

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);
@@ -269,14 +270,21 @@ protected Map<String, Object> 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()));
         }
 
         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();
     }

src/test/java/com/logtail/logback/BestEffortSerializationTest.java

@@ -0,0 +1,77 @@
+package com.logtail.logback;
+
+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
+ * 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<String> 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()));
+    }
+
+    @Test
+    public void testGuardedValuesSurvivePolymorphicTyping() throws Exception {
+        ObjectMapper typed = new ObjectMapper()
+                .registerModule(new BestEffortSerialization())
+                .activateDefaultTyping(LaissezFaireSubTypeValidator.instance, ObjectMapper.DefaultTyping.OBJECT_AND_NON_CONCRETE);
+
+        Map<String, Object> 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\""));
+    }
+}