Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 94 additions & 0 deletions bench/template_cel_env_bench_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
package bench_test

import (
"fmt"
"testing"
"time"

dutyctx "github.com/flanksource/duty/context"
"github.com/flanksource/gomplate/v3"
"github.com/google/cel-go/cel"
celtypes "github.com/google/cel-go/common/types"
"github.com/google/cel-go/common/types/ref"
)

// BenchmarkRunTemplateCELCacheHitRegisteredEnvFuncs isolates the duty-side
// overhead of constructing registered CEL env functions on every RunTemplate
// call. The expression does not call any registered function; the benchmark only
// varies how many functions are registered globally.
func BenchmarkRunTemplateCELCacheHitRegisteredEnvFuncs(b *testing.B) {
env := map[string]any{
"id": "0192f0a4-1234-7000-8000-aaaaaaaaaaaa",
"namespace": "default",
"name": "nginx-7c5ddbdf54-abcde",
"index": 42,
"config_type": "Kubernetes::Pod",
}

for _, registeredFuncs := range []int{0, 18, 64} {
b.Run(fmt.Sprintf("registered=%02d", registeredFuncs), func(b *testing.B) {
installBenchmarkCelEnvFuncs(b, registeredFuncs)

ctx := dutyctx.New()
tmpl := gomplate.Template{
Expression: `config_type == "Kubernetes::Pod"`,
CacheKey: fmt.Sprintf("benchmark.run-template.cel-env-funcs.%d", registeredFuncs),
CacheTime: time.Hour,
}

// Warm gomplate's compiled CEL program cache. Any measured allocation after
// this point should be steady-state RunTemplate overhead, not CEL compile.
if _, err := ctx.RunTemplateBool(tmpl, env); err != nil {
b.Fatal(err)
}

b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
ok, err := ctx.RunTemplateBool(tmpl, env)
if err != nil {
b.Fatal(err)
}
if !ok {
b.Fatal("expected expression to evaluate true")
}
Comment on lines +41 to +54

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Locate the benchmark test file and check its structure
find . -name "template_cel_env_bench_test.go" -o -name "*_test.go" -path "*/bench/*" 2>/dev/null

Repository: flanksource/duty

Length of output: 323


🏁 Script executed:

# Read the benchmark test file
cat -n bench/template_cel_env_bench_test.go

Repository: flanksource/duty

Length of output: 3651


🏁 Script executed:

# Check the imports and lines around 41-54 in detail
sed -n '1,80p' bench/template_cel_env_bench_test.go | cat -n

Repository: flanksource/duty

Length of output: 3149


Use gomega assertions in this _test.go benchmark.

This benchmark uses assertion-style b.Fatal checks instead of gomega expectations. Please switch to gomega.NewWithT(b) and Expect(...) for these checks.

Proposed patch
 import (
 	"fmt"
 	"testing"
 	"time"
 
 	dutyctx "github.com/flanksource/duty/context"
 	"github.com/flanksource/gomplate/v3"
 	"github.com/google/cel-go/cel"
 	celtypes "github.com/google/cel-go/common/types"
 	"github.com/google/cel-go/common/types/ref"
+	. "github.com/onsi/gomega"
 )
@@
 	for _, registeredFuncs := range []int{0, 18, 64} {
 		b.Run(fmt.Sprintf("registered=%02d", registeredFuncs), func(b *testing.B) {
+			g := NewWithT(b)
 			installBenchmarkCelEnvFuncs(b, registeredFuncs)
@@
-			if _, err := ctx.RunTemplateBool(tmpl, env); err != nil {
-				b.Fatal(err)
-			}
+			_, err := ctx.RunTemplateBool(tmpl, env)
+			g.Expect(err).NotTo(HaveOccurred())
@@
 			b.ReportAllocs()
 			b.ResetTimer()
 			for i := 0; i < b.N; i++ {
-				ok, err := ctx.RunTemplateBool(tmpl, env)
-				if err != nil {
-					b.Fatal(err)
-				}
-				if !ok {
-					b.Fatal("expected expression to evaluate true")
-				}
+				ok, err := ctx.RunTemplateBool(tmpl, env)
+				g.Expect(err).NotTo(HaveOccurred())
+				g.Expect(ok).To(BeTrue())
 			}
 		})
 	}
 }

Per coding guidelines, **/*_test.go: "Always use github.com/onsi/gomega package for assertions in test files" and "use the gomega.NewWithT(t) approach for assertions."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if _, err := ctx.RunTemplateBool(tmpl, env); err != nil {
b.Fatal(err)
}
b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
ok, err := ctx.RunTemplateBool(tmpl, env)
if err != nil {
b.Fatal(err)
}
if !ok {
b.Fatal("expected expression to evaluate true")
}
_, err := ctx.RunTemplateBool(tmpl, env)
g.Expect(err).NotTo(HaveOccurred())
b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
ok, err := ctx.RunTemplateBool(tmpl, env)
g.Expect(err).NotTo(HaveOccurred())
g.Expect(ok).To(BeTrue())
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bench/template_cel_env_bench_test.go` around lines 41 - 54, The benchmark
test uses b.Fatal for assertions instead of gomega expectations, which violates
the coding guidelines. Replace the b.Fatal error checks and the b.Fatal boolean
check in the benchmark loop with gomega assertions by first creating a gomega
instance with gomega.NewWithT(b), then use Expect to verify the error is nil and
the result ok is true. Specifically, replace the two b.Fatal calls related to
the ctx.RunTemplateBool function calls with gomega Expect assertions that check
for nil errors and that ok evaluates to true.

Source: Coding guidelines

}
})
}
}

func installBenchmarkCelEnvFuncs(b *testing.B, count int) {
b.Helper()

oldCelEnvFuncs := dutyctx.CelEnvFuncs
oldTemplateFuncs := dutyctx.TemplateFuncs

dutyctx.CelEnvFuncs = make(map[string]func(dutyctx.Context) cel.EnvOption, count)
for i := 0; i < count; i++ {
name := fmt.Sprintf("bench.func_%02d", i)
dutyctx.CelEnvFuncs[name] = benchmarkCelEnvFunc(name, fmt.Sprintf("bench_func_%02d_string", i))
}
dutyctx.TemplateFuncs = nil

b.Cleanup(func() {
dutyctx.CelEnvFuncs = oldCelEnvFuncs
dutyctx.TemplateFuncs = oldTemplateFuncs
})
}

func benchmarkCelEnvFunc(name, overloadID string) func(dutyctx.Context) cel.EnvOption {
return func(ctx dutyctx.Context) cel.EnvOption {
return cel.Function(name,
cel.Overload(overloadID,
[]*cel.Type{cel.StringType},
cel.StringType,
cel.UnaryBinding(func(arg ref.Val) ref.Val {
// Capture ctx like production DB/catalog functions do. This binding is
// intentionally never called by the benchmark expression.
_ = ctx
return celtypes.String(fmt.Sprintf("%s:%v", name, arg.Value()))
}),
),
)
}
}
60 changes: 57 additions & 3 deletions context/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package context
import (
"fmt"
"strconv"
"strings"

"github.com/flanksource/commons/collections"
"github.com/flanksource/commons/logger"
Expand All @@ -23,9 +24,7 @@ func (k Context) RunTemplate(t gomplate.Template, env map[string]any) (string, e
} else {
l.V(1).Infof("Running template: %s", t.String())
}
for _, f := range CelEnvFuncs {
t.CelEnvs = append(t.CelEnvs, f(k))
}
appendReferencedCelEnvFuncs(k, &t)
if t.Functions == nil {
t.Functions = make(map[string]any)
}
Expand Down Expand Up @@ -78,6 +77,61 @@ func (k Context) RunTemplate(t gomplate.Template, env map[string]any) (string, e
return val, nil
}

func appendReferencedCelEnvFuncs(ctx Context, t *gomplate.Template) {
if t.Expression == "" || !strings.Contains(t.Expression, "(") {
return
}

for name, f := range CelEnvFuncs {
if celExpressionCalls(t.Expression, name) || (strings.HasSuffix(name, "Cel") && celExpressionCalls(t.Expression, strings.TrimSuffix(name, "Cel"))) {
t.CelEnvs = append(t.CelEnvs, f(ctx))
}
}
}

func celExpressionCalls(expr, name string) bool {
if name == "" {
return false
}

for offset := 0; offset < len(expr); {
idx := strings.Index(expr[offset:], name)
if idx < 0 {
return false
}
idx += offset
afterName := idx + len(name)
if isCelNameBoundary(expr, idx, afterName) {
for afterName < len(expr) && isCELWhitespace(expr[afterName]) {
afterName++
}
if afterName < len(expr) && expr[afterName] == '(' {
return true
}
}
offset = idx + len(name)
}
return false
}

func isCelNameBoundary(expr string, start, end int) bool {
if start > 0 && isCELNameChar(expr[start-1]) {
return false
}
if end < len(expr) && isCELNameChar(expr[end]) {
return false
}
return true
}

func isCELNameChar(ch byte) bool {
return ch == '.' || ch == '_' || ('0' <= ch && ch <= '9') || ('A' <= ch && ch <= 'Z') || ('a' <= ch && ch <= 'z')
}

func isCELWhitespace(ch byte) bool {
return ch == ' ' || ch == '\t' || ch == '\n' || ch == '\r'
}

func (k Context) RunTemplateBool(t gomplate.Template, env map[string]any) (bool, error) {
output, err := k.RunTemplate(t, env)
if err != nil {
Expand Down
89 changes: 89 additions & 0 deletions context/template_test.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,101 @@
package context

import (
"sync/atomic"
"testing"

"github.com/flanksource/gomplate/v3"
"github.com/google/cel-go/cel"
celtypes "github.com/google/cel-go/common/types"
"github.com/google/cel-go/common/types/ref"
. "github.com/onsi/gomega"
)

func TestRunTemplateOnlyBuildsReferencedCelEnvFuncs(t *testing.T) {
g := NewWithT(t)
oldCelEnvFuncs := CelEnvFuncs
oldTemplateFuncs := TemplateFuncs
defer func() {
CelEnvFuncs = oldCelEnvFuncs
TemplateFuncs = oldTemplateFuncs
}()

var usedBuilt atomic.Int32
var unusedBuilt atomic.Int32
CelEnvFuncs = map[string]func(Context) cel.EnvOption{
"bench.usedCel": func(ctx Context) cel.EnvOption {
usedBuilt.Add(1)
return cel.Function("bench.used",
cel.Overload("bench_used_string",
[]*cel.Type{cel.StringType},
cel.BoolType,
cel.UnaryBinding(func(arg ref.Val) ref.Val {
return celtypes.Bool(arg.Value() == "default")
}),
),
)
},
"bench.unused": func(ctx Context) cel.EnvOption {
unusedBuilt.Add(1)
return cel.Function("bench.unused",
cel.Overload("bench_unused_string",
[]*cel.Type{cel.StringType},
cel.StringType,
cel.UnaryBinding(func(arg ref.Val) ref.Val {
return celtypes.String(arg.Value().(string))
}),
),
)
},
}
TemplateFuncs = nil

ctx := New()
env := map[string]any{"config_namespace": "default"}

ok, err := ctx.RunTemplateBool(gomplate.Template{
Expression: `config_namespace == "default"`,
CacheKey: "test.run-template.no-registered-cel-func",
}, env)
g.Expect(err).NotTo(HaveOccurred())
g.Expect(ok).To(BeTrue())
g.Expect(usedBuilt.Load()).To(Equal(int32(0)))
g.Expect(unusedBuilt.Load()).To(Equal(int32(0)))

ok, err = ctx.RunTemplateBool(gomplate.Template{
Expression: `bench.used(config_namespace)`,
CacheKey: "test.run-template.referenced-cel-func",
}, env)
g.Expect(err).NotTo(HaveOccurred())
g.Expect(ok).To(BeTrue())
g.Expect(usedBuilt.Load()).To(Equal(int32(1)))
g.Expect(unusedBuilt.Load()).To(Equal(int32(0)))
}

func TestCelExpressionCalls(t *testing.T) {
tests := []struct {
name string
expr string
fn string
want bool
}{
{name: "direct namespaced call", expr: `bench.used(config_namespace)`, fn: "bench.used", want: true},
{name: "whitespace before args", expr: `bench.used (config_namespace)`, fn: "bench.used", want: true},
{name: "prefix is not a call", expr: `bench.used_extra(config_namespace)`, fn: "bench.used", want: false},
{name: "suffix is not a call", expr: `other.bench.used(config_namespace)`, fn: "bench.used", want: false},
{name: "similar db function prefix", expr: `db.external_users_all(scraper_id)`, fn: "db.external_users", want: false},
{name: "exact db function", expr: `db.external_users_all(scraper_id)`, fn: "db.external_users_all", want: true},
{name: "no function call", expr: `config_type == "Kubernetes::Pod"`, fn: "db.external_users", want: false},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
g := NewWithT(t)
g.Expect(celExpressionCalls(tt.expr, tt.fn)).To(Equal(tt.want))
})
}
}

func TestRunTemplate(t *testing.T) {
t.Parallel()

Expand Down
Loading