diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 25a6b617a63..5246bcc5ae7 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -73,6 +73,7 @@ https://github.com/elastic/beats/compare/v8.8.1\...main[Check the HEAD diff] - Do not print context cancelled error message when running under agent {pull}36006[36006] - Fix recovering from invalid output configuration when running under Elastic-Agent {pull}36016[36016] - Improve StreamBuf append to improve performance when reading long lines from files. {pull}35928[35928] +- Eliminate cloning of event in deepUpdate {pull}35945[35945] *Auditbeat* diff --git a/libbeat/beat/event.go b/libbeat/beat/event.go index 94aa16bb63f..54fc3e27cc3 100644 --- a/libbeat/beat/event.go +++ b/libbeat/beat/event.go @@ -110,45 +110,62 @@ func (e *Event) deepUpdate(d mapstr.M, overwrite bool) { if len(d) == 0 { return } - fieldsUpdate := d.Clone() // so we can delete redundant keys - var metaUpdate mapstr.M + // It's supported to update the timestamp using this function. + // However, we must handle it separately since it's a separate field of the event. + timestampValue, timestampExists := d[timestampFieldKey] + if timestampExists { + if overwrite { + _ = e.setTimestamp(timestampValue) + } - for fieldKey, value := range d { - switch fieldKey { + // Temporary delete it from the update map, + // so we can do `e.Fields.DeepUpdate(d)` or + // `e.Fields.DeepUpdateNoOverwrite(d)` later + delete(d, timestampFieldKey) + } - // one of the updates is the timestamp which is not a part of the event fields - case timestampFieldKey: - if overwrite { - _ = e.setTimestamp(value) - } - delete(fieldsUpdate, fieldKey) + // It's supported to update the metadata using this function. + // However, we must handle it separately since it's a separate field of the event. + metaValue, metaExists := d[metadataFieldKey] + if metaExists { + var metaUpdate mapstr.M - // some updates are addressed for the metadata not the fields - case metadataFieldKey: - switch meta := value.(type) { - case mapstr.M: - metaUpdate = meta - case map[string]interface{}: - metaUpdate = mapstr.M(meta) - } + switch meta := metaValue.(type) { + case mapstr.M: + metaUpdate = meta + case map[string]interface{}: + metaUpdate = mapstr.M(meta) + } - delete(fieldsUpdate, fieldKey) + if metaUpdate != nil { + if e.Meta == nil { + e.Meta = mapstr.M{} + } + if overwrite { + e.Meta.DeepUpdate(metaUpdate) + } else { + e.Meta.DeepUpdateNoOverwrite(metaUpdate) + } } + + // Temporary delete it from the update map, + // so we can do `e.Fields.DeepUpdate(d)` or + // `e.Fields.DeepUpdateNoOverwrite(d)` later + delete(d, metadataFieldKey) } - if metaUpdate != nil { - if e.Meta == nil { - e.Meta = mapstr.M{} + // At the end we revert all changes we made to the update map + defer func() { + if timestampExists { + d[timestampFieldKey] = timestampValue } - if overwrite { - e.Meta.DeepUpdate(metaUpdate) - } else { - e.Meta.DeepUpdateNoOverwrite(metaUpdate) + if metaExists { + d[metadataFieldKey] = metaValue } - } + }() - if len(fieldsUpdate) == 0 { + if len(d) == 0 { return } @@ -157,9 +174,9 @@ func (e *Event) deepUpdate(d mapstr.M, overwrite bool) { } if overwrite { - e.Fields.DeepUpdate(fieldsUpdate) + e.Fields.DeepUpdate(d) } else { - e.Fields.DeepUpdateNoOverwrite(fieldsUpdate) + e.Fields.DeepUpdateNoOverwrite(d) } } diff --git a/libbeat/beat/event_test.go b/libbeat/beat/event_test.go index 5575f495106..cd165a3c459 100644 --- a/libbeat/beat/event_test.go +++ b/libbeat/beat/event_test.go @@ -18,6 +18,7 @@ package beat import ( + "crypto/rand" "testing" "time" @@ -26,11 +27,59 @@ import ( "github.com/elastic/elastic-agent-libs/mapstr" ) +const ( + propSize = 1024 * 2014 * 10 +) + +var largeProp string + +func init() { + b := make([]byte, propSize) + _, _ = rand.Read(b) + largeProp = string(b) +} + func newEmptyEvent() *Event { return &Event{Fields: mapstr.M{}} } -func TestEventPutGetTimestamp(t *testing.T) { +func newEvent(e mapstr.M) *Event { + n := &mapstr.M{ + "Fields": mapstr.M{ + "large_prop": largeProp, + }, + } + n.DeepUpdate(e) + var ts time.Time + var meta mapstr.M + var fields mapstr.M + var private mapstr.M + + v, ex := (*n)["Timestamp"] + if ex { + ts = v.(time.Time) + } + v, ex = (*n)["Meta"] + if ex { + meta = v.(mapstr.M) + } + v, ex = (*n)["Fields"] + if ex { + fields = v.(mapstr.M) + } + v, ex = (*n)["Private"] + if ex { + private = v.(mapstr.M) + } + return &Event{ + Timestamp: ts, + Meta: meta, + Fields: fields, + Private: private, + } +} + +func BenchmarkTestEventPutGetTimestamp(b *testing.B) { evt := newEmptyEvent() ts := time.Now() @@ -38,17 +87,17 @@ func TestEventPutGetTimestamp(t *testing.T) { v, err := evt.GetValue("@timestamp") if err != nil { - t.Fatal(err) + b.Fatal(err) } - assert.Equal(t, ts, v) - assert.Equal(t, ts, evt.Timestamp) + assert.Equal(b, ts, v) + assert.Equal(b, ts, evt.Timestamp) // The @timestamp is not written into Fields. - assert.Nil(t, evt.Fields["@timestamp"]) + assert.Nil(b, evt.Fields["@timestamp"]) } -func TestDeepUpdate(t *testing.T) { +func BenchmarkTestDeepUpdate(b *testing.B) { ts := time.Now() cases := []struct { @@ -60,37 +109,43 @@ func TestDeepUpdate(t *testing.T) { }{ { name: "does nothing if no update", - event: &Event{}, + event: newEvent(mapstr.M{}), update: mapstr.M{}, - expected: &Event{}, + expected: newEvent(mapstr.M{}), }, { name: "updates timestamp", - event: &Event{}, + event: newEvent(mapstr.M{}), update: mapstr.M{ timestampFieldKey: ts, }, overwrite: true, expected: &Event{ Timestamp: ts, + Fields: mapstr.M{ + "large_prop": largeProp, + }, }, }, { name: "does not overwrite timestamp", - event: &Event{ - Timestamp: ts, - }, + event: newEvent(mapstr.M{ + "Timestamp": ts, + }), update: mapstr.M{ timestampFieldKey: time.Now().Add(time.Hour), }, overwrite: false, expected: &Event{ Timestamp: ts, + Fields: mapstr.M{ + "large_prop": largeProp, + }, }, }, { name: "initializes metadata if nil", - event: &Event{}, + event: newEvent(mapstr.M{}), update: mapstr.M{ metadataFieldKey: mapstr.M{ "first": "new", @@ -102,15 +157,18 @@ func TestDeepUpdate(t *testing.T) { "first": "new", "second": 42, }, + Fields: mapstr.M{ + "large_prop": largeProp, + }, }, }, { name: "updates metadata but does not overwrite", - event: &Event{ - Meta: mapstr.M{ + event: newEvent(mapstr.M{ + "Meta": mapstr.M{ "first": "initial", }, - }, + }), update: mapstr.M{ metadataFieldKey: mapstr.M{ "first": "new", @@ -123,15 +181,18 @@ func TestDeepUpdate(t *testing.T) { "first": "initial", "second": 42, }, + Fields: mapstr.M{ + "large_prop": largeProp, + }, }, }, { name: "updates metadata and overwrites", - event: &Event{ - Meta: mapstr.M{ + event: newEvent(mapstr.M{ + "Meta": mapstr.M{ "first": "initial", }, - }, + }), update: mapstr.M{ metadataFieldKey: mapstr.M{ "first": "new", @@ -144,15 +205,18 @@ func TestDeepUpdate(t *testing.T) { "first": "new", "second": 42, }, + Fields: mapstr.M{ + "large_prop": largeProp, + }, }, }, { name: "updates fields but does not overwrite", - event: &Event{ - Fields: mapstr.M{ + event: newEvent(mapstr.M{ + "Fields": mapstr.M{ "first": "initial", }, - }, + }), update: mapstr.M{ "first": "new", "second": 42, @@ -160,18 +224,19 @@ func TestDeepUpdate(t *testing.T) { overwrite: false, expected: &Event{ Fields: mapstr.M{ - "first": "initial", - "second": 42, + "first": "initial", + "second": 42, + "large_prop": largeProp, }, }, }, { name: "updates metadata and overwrites", - event: &Event{ - Fields: mapstr.M{ + event: newEvent(mapstr.M{ + "Fields": mapstr.M{ "first": "initial", }, - }, + }), update: mapstr.M{ "first": "new", "second": 42, @@ -179,123 +244,125 @@ func TestDeepUpdate(t *testing.T) { overwrite: true, expected: &Event{ Fields: mapstr.M{ - "first": "new", - "second": 42, + "first": "new", + "second": 42, + "large_prop": largeProp, }, }, }, { name: "initializes fields if nil", - event: &Event{}, + event: newEvent(mapstr.M{}), update: mapstr.M{ "first": "new", "second": 42, }, expected: &Event{ Fields: mapstr.M{ - "first": "new", - "second": 42, + "first": "new", + "second": 42, + "large_prop": largeProp, }, }, }, } for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { + b.Run(tc.name, func(b *testing.B) { tc.event.deepUpdate(tc.update, tc.overwrite) - assert.Equal(t, tc.expected.Timestamp, tc.event.Timestamp) - assert.Equal(t, tc.expected.Fields, tc.event.Fields) - assert.Equal(t, tc.expected.Meta, tc.event.Meta) + assert.Equal(b, tc.expected.Timestamp, tc.event.Timestamp) + assert.Equal(b, tc.expected.Fields, tc.event.Fields) + assert.Equal(b, tc.expected.Meta, tc.event.Meta) }) } } -func TestEventMetadata(t *testing.T) { +func BenchmarkTestEventMetadata(b *testing.B) { const id = "123" newMeta := func() mapstr.M { return mapstr.M{"_id": id} } - t.Run("put", func(t *testing.T) { + b.Run("put", func(b *testing.B) { evt := newEmptyEvent() meta := newMeta() evt.PutValue("@metadata", meta) - assert.Equal(t, meta, evt.Meta) - assert.Empty(t, evt.Fields) + assert.Equal(b, meta, evt.Meta) + assert.Empty(b, evt.Fields) }) - t.Run("get", func(t *testing.T) { + b.Run("get", func(b *testing.B) { evt := newEmptyEvent() evt.Meta = newMeta() meta, err := evt.GetValue("@metadata") - assert.NoError(t, err) - assert.Equal(t, evt.Meta, meta) + assert.NoError(b, err) + assert.Equal(b, evt.Meta, meta) }) - t.Run("put sub-key", func(t *testing.T) { + b.Run("put sub-key", func(b *testing.B) { evt := newEmptyEvent() evt.PutValue("@metadata._id", id) - assert.Equal(t, newMeta(), evt.Meta) - assert.Empty(t, evt.Fields) + assert.Equal(b, newMeta(), evt.Meta) + assert.Empty(b, evt.Fields) }) - t.Run("get sub-key", func(t *testing.T) { + b.Run("get sub-key", func(b *testing.B) { evt := newEmptyEvent() evt.Meta = newMeta() v, err := evt.GetValue("@metadata._id") - assert.NoError(t, err) - assert.Equal(t, id, v) + assert.NoError(b, err) + assert.Equal(b, id, v) }) - t.Run("delete", func(t *testing.T) { + b.Run("delete", func(b *testing.B) { evt := newEmptyEvent() evt.Meta = newMeta() err := evt.Delete("@metadata") - assert.NoError(t, err) - assert.Nil(t, evt.Meta) + assert.NoError(b, err) + assert.Nil(b, evt.Meta) }) - t.Run("delete sub-key", func(t *testing.T) { + b.Run("delete sub-key", func(b *testing.B) { evt := newEmptyEvent() evt.Meta = newMeta() err := evt.Delete("@metadata._id") - assert.NoError(t, err) - assert.Empty(t, evt.Meta) + assert.NoError(b, err) + assert.Empty(b, evt.Meta) }) - t.Run("setID", func(t *testing.T) { + b.Run("setID", func(b *testing.B) { evt := newEmptyEvent() evt.SetID(id) - assert.Equal(t, newMeta(), evt.Meta) + assert.Equal(b, newMeta(), evt.Meta) }) - t.Run("put non-metadata", func(t *testing.T) { + b.Run("put non-metadata", func(b *testing.B) { evt := newEmptyEvent() evt.PutValue("@metadataSpecial", id) - assert.Equal(t, mapstr.M{"@metadataSpecial": id}, evt.Fields) + assert.Equal(b, mapstr.M{"@metadataSpecial": id}, evt.Fields) }) - t.Run("delete non-metadata", func(t *testing.T) { + b.Run("delete non-metadata", func(b *testing.B) { evt := newEmptyEvent() evt.Meta = newMeta() err := evt.Delete("@metadataSpecial") - assert.Error(t, err) - assert.Equal(t, newMeta(), evt.Meta) + assert.Error(b, err) + assert.Equal(b, newMeta(), evt.Meta) }) }