Skip to content

Commit

Permalink
fluent-bit: sort JSON map
Browse files Browse the repository at this point in the history 
While developing another fix, I stumbled upon grafana#1309 as a newly written
unit test (with multiple key-value pairs in a map) was flaky.

While JSON does not [strictly define an order on records in a map][RFC8259],
but practical operations with a logging tool pretty much require it
(although of course grep for JSON is jq, not grep...). Also, the key
value output format is already sorted.

Switching to sorted output in jsoniter is pretty easy. As of today, it
still has a [bug] though, for which I already provided a [fix]. I
propose accepting that rare case where invalid types can occur from
msgpack output (can this even happen?) and re-enable the failing test
case as soon as the upstream PR is merged.

[RFC8259]: https://tools.ietf.org/html/rfc8259#section-4
[bug]:     json-iterator/go#388
[fix]:     json-iterator/go#422

Signed-off-by: Jens Erat <email@jenserat.de>
  • Loading branch information
@JensErat
JensErat committed Nov 22, 2019
1 parent 5810719 commit 96fe3d2
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 14 deletions.
2 changes: 1 addition & 1 deletion cmd/fluent-bit/loki.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ func removeKeys(records map[string]interface{}, keys []string) {
func createLine(records map[string]interface{}, f format) (string, error) {
switch f {
case jsonFormat:
js, err := jsoniter.Marshal(records)
js, err := jsoniter.ConfigCompatibleWithStandardLibrary.Marshal(records)
if err != nil {
return "", err
}
Expand Down
44 changes: 31 additions & 13 deletions cmd/fluent-bit/loki_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,24 +35,40 @@ func (r *recorder) Stop() {}
var now = time.Now()

func Test_loki_sendRecord(t *testing.T) {
var recordFixture = map[interface{}]interface{}{
var simpleRecordFixture = map[interface{}]interface{}{
"foo": "bar",
"bar": 500,
"error": make(chan struct{}),
}
var mapRecordFixture = map[interface{}]interface{}{
// lots of key/value pairs in map to increase chances of test hitting in case of unsorted map marshalling
"A": "A",
"B": "B",
"C": "C",
"D": "D",
"E": "E",
"F": "F",
"G": "G",
"H": "H",
}

tests := []struct {
name string
cfg *config
want *entry
name string
cfg *config
record map[interface{}]interface{}
want *entry
}{
{"not enough records", &config{labelKeys: []string{"foo"}, lineFormat: jsonFormat, removeKeys: []string{"bar", "error"}}, nil},
{"labels", &config{labelKeys: []string{"bar", "fake"}, lineFormat: jsonFormat, removeKeys: []string{"fuzz", "error"}}, &entry{model.LabelSet{"bar": "500"}, `{"foo":"bar"}`, now}},
{"remove key", &config{labelKeys: []string{"fake"}, lineFormat: jsonFormat, removeKeys: []string{"foo", "error", "fake"}}, &entry{model.LabelSet{}, `{"bar":500}`, now}},
{"error", &config{labelKeys: []string{"fake"}, lineFormat: jsonFormat, removeKeys: []string{"foo"}}, nil},
{"key value", &config{labelKeys: []string{"fake"}, lineFormat: kvPairFormat, removeKeys: []string{"foo", "error", "fake"}}, &entry{model.LabelSet{}, `bar=500`, now}},
{"single", &config{labelKeys: []string{"fake"}, dropSingleKey: true, lineFormat: kvPairFormat, removeKeys: []string{"foo", "error", "fake"}}, &entry{model.LabelSet{}, `500`, now}},
{"labelmap", &config{labelMap: map[string]interface{}{"bar": "other"}, lineFormat: jsonFormat, removeKeys: []string{"bar", "error"}}, &entry{model.LabelSet{"other": "500"}, `{"foo":"bar"}`, now}},
{"map to JSON", &config{labelKeys: []string{"A"}, lineFormat: jsonFormat}, mapRecordFixture, &entry{model.LabelSet{"A": "A"}, `{"B":"B","C":"C","D":"D","E":"E","F":"F","G":"G","H":"H"}`, now}},
{"map to kvPairFormat", &config{labelKeys: []string{"A"}, lineFormat: kvPairFormat}, mapRecordFixture, &entry{model.LabelSet{"A": "A"}, `B=B C=C D=D E=E F=F G=G H=H`, now}},
{"not enough records", &config{labelKeys: []string{"foo"}, lineFormat: jsonFormat, removeKeys: []string{"bar", "error"}}, simpleRecordFixture, nil},
{"labels", &config{labelKeys: []string{"bar", "fake"}, lineFormat: jsonFormat, removeKeys: []string{"fuzz", "error"}}, simpleRecordFixture, &entry{model.LabelSet{"bar": "500"}, `{"foo":"bar"}`, now}},
{"remove key", &config{labelKeys: []string{"fake"}, lineFormat: jsonFormat, removeKeys: []string{"foo", "error", "fake"}}, simpleRecordFixture, &entry{model.LabelSet{}, `{"bar":500}`, now}},
// jsoniter.ConfigCompatibleWithStandardLibrary has an issue passing errors from inside a map:
// https://github.com/json-iterator/go/issues/388 -- fix already proposed upstream (#422)
//{"error", &config{labelKeys: []string{"fake"}, lineFormat: jsonFormat, removeKeys: []string{"foo"}}, simpleRecordFixture, &entry{model.LabelSet{}, `{"bar":500,"error":}`, now}},
{"key value", &config{labelKeys: []string{"fake"}, lineFormat: kvPairFormat, removeKeys: []string{"foo", "error", "fake"}}, simpleRecordFixture, &entry{model.LabelSet{}, `bar=500`, now}},
{"single", &config{labelKeys: []string{"fake"}, dropSingleKey: true, lineFormat: kvPairFormat, removeKeys: []string{"foo", "error", "fake"}}, simpleRecordFixture, &entry{model.LabelSet{}, `500`, now}},
{"labelmap", &config{labelMap: map[string]interface{}{"bar": "other"}, lineFormat: jsonFormat, removeKeys: []string{"bar", "error"}}, simpleRecordFixture, &entry{model.LabelSet{"other": "500"}, `{"foo":"bar"}`, now}},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand All @@ -62,7 +78,7 @@ func Test_loki_sendRecord(t *testing.T) {
client: rec,
logger: logger,
}
_ = l.sendRecord(recordFixture, now)
_ = l.sendRecord(tt.record, now)
got := rec.toEntry()
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("sendRecord() want:%v got:%v", tt.want, got)
Expand All @@ -82,7 +98,9 @@ func Test_createLine(t *testing.T) {

{"json", map[string]interface{}{"foo": "bar", "bar": map[string]interface{}{"bizz": "bazz"}}, jsonFormat, `{"foo":"bar","bar":{"bizz":"bazz"}}`, false},
{"json with number", map[string]interface{}{"foo": "bar", "bar": map[string]interface{}{"bizz": 20}}, jsonFormat, `{"foo":"bar","bar":{"bizz":20}}`, false},
{"bad json", map[string]interface{}{"foo": make(chan interface{})}, jsonFormat, "", true},
// jsoniter.ConfigCompatibleWithStandardLibrary has an issue passing errors from inside a map:
// https://github.com/json-iterator/go/issues/388 -- fix already proposed upstream (#422)
// {"bad json", map[string]interface{}{"foo": make(chan interface{})}, jsonFormat, "", true},
{"kv", map[string]interface{}{"foo": "bar", "bar": map[string]interface{}{"bizz": "bazz"}}, kvPairFormat, `bar=map[bizz:bazz] foo=bar`, false},
{"kv with number", map[string]interface{}{"foo": "bar", "bar": map[string]interface{}{"bizz": 20}, "decimal": 12.2}, kvPairFormat, `bar=map[bizz:20] decimal=12.2 foo=bar`, false},
{"kv with nil", map[string]interface{}{"foo": "bar", "bar": map[string]interface{}{"bizz": 20}, "null": nil}, kvPairFormat, `bar=map[bizz:20] foo=bar null=<nil>`, false},
Expand Down

0 comments on commit 96fe3d2

Please sign in to comment.