Skip to content

Commit cab5754

Browse files
authored
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
1 parent a2d5f75 commit cab5754

File tree

3 files changed

+83
-6
lines changed

3 files changed

+83
-6
lines changed
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
# Kind can be one of:
2+
# - breaking-change: a change to previously-documented behavior
3+
# - deprecation: functionality that is being removed in a later release
4+
# - bug-fix: fixes a problem in a previous version
5+
# - enhancement: extends functionality but does not break or fix existing behavior
6+
# - feature: new functionality
7+
# - known-issue: problems that we are aware of in a given version
8+
# - security: impacts on the security of a product or a user’s deployment.
9+
# - upgrade: important information for someone upgrading from a prior version
10+
# - other: does not fit into any of the other categories
11+
kind: enhancement
12+
13+
# Change summary; a 80ish characters long description of the change.
14+
summary: Cache conditions when applying variables to config
15+
16+
# Long description; in case the summary is not enough to describe the change
17+
# this field accommodate a description without length limits.
18+
# NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment.
19+
#description:
20+
21+
# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc.
22+
component: elastic-agent
23+
24+
# PR URL; optional; the PR number that added the changeset.
25+
# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added.
26+
# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number.
27+
# Please provide it if you are adding a fragment for a different PR.
28+
#pr: https://github.com/owner/repo/1234
29+
30+
# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of).
31+
# If not present is automatically filled by the tooling with the issue linked to the PR number.
32+
#issue: https://github.com/owner/repo/1234

internal/pkg/agent/transpiler/ast.go

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -238,13 +238,14 @@ func (d *Dict) sort() {
238238

239239
// Key represents a Key / value pair in the dictionary.
240240
type Key struct {
241-
name string
242-
value Node
241+
name string
242+
value Node
243+
condition *eql.Expression
243244
}
244245

245246
// NewKey creates a new key with provided name node pair.
246247
func NewKey(name string, val Node) *Key {
247-
return &Key{name, val}
248+
return &Key{name: name, value: val}
248249
}
249250

250251
func (k *Key) String() string {
@@ -334,11 +335,18 @@ func (k *Key) Apply(vars *Vars) (Node, error) {
334335
case *BoolVal:
335336
return k, nil
336337
case *StrVal:
337-
cond, err := eql.Eval(v.value, vars, true)
338+
var err error
339+
if k.condition == nil {
340+
k.condition, err = eql.New(v.value)
341+
if err != nil {
342+
return nil, fmt.Errorf(`invalid condition "%s": %w`, v.value, err)
343+
}
344+
}
345+
cond, err := k.condition.Eval(vars, true)
338346
if err != nil {
339347
return nil, fmt.Errorf(`condition "%s" evaluation failed: %w`, v.value, err)
340348
}
341-
return &Key{k.name, NewBoolVal(cond)}, nil
349+
return &Key{name: k.name, value: NewBoolVal(cond)}, nil
342350
}
343351
return nil, fmt.Errorf("condition key's value must be a string; received %T", k.value)
344352
}
@@ -349,7 +357,7 @@ func (k *Key) Apply(vars *Vars) (Node, error) {
349357
if v == nil {
350358
return nil, nil
351359
}
352-
return &Key{k.name, v}, nil
360+
return &Key{name: k.name, value: v}, nil
353361
}
354362

355363
// Processors returns any attached processors, because of variable substitution.

internal/pkg/agent/transpiler/ast_test.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import (
1111

1212
"github.com/elastic/elastic-agent-libs/mapstr"
1313

14+
"github.com/elastic/elastic-agent/internal/pkg/eql"
15+
1416
"github.com/google/go-cmp/cmp"
1517
"github.com/stretchr/testify/assert"
1618
"github.com/stretchr/testify/require"
@@ -1108,6 +1110,41 @@ func TestLookup(t *testing.T) {
11081110
}
11091111
}
11101112

1113+
func TestCondition(t *testing.T) {
1114+
vars := mustMakeVars(map[string]interface{}{
1115+
"other": map[string]interface{}{
1116+
"data": "info",
1117+
}})
1118+
1119+
input := NewKey("condition", NewStrVal("${other.data} == 'info'"))
1120+
expected := NewKey("condition", NewBoolVal(true))
1121+
1122+
// the condition string hasn't been parsed yet
1123+
assert.Nil(t, input.condition)
1124+
1125+
output, err := input.Apply(vars)
1126+
require.NoError(t, err)
1127+
assert.Equal(t, expected, output)
1128+
1129+
// check if the condition was parsed and cached
1130+
assert.NotNil(t, input.condition)
1131+
condition, err := eql.New(input.value.Value().(string))
1132+
require.NoError(t, err)
1133+
assert.Equal(t, condition, input.condition)
1134+
1135+
// create a dict with the key
1136+
dict := NewDict([]Node{input})
1137+
ast := &AST{root: NewKey("key", dict)}
1138+
// the cached condition remains
1139+
assert.Equal(t, condition, input.condition)
1140+
1141+
// replace the key with a new one, without a cached condition
1142+
input2 := NewKey("condition", NewStrVal("${other.data} == 'info'"))
1143+
err = Insert(ast, input2, "")
1144+
require.NoError(t, err)
1145+
assert.Nil(t, input2.condition)
1146+
}
1147+
11111148
func mustMakeVars(mapping map[string]interface{}) *Vars {
11121149
v, err := NewVars("", mapping, nil)
11131150
if err != nil {

0 commit comments

Comments
 (0)