Skip to content

reduce per-event work in rule manager hot path #794

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
YakirOren merged 1 commit into from
Apr 26, 2026
Merged

reduce per-event work in rule manager hot path #794

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 12 additions & 3 deletions pkg/rulemanager/cel/cel.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,10 @@ func (c *CEL) getOrCreateProgram(expression string) (cel.Program, error) {
return program, nil
}

func (c *CEL) createEvalContext(event *events.EnrichedEvent) map[string]any {
// CreateEvalContext builds a CEL evaluation context for the given event.
// The returned map is safe to reuse across multiple sequential EvaluateRuleWithContext
// calls for the same event — program.Eval reads but does not mutate it.
func (c *CEL) CreateEvalContext(event *events.EnrichedEvent) map[string]any {
eventType := event.Event.GetEventType()

// Apply event converter if one is registered, otherwise cast to CelEvent
Expand Down Expand Up @@ -197,8 +200,14 @@ func (c *CEL) evaluateProgramWithContext(expression string, evalContext map[stri

func (c *CEL) EvaluateRule(event *events.EnrichedEvent, expressions []typesv1.RuleExpression) (bool, error) {
eventType := event.Event.GetEventType()
evalContext := c.createEvalContext(event)
evalContext := c.CreateEvalContext(event)
return c.EvaluateRuleWithContext(evalContext, eventType, expressions)
}

// EvaluateRuleWithContext evaluates expressions against a pre-built evaluation context.
// Callers who process multiple rules for the same event should build the context once
// via CreateEvalContext and reuse it here to avoid per-rule allocation.
func (c *CEL) EvaluateRuleWithContext(evalContext map[string]any, eventType utils.EventType, expressions []typesv1.RuleExpression) (bool, error) {
for _, expression := range expressions {
if expression.EventType != eventType {
continue
Expand Down Expand Up @@ -227,7 +236,7 @@ func (c *CEL) EvaluateRule(event *events.EnrichedEvent, expressions []typesv1.Ru
}

func (c *CEL) EvaluateExpression(event *events.EnrichedEvent, expression string) (string, error) {
evalContext := c.createEvalContext(event)
evalContext := c.CreateEvalContext(event)

out, err := c.evaluateProgramWithContext(expression, evalContext)
if err != nil {
Expand Down
2 changes: 2 additions & 0 deletions pkg/rulemanager/cel/cel_interface.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ import (

type RuleEvaluator interface {
EvaluateRule(event *events.EnrichedEvent, expressions []typesv1.RuleExpression) (bool, error)
EvaluateRuleWithContext(evalContext map[string]any, eventType utils.EventType, expressions []typesv1.RuleExpression) (bool, error)
EvaluateExpression(event *events.EnrichedEvent, expression string) (string, error)
CreateEvalContext(event *events.EnrichedEvent) map[string]any
RegisterHelper(function cel.EnvOption) error
RegisterCustomType(eventType utils.EventType, obj interface{}) error
RegisterEventConverter(eventType utils.EventType, converter func(utils.K8sEvent) utils.K8sEvent)
Expand Down
26 changes: 20 additions & 6 deletions pkg/rulemanager/rule_manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"crypto/md5"
"fmt"
"runtime/pprof"
"strconv"
"time"

"github.com/armosec/armoapi-go/armotypes"
Expand Down Expand Up @@ -210,6 +211,7 @@ func (rm *RuleManager) ReportEnrichedEvent(enrichedEvent *events.EnrichedEvent)
eventType := enrichedEvent.Event.GetEventType()

var eventFields prefilter.EventFields
var evalContext map[string]any

for _, rule := range rules {
if !rule.Enabled {
Expand Down Expand Up @@ -245,11 +247,15 @@ func (rm *RuleManager) ReportEnrichedEvent(enrichedEvent *events.EnrichedEvent)
continue
}

if evalContext == nil {
evalContext = rm.celEvaluator.CreateEvalContext(enrichedEvent)
}

startTime := time.Now()
var shouldAlert bool
var err error
pprof.Do(context.Background(), pprof.Labels("rule", rule.ID), func(_ context.Context) {
shouldAlert, err = rm.celEvaluator.EvaluateRule(enrichedEvent, ruleExpressions)
shouldAlert, err = rm.celEvaluator.EvaluateRuleWithContext(evalContext, eventType, ruleExpressions)
})
evaluationTime := time.Since(startTime)
rm.metrics.ReportRuleEvaluationTime(rule.Name, eventType, evaluationTime)
Expand Down Expand Up @@ -305,9 +311,11 @@ func (rm *RuleManager) enrichEventWithContext(enrichedEvent *events.EnrichedEven
}
if contextInfo, found := rm.mntnsRegistry.Lookup(mntnsID); found {
enrichedEvent.SourceContext = contextInfo
logger.L().Debug("RuleManager - enriched event with context",
helpers.String("mntns", fmt.Sprintf("%d", mntnsID)),
helpers.String("context", string(contextInfo.Context())))
if logger.L().GetLevel() == helpers.DebugLevel.String() {
logger.L().Debug("RuleManager - enriched event with context",
helpers.String("mntns", strconv.FormatUint(mntnsID, 10)),
helpers.String("context", string(contextInfo.Context())))
}
Comment on lines +314 to +318

@coderabbitai coderabbitai Bot Apr 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect the kubescape go-logger API to confirm what GetLevel returns and
# whether a level-aware "is enabled" helper exists.
fd -t f -e go . | xargs rg -nP -C2 '\bGetLevel\s*\(' | head -50
echo '---'
rg -nP -C2 '\bDebugLevel\b' --type=go | head -40
echo '---'
# Look for any IsEnabled / Enabled-style API on the logger.
rg -nP -C2 '\b(IsEnabled|Enabled|IsLevelEnabled)\s*\(' --type=go | head -40

Repository: kubescape/node-agent

Length of output: 4277

🌐 Web query:

kubescape go-logger Logger GetLevel IsEnabled debug level API

💡 Result:

The kubescape/go-logger package provides a unified logging interface (accessed via logger.L) that wraps multiple underlying loggers (pretty, slog, zap, icon, mock) switchable via InitLogger(name) or env var KS_LOGGER_NAME (default "pretty"). Log levels are set via env var KS_LOGGER_LEVEL (default "info"; likely supports standard levels like "debug", "info", "warn", "error"). No public API methods named GetLevel or IsEnabled are documented or found in official sources for the Logger interface. Usage examples show direct level-specific calls like L.Debug("msg"), L.Info("msg") without prior level checks. Individual wrapped loggers (e.g., zap via AtomicLevel.Enabled(lvl), zerolog via logger.Info.Enabled) may support such methods internally, but they are not exposed on the unified Logger. To enable debug level: Set KS_LOGGER_LEVEL=debug (or equivalent via InitLogger if supported). No programmatic level querying/checking API is evidenced in docs or pkg.go.dev pages.

Citations:

🏁 Script executed:

#!/bin/bash

# Find where GetLevel is implemented - check helpers or logger packages
echo "=== Searching for GetLevel implementation ==="
rg -nP '\bGetLevel\s*\(' --type=go -A 3 | head -60

echo "=== Checking Debug method signature in logger ==="
rg -nP 'func.*Debug\s*\(' --type=go -A 2 | grep -A 2 "logger\|Logger" | head -40

echo "=== Looking for DebugLevel definition ==="
rg -nP '\bDebugLevel\b' --type=go -B 2 -A 2 | head -60

echo "=== Checking go-logger or logger package imports ==="
fd -t f -e go . -exec grep -l "go-logger\|logger.L()" {} \; | head -5

Repository: kubescape/node-agent

Length of output: 1881

🏁 Script executed:

#!/bin/bash

# Check the helpers package to understand DebugLevel
echo "=== Finding helpers package with DebugLevel ==="
fd -t f -name "helpers.go" -o -name "*helpers*" --type f | head -10

Repository: kubescape/node-agent

Length of output: 361

🏁 Script executed:

#!/bin/bash

# Look at the actual logger package being imported
echo "=== Check imports in the rule_manager file ==="
head -30 pkg/rulemanager/rule_manager.go | grep -E "import|logger"

echo ""
echo "=== Check if logger.L() is from go-logger or local package ==="
rg -nP 'import.*logger' pkg/rulemanager/rule_manager.go

Repository: kubescape/node-agent

Length of output: 260

🏁 Script executed:

#!/bin/bash

# Find the go-logger package - check vendor or go.mod
echo "=== Check go.mod for go-logger ==="
rg -nP 'go-logger' go.mod

echo ""
echo "=== Look for vendor directory ==="
find . -type d -name "go-logger" 2>/dev/null | head -5

echo ""
echo "=== Check if there are any level comparison utilities ==="
rg -nP '(DebugLevel|TraceLevel|VerboseLevel)\b' --type=go | head -30

Repository: kubescape/node-agent

Length of output: 486

🏁 Script executed:

#!/bin/bash

# Look at how logger.L() is used - especially to understand what GetLevel returns
echo "=== All uses of logger.L().GetLevel() ==="
rg -nP 'logger\.L\(\)\.GetLevel' --type=go -B 1 -A 1

echo ""
echo "=== Check cmd/main.go to see how it compares levels ==="
sed -n '370,380p' cmd/main.go

Repository: kubescape/node-agent

Length of output: 998

🏁 Script executed:

#!/bin/bash

# Try to find logger implementation or interface definition in the codebase
echo "=== Looking for logger interface or Logger type definition ==="
rg -nP 'type Logger interface|func L\(\)' --type=go -A 5 | head -50

echo ""
echo "=== Check if Debug() method is being called without prior level check elsewhere ==="
rg -nP 'logger\.L\(\)\.Debug\(' --type=go | head -20

Repository: kubescape/node-agent

Length of output: 2820

🏁 Script executed:

#!/bin/bash

# Check what log levels are actually defined in helpers
echo "=== Look for log level constants in helpers ==="
rg -nP 'Level\s*(=|:)' --type=go | grep -i 'debug\|trace\|verbose\|info\|warn\|error' | head -30

echo ""
echo "=== Check helpers package imports ==="
head -50 pkg/rulemanager/rule_manager.go | grep -A 20 "import"

Repository: kubescape/node-agent

Length of output: 906

🏁 Script executed:

#!/bin/bash

# Check the go-logger API directly by examining its usage patterns
echo "=== All places calling helpers.DebugLevel ==="
rg -nP 'helpers\.DebugLevel' --type=go -B 1 -A 1

echo ""
echo "=== Check what helpers package is actually exported ==="
rg -nP '(const|type|var).*Level' --type=go | head -20

Repository: kubescape/node-agent

Length of output: 444

🌐 Web query:

go-logger kubescape github logger levels supported debug trace info

💡 Result:

The kubescape/go-logger package supports logger levels configurable via the KS_LOGGER_LEVEL environment variable (default: info). The CLI flag -l, --logger sets the log level, explicitly listing: debug, info, warning, error, fatal. Examples show Info method usage with [info] prefix. No explicit mention of "trace" level in official sources; it aligns with standard Go log/slog and Zap levels (Debug, Info, Warn/Error, Fatal), without Trace.

Citations:

Remove the debug-level gate or verify Debug() self-gates before adding manual checks.

The concern about the exact-equality check is partially flawed: kubescape/go-logger supports only five levels (debug, info, warning, error, fatal) with no more-verbose levels like "trace" to exclude.

However, two separate issues remain:

  1. No IsEnabled() API exists. The suggestion to use a level-aware predicate cannot be implemented; go-logger does not expose such a method publicly.
  2. The check may be redundant. Throughout the codebase, Debug() is called in 50+ places without prior GetLevel() checks, suggesting the logger already self-gates. If that's the case, the manual gate here (and its helpers.DebugLevel.String() allocation) is unnecessary. Verify whether Debug() self-gates; if it does, remove the condition entirely.
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@pkg/rulemanager/rule_manager.go` around lines 314 - 318, The manual level
check using logger.L().GetLevel() == helpers.DebugLevel.String() is redundant;
verify that logger.L().Debug() self-gates and then remove the surrounding
if-block and the helpers.DebugLevel.String() allocation—call
logger.L().Debug(...) directly with the same arguments (helpers.String("mntns",
strconv.FormatUint(mntnsID, 10)), helpers.String("context",
string(contextInfo.Context()))) so the enriched-event log is emitted only when
Debug is enabled without the manual check.

}
}

Expand Down Expand Up @@ -362,22 +370,28 @@ func (rm *RuleManager) EvaluatePolicyRulesForEvent(eventType utils.EventType, ev
creator := rm.ruleBindingCache.GetRuleCreator()
rules := creator.CreateRulePolicyRulesByEventType(eventType)

enrichedEvent := &events.EnrichedEvent{Event: event}
var evalContext map[string]any

for _, rule := range rules {
if !rule.SupportPolicy {
continue
}

enrichedEvent := &events.EnrichedEvent{Event: event}
ruleExpressions := rm.getRuleExpressions(rule, eventType)
if len(ruleExpressions) == 0 {
continue
}

if evalContext == nil {
evalContext = rm.celEvaluator.CreateEvalContext(enrichedEvent)
}

startTime := time.Now()
var shouldAlert bool
var err error
pprof.Do(context.Background(), pprof.Labels("rule", rule.ID), func(_ context.Context) {
shouldAlert, err = rm.celEvaluator.EvaluateRule(enrichedEvent, ruleExpressions)
shouldAlert, err = rm.celEvaluator.EvaluateRuleWithContext(evalContext, eventType, ruleExpressions)
})
evaluationTime := time.Since(startTime)
rm.metrics.ReportRuleEvaluationTime(rule.ID, eventType, evaluationTime)
Expand Down
Loading