Feat: Write full message into short_message field

Following the principle of least surprise, we write messages completely into the `short_message` field, even if they contain several lines. To not break gelf standard, we also write the full message, always, to the `full_message` field. Resolves Graylog2#11
valentin-krasontovitsch · Dec 4, 2019 · a370f46 · a370f46
1 parent 1550ee6
commit a370f46
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 63 deletions.
diff --git a/gelf/message.go b/gelf/message.go
@@ -125,23 +125,12 @@ func constructMessage(p []byte, hostname string, facility string, file string, l
 	// remove trailing and leading whitespace
 	p = bytes.TrimSpace(p)
 
-	// If there are newlines in the message, use the first line
-	// for the short message and set the full message to the
-	// original input.  If the input has no newlines, stick the
-	// whole thing in Short.
-	short := p
-	full := []byte("")
-	if i := bytes.IndexRune(p, '\n'); i > 0 {
-		short = p[:i]
-		full = p
-	}
-
 	m = &Message{
 		Version:  "1.1",
 		Host:     hostname,
-		Short:    string(short),
-		Full:     string(full),
 		TimeUnix: float64(time.Now().UnixNano()) / float64(time.Second),
+		Short:    string(p),
+		Full:     string(p),
 		Level:    6, // info
 		Facility: facility,
 		Extra: map[string]interface{}{

diff --git a/gelf/message_test.go b/gelf/message_test.go
@@ -25,3 +25,11 @@ func TestWrongFieldTypes(t *testing.T) {
 	}
 
 }
+
+func Test_constructMessage_WritesMultilineMessageToBothMessageFields(t *testing.T) {
+	msgText := "hello\nthere"
+	msg := constructMessage([]byte(msgText), "", "", "", 0)
+	if msg.Short != msgText || msg.Full != msgText {
+		t.Errorf("Short field of message \"%s\" does not coincide with message text \"%s\"", msg.Short, msgText)
+	}
+}
diff --git a/gelf/tcpwriter_test.go b/gelf/tcpwriter_test.go
@@ -72,7 +72,7 @@ func TestWriteSmallMultiLineTCP(t *testing.T) {
 		return
 	}
 
-	assertMessages(msg, "awesomesauce", msgData, t)
+	assertMessages(msg, msgData, msgData, t)
 }
 
 func TestWriteSmallOneLineTCP(t *testing.T) {
@@ -85,7 +85,7 @@ func TestWriteSmallOneLineTCP(t *testing.T) {
 		return
 	}
 
-	assertMessages(msg, msgDataTrunc, "", t)
+	assertMessages(msg, msgDataTrunc, msgDataTrunc, t)
 
 	fileExpected := "/go-gelf/gelf/tcpwriter_test.go"
 	if !strings.HasSuffix(msg.Extra["_file"].(string), fileExpected) {
@@ -114,7 +114,7 @@ func TestWriteBigMessageTCP(t *testing.T) {
 		return
 	}
 
-	assertMessages(msg, "awesomesauce", msgData, t)
+	assertMessages(msg, msgData, msgData, t)
 }
 
 func TestWriteMultiPacketMessageTCP(t *testing.T) {
@@ -131,7 +131,7 @@ func TestWriteMultiPacketMessageTCP(t *testing.T) {
 		return
 	}
 
-	assertMessages(msg, "awesomesauce", msgData, t)
+	assertMessages(msg, msgData, msgData, t)
 }
 
 func TestExtraDataTCP(t *testing.T) {
@@ -145,8 +145,8 @@ func TestExtraDataTCP(t *testing.T) {
 		"_line": 186,
 	}
 
-	short := "quick"
-	full := short + "\nwith more detail"
+	short := "quick" + "\nwith more detail"
+	full := short
 	m := Message{
 		Version:  "1.0",
 		Host:     "fake-host",
@@ -198,8 +198,8 @@ func TestWrite2MessagesWithConnectionDropTCP(t *testing.T) {
 		return
 	}
 
-	assertMessages(msg1, "First message", msgData1, t)
-	assertMessages(msg2, "Second message", msgData2, t)
+	assertMessages(msg1, msgData1, msgData1, t)
+	assertMessages(msg2, msgData2, msgData2, t)
 }
 
 func TestWrite2MessagesWithServerDropTCP(t *testing.T) {
@@ -212,7 +212,7 @@ func TestWrite2MessagesWithServerDropTCP(t *testing.T) {
 		return
 	}
 
-	assertMessages(msg1, "First message", msgData1, t)
+	assertMessages(msg1, msgData1, msgData1, t)
 }
 
 func setupConnections() (*TCPReader, chan string, chan string, *TCPWriter, error) {

diff --git a/gelf/udpwriter_test.go b/gelf/udpwriter_test.go
@@ -76,16 +76,7 @@ func TestWriteSmallMultiLine(t *testing.T) {
 			t.Errorf("sendAndRecv: %s", err)
 			return
 		}
-
-		if msg.Short != "awesomesauce" {
-			t.Errorf("msg.Short: expected %s, got %s", "awesomesauce", msg.Full)
-			return
-		}
-
-		if msg.Full != msgData {
-			t.Errorf("msg.Full: expected %s, got %s", msgData, msg.Full)
-			return
-		}
+		assertMessages(msg, msgData, msgData, t)
 	}
 }
 
@@ -100,17 +91,7 @@ func TestWriteSmallOneLine(t *testing.T) {
 		return
 	}
 
-	// we should remove the trailing newline
-	if msg.Short != msgDataTrunc {
-		t.Errorf("msg.Short: expected %s, got %s",
-			msgDataTrunc, msg.Short)
-		return
-	}
-
-	if msg.Full != "" {
-		t.Errorf("msg.Full: expected %s, got %s", msgData, msg.Full)
-		return
-	}
+	assertMessages(msg, msgDataTrunc, msgDataTrunc, t)
 
 	fileExpected := "/go-gelf/gelf/udpwriter_test.go"
 	if !strings.HasSuffix(msg.Extra["_file"].(string), fileExpected) {
@@ -159,15 +140,7 @@ func TestWriteBigChunked(t *testing.T) {
 			return
 		}
 
-		if msg.Short != "awesomesauce" {
-			t.Errorf("msg.Short: expected %s, got %s", msgData, msg.Full)
-			return
-		}
-
-		if msg.Full != msgData {
-			t.Errorf("msg.Full: expected %s, got %s", msgData, msg.Full)
-			return
-		}
+		assertMessages(msg, msgData, msgData, t)
 	}
 }
 
@@ -183,8 +156,8 @@ func TestExtraData(t *testing.T) {
 		"_line": 186,
 	}
 
-	short := "quick"
-	full := short + "\nwith more detail"
+	short := "quick\nwith more detail"
+	full := short
 	m := Message{
 		Version:  "1.0",
 		Host:     "fake-host",
@@ -204,15 +177,7 @@ func TestExtraData(t *testing.T) {
 			return
 		}
 
-		if msg.Short != short {
-			t.Errorf("msg.Short: expected %s, got %s", short, msg.Full)
-			return
-		}
-
-		if msg.Full != full {
-			t.Errorf("msg.Full: expected %s, got %s", full, msg.Full)
-			return
-		}
+		assertMessages(msg, short, full, t)
 
 		if len(msg.Extra) != 3 {
 			t.Errorf("extra extra fields in %v", msg.Extra)