Avoid unnecessary copies during config generation (#6184)

* Fix a bug in AST dictionary shallow cloning

* Avoid unnecessary deepcopy during config generation

* Don't unnecessarily copy AST when rendering inputs

# Conflicts:
#	internal/pkg/agent/transpiler/ast.go

(cherry picked from commit 398f322)

# Conflicts:
#	internal/pkg/agent/transpiler/ast.go

changelog/fragments/1733158776-avoid-copies-config-gen.yaml

@@ -0,0 +1,32 @@
+# Kind can be one of:
+# - breaking-change: a change to previously-documented behavior
+# - deprecation: functionality that is being removed in a later release
+# - bug-fix: fixes a problem in a previous version
+# - enhancement: extends functionality but does not break or fix existing behavior
+# - feature: new functionality
+# - known-issue: problems that we are aware of in a given version
+# - security: impacts on the security of a product or a user’s deployment.
+# - upgrade: important information for someone upgrading from a prior version
+# - other: does not fit into any of the other categories
+kind: enhancement
+
+# Change summary; a 80ish characters long description of the change.
+summary: Avoid unnecessary copies when generating component configurations
+
+# Long description; in case the summary is not enough to describe the change
+# this field accommodate a description without length limits.
+# NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment.
+#description:
+
+# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc.
+component: elastic-agent
+
+# PR URL; optional; the PR number that added the changeset.
+# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added.
+# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number.
+# Please provide it if you are adding a fragment for a different PR.
+#pr: https://github.com/owner/repo/1234
+
+# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of).
+# If not present is automatically filled by the tooling with the issue linked to the PR number.
+#issue: https://github.com/owner/repo/1234

internal/pkg/agent/application/coordinator/coordinator.go

@@ -1282,7 +1282,7 @@ func (c *Coordinator) generateComponentModel() (err error) {
 		c.setComponentGenError(err)
 	}()
 
-	ast := c.ast.Clone()
+	ast := c.ast.ShallowClone()
 	inputs, ok := transpiler.Lookup(ast, "inputs")
 	if ok {
 		renderedInputs, err := transpiler.RenderInputs(inputs, c.vars)

internal/pkg/agent/transpiler/ast.go

@@ -58,7 +58,7 @@ type Node interface {
 	// Hash compute a sha256 hash of the current node and recursively call any children.
 	Hash() []byte
 
-	// Apply apply the current vars, returning the new value for the node.
+	// Apply apply the current vars, returning the new value for the node. This does not modify the original Node.
 	Apply(*Vars) (Node, error)
 
 	// Processors returns any attached processors, because of variable substitution.
@@ -147,7 +147,8 @@ func (d *Dict) ShallowClone() Node {
 		if i == nil {
 			continue
 		}
-		nodes = append(nodes, i)
+		// Dict nodes are key-value pairs, and we do want to make a copy of the key here
+		nodes = append(nodes, i.ShallowClone())
 
 	}
 	return &Dict{value: nodes}
@@ -162,7 +163,7 @@ func (d *Dict) Hash() []byte {
 	return h.Sum(nil)
 }
 
-// Apply applies the vars to all the nodes in the dictionary.
+// Apply applies the vars to all the nodes in the dictionary. This does not modify the original dictionary.
 func (d *Dict) Apply(vars *Vars) (Node, error) {
 	nodes := make([]Node, 0, len(d.value))
 	for _, v := range d.value {
@@ -277,7 +278,7 @@ func (k *Key) Hash() []byte {
 	return h.Sum(nil)
 }
 
-// Apply applies the vars to the value.
+// Apply applies the vars to the value. This does not modify the original node.
 func (k *Key) Apply(vars *Vars) (Node, error) {
 	if k.value == nil {
 		return k, nil
@@ -397,7 +398,7 @@ func (l *List) ShallowClone() Node {
 	return &List{value: nodes}
 }
 
-// Apply applies the vars to all nodes in the list.
+// Apply applies the vars to all nodes in the list. This does not modify the original list.
 func (l *List) Apply(vars *Vars) (Node, error) {
 	nodes := make([]Node, 0, len(l.value))
 	for _, v := range l.value {
@@ -472,7 +473,7 @@ func (s *StrVal) Hash() []byte {
 	return []byte(s.value)
 }
 
-// Apply applies the vars to the string value.
+// Apply applies the vars to the string value. This does not modify the original string.
 func (s *StrVal) Apply(vars *Vars) (Node, error) {
 	return vars.Replace(s.value)
 }

internal/pkg/agent/transpiler/ast_test.go

@@ -9,6 +9,8 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/elastic/elastic-agent-libs/mapstr"
+
 	"github.com/google/go-cmp/cmp"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -852,6 +854,41 @@ func TestHash(t *testing.T) {
 	}
 }
 
+func TestApplyDoesNotMutate(t *testing.T) {
+	tests := map[string]struct {
+		input Node
+	}{
+		"dict": {
+			&Dict{
+				value: []Node{
+					&Key{name: "str", value: &StrVal{value: "${var}"}},
+				},
+			},
+		},
+		"list": {
+			&List{
+				value: []Node{
+					&StrVal{value: "${var}"},
+				},
+			},
+		},
+		"key": {
+			&Key{name: "str", value: &StrVal{value: "${var}"}},
+		},
+		"str": {&StrVal{value: "${var}"}},
+	}
+
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			vars, err := NewVars("", map[string]any{"var": "value"}, mapstr.M{})
+			require.NoError(t, err)
+			applied, err := test.input.Apply(vars)
+			require.NoError(t, err)
+			assert.NotEqual(t, test.input, applied)
+		})
+	}
+}
+
 func TestShallowClone(t *testing.T) {
 	tests := map[string]struct {
 		input *AST
@@ -909,7 +946,7 @@ func TestShallowClone(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			cloned := test.input.ShallowClone()
 			assert.Equal(t, test.input, cloned)
-			err := test.input.Insert(&AST{root: &BoolVal{value: true}}, "key")
+			err := test.input.Insert(&AST{root: &Key{name: "integer", value: &IntVal{value: 7}}}, "integer")
 			if err == nil {
 				assert.NotEqual(t, test.input, cloned)
 			} else if list, ok := test.input.root.(*List); ok {

internal/pkg/agent/transpiler/utils.go

@@ -26,14 +26,15 @@ func RenderInputs(inputs Node, varsArray []*Vars) (Node, error) {
 	nodesMap := map[string]*Dict{}
 	for _, vars := range varsArray {
 		for _, node := range l.Value().([]Node) {
-			dict, ok := node.Clone().(*Dict)
+			dict, ok := node.(*Dict)
 			if !ok {
 				continue
 			}
 			hadStreams := false
 			if streams := getStreams(dict); streams != nil {
 				hadStreams = true
 			}
+			// Apply creates a new Node with a deep copy of all the values
 			n, err := dict.Apply(vars)
 			if errors.Is(err, ErrNoMatch) {
 				// has a variable that didn't exist, so we ignore it