From 25baa7ee33eab0eb762f2b3597554c21f6a9b867 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20=C5=9Awi=C4=85tek?= Date: Wed, 11 Dec 2024 10:37:10 +0000 Subject: [PATCH] Cache conditions when applying variables to config (#6229) * Cache conditions when applying variables to config # Conflicts: # internal/pkg/agent/transpiler/ast_test.go * Add test for AST insertion (cherry picked from commit cab5754e4b8e6640d92cf9213142e4a5570ec968) --- ...1733411331-cache-conditions-in-config.yaml | 32 ++++++++++++++++ internal/pkg/agent/transpiler/ast.go | 20 +++++++--- internal/pkg/agent/transpiler/ast_test.go | 37 +++++++++++++++++++ 3 files changed, 83 insertions(+), 6 deletions(-) create mode 100644 changelog/fragments/1733411331-cache-conditions-in-config.yaml diff --git a/changelog/fragments/1733411331-cache-conditions-in-config.yaml b/changelog/fragments/1733411331-cache-conditions-in-config.yaml new file mode 100644 index 00000000000..57c424a38d2 --- /dev/null +++ b/changelog/fragments/1733411331-cache-conditions-in-config.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: Cache conditions when applying variables to config + +# 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 diff --git a/internal/pkg/agent/transpiler/ast.go b/internal/pkg/agent/transpiler/ast.go index 51fab56ee54..82987336235 100644 --- a/internal/pkg/agent/transpiler/ast.go +++ b/internal/pkg/agent/transpiler/ast.go @@ -238,13 +238,14 @@ func (d *Dict) sort() { // Key represents a Key / value pair in the dictionary. type Key struct { - name string - value Node + name string + value Node + condition *eql.Expression } // NewKey creates a new key with provided name node pair. func NewKey(name string, val Node) *Key { - return &Key{name, val} + return &Key{name: name, value: val} } func (k *Key) String() string { @@ -334,11 +335,18 @@ func (k *Key) Apply(vars *Vars) (Node, error) { case *BoolVal: return k, nil case *StrVal: - cond, err := eql.Eval(v.value, vars, true) + var err error + if k.condition == nil { + k.condition, err = eql.New(v.value) + if err != nil { + return nil, fmt.Errorf(`invalid condition "%s": %w`, v.value, err) + } + } + cond, err := k.condition.Eval(vars, true) if err != nil { return nil, fmt.Errorf(`condition "%s" evaluation failed: %w`, v.value, err) } - return &Key{k.name, NewBoolVal(cond)}, nil + return &Key{name: k.name, value: NewBoolVal(cond)}, nil } return nil, fmt.Errorf("condition key's value must be a string; received %T", k.value) } @@ -349,7 +357,7 @@ func (k *Key) Apply(vars *Vars) (Node, error) { if v == nil { return nil, nil } - return &Key{k.name, v}, nil + return &Key{name: k.name, value: v}, nil } // Processors returns any attached processors, because of variable substitution. diff --git a/internal/pkg/agent/transpiler/ast_test.go b/internal/pkg/agent/transpiler/ast_test.go index 41897e4aa16..948df76c51e 100644 --- a/internal/pkg/agent/transpiler/ast_test.go +++ b/internal/pkg/agent/transpiler/ast_test.go @@ -11,6 +11,8 @@ import ( "github.com/elastic/elastic-agent-libs/mapstr" + "github.com/elastic/elastic-agent/internal/pkg/eql" + "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -1108,6 +1110,41 @@ func TestLookup(t *testing.T) { } } +func TestCondition(t *testing.T) { + vars := mustMakeVars(map[string]interface{}{ + "other": map[string]interface{}{ + "data": "info", + }}) + + input := NewKey("condition", NewStrVal("${other.data} == 'info'")) + expected := NewKey("condition", NewBoolVal(true)) + + // the condition string hasn't been parsed yet + assert.Nil(t, input.condition) + + output, err := input.Apply(vars) + require.NoError(t, err) + assert.Equal(t, expected, output) + + // check if the condition was parsed and cached + assert.NotNil(t, input.condition) + condition, err := eql.New(input.value.Value().(string)) + require.NoError(t, err) + assert.Equal(t, condition, input.condition) + + // create a dict with the key + dict := NewDict([]Node{input}) + ast := &AST{root: NewKey("key", dict)} + // the cached condition remains + assert.Equal(t, condition, input.condition) + + // replace the key with a new one, without a cached condition + input2 := NewKey("condition", NewStrVal("${other.data} == 'info'")) + err = Insert(ast, input2, "") + require.NoError(t, err) + assert.Nil(t, input2.condition) +} + func mustMakeVars(mapping map[string]interface{}) *Vars { v, err := NewVars("", mapping, nil) if err != nil {