[7.17](backport #35945) Decrease Clones (#36593)

* Decrease Clones (#35945) * Decrease Clones * Adding changelog * CodeReview changes (cherry picked from commit faf88b7) # Conflicts: # libbeat/beat/event.go # libbeat/beat/event_test.go --------- Co-authored-by: Evgeniy Belyi <[email protected]> Co-authored-by: Denis Rechkunov <[email protected]>
elastic · Sep 18, 2023 · 318fdba · 318fdba
1 parent f68f110
commit 318fdba
Show file tree

Hide file tree

Showing 3 changed files with 181 additions and 103 deletions.
diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc
@@ -30,7 +30,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
 
 *Affecting all Beats*
 
-*Filebeat*
+- Eliminate cloning of event in deepUpdate {pull}35945[35945]
 
 *Auditbeat*
 
@@ -141,9 +141,3 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
 *Functionbeat*
 
 ==== Known Issue
-
-
-
-
-
-
diff --git a/libbeat/beat/event.go b/libbeat/beat/event.go
@@ -98,45 +98,62 @@ func (e *Event) deepUpdate(d common.MapStr, overwrite bool) {
 	if len(d) == 0 {
 		return
 	}
-	fieldsUpdate := d.Clone() // so we can delete redundant keys
 
-	var metaUpdate common.MapStr
+	// 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 common.MapStr
+
+		switch meta := metaValue.(type) {
+		case common.MapStr:
+			metaUpdate = meta
+		case map[string]interface{}:
+			metaUpdate = common.MapStr(meta)
+		}
 
-		// some updates are addressed for the metadata not the fields
-		case metadataFieldKey:
-			switch meta := value.(type) {
-			case common.MapStr:
-				metaUpdate = meta
-			case map[string]interface{}:
-				metaUpdate = common.MapStr(meta)
+		if metaUpdate != nil {
+			if e.Meta == nil {
+				e.Meta = common.MapStr{}
+			}
+			if overwrite {
+				e.Meta.DeepUpdate(metaUpdate)
+			} else {
+				e.Meta.DeepUpdateNoOverwrite(metaUpdate)
 			}
-
-			delete(fieldsUpdate, fieldKey)
 		}
+
+		// 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 = common.MapStr{}
+	// 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
 	}
 
@@ -145,9 +162,9 @@ func (e *Event) deepUpdate(d common.MapStr, overwrite bool) {
 	}
 
 	if overwrite {
-		e.Fields.DeepUpdate(fieldsUpdate)
+		e.Fields.DeepUpdate(d)
 	} else {
-		e.Fields.DeepUpdateNoOverwrite(fieldsUpdate)
+		e.Fields.DeepUpdateNoOverwrite(d)
 	}
 }