tune MessageIdSupplier

build.gradle.kts

@@ -26,6 +26,7 @@ dependencies {
     api("ch.qos.logback:logback-classic:1.4.11")
     testImplementation("org.junit.jupiter:junit-jupiter:5.10.0")
     testRuntimeOnly("org.junit.platform:junit-platform-launcher")
+    testImplementation("org.assertj:assertj-core:3.11.1")
     testImplementation("com.fasterxml.jackson.core:jackson-databind:2.15.2")
     testImplementation("org.bouncycastle:bcpkix-jdk18on:1.76")
 }

src/main/java/de/siegmar/logbackgelf/MessageIdSupplier.java

@@ -19,31 +19,55 @@
 
 package de.siegmar.logbackgelf;
 
-import java.util.Random;
+import java.util.concurrent.ThreadLocalRandom;
 import java.util.function.LongSupplier;
 
 /**
  * Supplier implementation for GELF message IDs as used for UDP chunks. Unfortunately the GELF
- * protocol limits the message id length to 8 bytes thus an UUID cannot be used (16 bytes).
+ * protocol limits the message id length to 8 bytes thus a UUID cannot be used (16 bytes).
  */
 public class MessageIdSupplier implements LongSupplier {
 
-    // static random to keep Spotbugs happy
-    private static final Random RANDOM = new Random();
-    private int machinePart = RANDOM.nextInt();
+    private static final long BITS_13 = 0b1_1111_1111_1111;
 
-    public int getMachinePart() {
-        return machinePart;
+    @SuppressWarnings("checkstyle:magicnumber")
+    @Override
+    public long getAsLong() {
+        /*
+         * Idea is borrowed from logstash-gelf by <a href="https://github.com/mp911de">Mark Paluch</a>, MIT licensed
+         * <a href="https://github.com/mp911de/logstash-gelf/blob/a938063de1f822c8d26c8d51ed3871db24355017/src/main/java/biz/paluch/logging/gelf/intern/GelfMessage.java">GelfMessage.java</a>
+         *
+         * Considerations about generating the message ID: The GELF documentation suggests to
+         * "Generate from millisecond timestamp + hostname, for example.":
+         * https://go2docs.graylog.org/5-1/getting_in_log_data/gelf.html#GELFviaUDP
+         *
+         * However, relying on current time in milliseconds on the same system will result in a high collision
+         * probability if lots of messages are generated quickly. Things will be even worse if multiple servers send
+         * to the same log server. Adding the hostname is not guaranteed to help, and if the hostname is the FQDN it
+         * is even unlikely to be unique at all.
+         *
+         * The GELF module used by Logstash uses the first eight bytes of an MD5 hash of the current time as floating
+         * point, a hyphen, and an eight byte random number: https://github.com/logstash-plugins/logstash-output-gelf
+         * https://github.com/graylog-labs/gelf-rb/blob/master/lib/gelf/notifier.rb#L239 It probably doesn't have to
+         * be that clever:
+         *
+         * Using the timestamp plus a random number will mean we only have to worry about collision of random numbers
+         * within the same milliseconds. How short can the timestamp be before it will collide with old timestamps?
+         * Every second Graylog will evict expired messaged (5 seconds old) from the pool:
+         * https://github.com/Graylog2/graylog2-server/blob/master/graylog2-server/src/main/java/org/graylog2/inputs/codecs/
+         * GelfChunkAggregator.java Thus, we just need six seconds which will require 13 bits.
+         * Then we can spend the rest on a random number.
+         */
+
+        return (random() & ~BITS_13) | (currentTime() & BITS_13);
     }
 
-    public void setMachinePart(final int machinePart) {
-        this.machinePart = machinePart;
+    long random() {
+        return ThreadLocalRandom.current().nextLong();
     }
 
-    @SuppressWarnings("checkstyle:magicnumber")
-    @Override
-    public long getAsLong() {
-        return (long) machinePart << 32 | System.nanoTime() & 0xffffffffL;
+    long currentTime() {
+        return System.currentTimeMillis();
     }
 
 }

src/test/java/de/siegmar/logbackgelf/GelfEncoderTest.java

@@ -86,6 +86,7 @@ public void simple() throws IOException {
 
     @Test
     public void newline() {
+
         encoder.setAppendNewline(true);
         encoder.start();
 

src/test/java/de/siegmar/logbackgelf/MessageIdSupplierTest.java

@@ -19,25 +19,39 @@
 
 package de.siegmar.logbackgelf;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
 
 import org.junit.jupiter.api.Test;
 
-public class MessageIdSupplierTest {
-
-    private final MessageIdSupplier messageIdSupplier = new MessageIdSupplier();
+class MessageIdSupplierTest {
 
     @Test
-    public void test() {
-        final Long messageId = messageIdSupplier.getAsLong();
-        assertNotEquals(messageId, messageIdSupplier.getAsLong());
+    void random() {
+        final MessageIdSupplier mis = new MessageIdSupplier();
+        assertNotEquals(mis.getAsLong(), mis.getAsLong());
+
+        assertThat(mis.getAsLong())
+            .isNotZero()
+            .isNotEqualTo(mis.getAsLong());
     }
 
     @Test
-    void machinePart() {
-        messageIdSupplier.setMachinePart(23282);
-        final Long messageId = messageIdSupplier.getAsLong();
-        assertNotEquals(messageId, messageIdSupplier.getAsLong());
+    void fixed() {
+        final MessageIdSupplier mis = new MessageIdSupplier() {
+            @Override
+            long random() {
+                return 0x776D_11CF_72A6_B233L;
+            }
+
+            @Override
+            long currentTime() {
+                return 0x18B_0B3E_CA4CL;
+            }
+        };
+
+        assertThat(mis.getAsLong())
+            .isEqualTo(0x776D_11CF_72A6_AA4CL);
     }
 
 }