Skip to content

Commit

Permalink
Feat: Write full message into short_message field
Browse files Browse the repository at this point in the history 
Following the principle of least surprise, we write messages completely into
thethe `short_message` field, even if they contain several lines.

As a consequence, we deprecate the `full_message` field, as it becomes
redundant.

Resolves Graylog2#11
  • Loading branch information
Valentin Krasontovitsch committed Apr 17, 2018
1 parent 4dbb9d7 commit f912e10
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 89 deletions.
17 changes: 1 addition & 16 deletions gelf/message.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ type Message struct {
Version string `json:"version"`
Host string `json:"host"`
Short string `json:"short_message"`
Full string `json:"full_message,omitempty"`
TimeUnix float64 `json:"timestamp"`
Level int32 `json:"level,omitempty"`
Facility string `json:"facility,omitempty"`
Expand Down Expand Up @@ -94,8 +93,6 @@ func (m *Message) UnmarshalJSON(data []byte) error {
m.Host, ok = v.(string)
case "short_message":
m.Short, ok = v.(string)
case "full_message":
m.Full, ok = v.(string)
case "timestamp":
m.TimeUnix, ok = v.(float64)
case "level":
Expand Down Expand Up @@ -127,22 +124,10 @@ 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),
Short: string(p),
TimeUnix: float64(time.Now().Unix()),
Level: 6, // info
Facility: facility,
Expand Down
9 changes: 8 additions & 1 deletion gelf/message_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ func TestWrongFieldTypes(t *testing.T) {
"version": `{"version": 1.1}`,
"host": `{"host": ["a", "b"]}`,
"short_message": `{"short_message": {"a": "b"}}`,
"full_message": `{"full_message": null}`,
"timestamp": `{"timestamp": "12345"}`,
"level": `{"level": false}`,
"facility": `{"facility": true}`,
Expand All @@ -25,3 +24,11 @@ func TestWrongFieldTypes(t *testing.T) {
}

}

func Test_constructMessage_WritesMultilineMessageToShortMessageField(t *testing.T) {
msgText := "hello\nthere"
msg := constructMessage([]byte(msgText), "", "", "", 0)
if msg.Short != msgText {
t.Errorf("Short field of message \"%s\" does not coincide with message text \"%s\"", msg.Short, msgText)
}
}
10 changes: 1 addition & 9 deletions gelf/reader.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,7 @@ func (r *Reader) Read(p []byte) (int, error) {
return -1, err
}

var data string

if msg.Full == "" {
data = msg.Short
} else {
data = msg.Full
}

return strings.NewReader(data).Read(p)
return strings.NewReader(msg.Short).Read(p)
}

func (r *Reader) ReadMessage() (*Message, error) {
Expand Down
27 changes: 10 additions & 17 deletions gelf/tcpwriter_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,16 +51,11 @@ func TestNewTCPWriterConfig(t *testing.T) {
}
}

func assertMessages(msg *Message, short, full string, t *testing.T) {
func assertMessages(msg *Message, short string, t *testing.T) {
if msg.Short != short {
t.Errorf("msg.Short: expected %s, got %s", short, msg.Short)
return
}

if msg.Full != full {
t.Errorf("msg.Full: expected %s, got %s", full, msg.Full)
}

}

func TestWriteSmallMultiLineTCP(t *testing.T) {
Expand All @@ -72,7 +67,7 @@ func TestWriteSmallMultiLineTCP(t *testing.T) {
return
}

assertMessages(msg, "awesomesauce", msgData, t)
assertMessages(msg, msgData, t)
}

func TestWriteSmallOneLineTCP(t *testing.T) {
Expand All @@ -85,7 +80,7 @@ func TestWriteSmallOneLineTCP(t *testing.T) {
return
}

assertMessages(msg, msgDataTrunc, "", t)
assertMessages(msg, msgDataTrunc, t)

fileExpected := "/go-gelf/gelf/tcpwriter_test.go"
if !strings.HasSuffix(msg.Extra["_file"].(string), fileExpected) {
Expand Down Expand Up @@ -114,7 +109,7 @@ func TestWriteBigMessageTCP(t *testing.T) {
return
}

assertMessages(msg, "awesomesauce", msgData, t)
assertMessages(msg, msgData, t)
}

func TestWriteMultiPacketMessageTCP(t *testing.T) {
Expand All @@ -131,7 +126,7 @@ func TestWriteMultiPacketMessageTCP(t *testing.T) {
return
}

assertMessages(msg, "awesomesauce", msgData, t)
assertMessages(msg, msgData, t)
}

func TestExtraDataTCP(t *testing.T) {
Expand All @@ -145,13 +140,11 @@ func TestExtraDataTCP(t *testing.T) {
"_line": 186,
}

short := "quick"
full := short + "\nwith more detail"
short := "quick" + "\nwith more detail"
m := Message{
Version: "1.0",
Host: "fake-host",
Short: string(short),
Full: string(full),
TimeUnix: float64(time.Now().Unix()),
Level: 6, // info
Facility: "writer_test",
Expand All @@ -165,7 +158,7 @@ func TestExtraDataTCP(t *testing.T) {
return
}

assertMessages(msg, short, full, t)
assertMessages(msg, short, t)

if len(msg.Extra) != 3 {
t.Errorf("extra extra fields in %v", msg.Extra)
Expand Down Expand Up @@ -198,8 +191,8 @@ func TestWrite2MessagesWithConnectionDropTCP(t *testing.T) {
return
}

assertMessages(msg1, "First message", msgData1, t)
assertMessages(msg2, "Second message", msgData2, t)
assertMessages(msg1, msgData1, t)
assertMessages(msg2, msgData2, t)
}

func TestWrite2MessagesWithServerDropTCP(t *testing.T) {
Expand All @@ -212,7 +205,7 @@ func TestWrite2MessagesWithServerDropTCP(t *testing.T) {
return
}

assertMessages(msg1, "First message", msgData1, t)
assertMessages(msg1, msgData1, t)
}

func setupConnections() (*TCPReader, chan string, chan string, *TCPWriter, error) {
Expand Down
51 changes: 5 additions & 46 deletions gelf/udpwriter_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, t)
}
}

Expand All @@ -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, t)

fileExpected := "/go-gelf/gelf/udpwriter_test.go"
if !strings.HasSuffix(msg.Extra["_file"].(string), fileExpected) {
Expand Down Expand Up @@ -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, t)
}
}

Expand All @@ -183,13 +156,11 @@ func TestExtraData(t *testing.T) {
"_line": 186,
}

short := "quick"
full := short + "\nwith more detail"
short := "quick\nwith more detail"
m := Message{
Version: "1.0",
Host: "fake-host",
Short: string(short),
Full: string(full),
TimeUnix: float64(time.Now().Unix()),
Level: 6, // info
Facility: "udpwriter_test",
Expand All @@ -204,15 +175,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, t)

if len(msg.Extra) != 3 {
t.Errorf("extra extra fields in %v", msg.Extra)
Expand Down Expand Up @@ -253,7 +216,6 @@ func BenchmarkWriteBestSpeed(b *testing.B) {
Version: "1.1",
Host: w.hostname,
Short: "short message",
Full: "full message",
TimeUnix: float64(time.Now().Unix()),
Level: 6, // info
Facility: w.Facility,
Expand All @@ -279,7 +241,6 @@ func BenchmarkWriteNoCompression(b *testing.B) {
Version: "1.1",
Host: w.hostname,
Short: "short message",
Full: "full message",
TimeUnix: float64(time.Now().Unix()),
Level: 6, // info
Facility: w.Facility,
Expand All @@ -305,7 +266,6 @@ func BenchmarkWriteDisableCompressionCompletely(b *testing.B) {
Version: "1.1",
Host: w.hostname,
Short: "short message",
Full: "full message",
TimeUnix: float64(time.Now().Unix()),
Level: 6, // info
Facility: w.Facility,
Expand All @@ -331,7 +291,6 @@ func BenchmarkWriteDisableCompressionAndPreencodeExtra(b *testing.B) {
Version: "1.1",
Host: w.hostname,
Short: "short message",
Full: "full message",
TimeUnix: float64(time.Now().Unix()),
Level: 6, // info
Facility: w.Facility,
Expand Down

0 comments on commit f912e10

Please sign in to comment.