plugins/logs: don't convert EventV1 to ast.Value twice (#5761)

The existing benchmarks on the masking can't be compared with before/after:
it's unfair, we're no longer converting the event to ast.Value inside that
method.

But in the overall query processing, this change should be beneficial: we
avoid converting every event _twice_.

Signed-off-by: Stephan Renatus <stephan@styra.com>
This commit is contained in:
Stephan Renatus
2023-03-17 10:31:43 +01:00
committed by GitHub
parent d0c1add575
commit c6a341baa5
3 changed files with 54 additions and 56 deletions
+10 -17
View File
@@ -609,7 +609,12 @@ func (p *Plugin) Log(ctx context.Context, decision *server.Info) error {
inputAST: decision.InputAST,
}
drop, err := p.dropEvent(ctx, decision.Txn, &event)
input, err := event.AST()
if err != nil {
return err
}
drop, err := p.dropEvent(ctx, decision.Txn, input)
if err != nil {
p.logger.Error("Log drop decision failed: %v.", err)
return nil
@@ -628,16 +633,14 @@ func (p *Plugin) Log(ctx context.Context, decision *server.Info) error {
event.Error = decision.Error
}
err = p.maskEvent(ctx, decision.Txn, &event)
if err != nil {
if err := p.maskEvent(ctx, decision.Txn, input, &event); err != nil {
// TODO(tsandall): see note below about error handling.
p.logger.Error("Log event masking failed: %v.", err)
return nil
}
if p.config.ConsoleLogs {
err := p.logEvent(event)
if err != nil {
if err := p.logEvent(event); err != nil {
p.logger.Error("Failed to log to console: %v.", err)
}
}
@@ -917,7 +920,7 @@ func (p *Plugin) bufferChunk(buffer *logBuffer, bs []byte) {
}
}
func (p *Plugin) maskEvent(ctx context.Context, txn storage.Transaction, event *EventV1) error {
func (p *Plugin) maskEvent(ctx context.Context, txn storage.Transaction, input ast.Value, event *EventV1) error {
mask, err := func() (rego.PreparedEvalQuery, error) {
@@ -953,11 +956,6 @@ func (p *Plugin) maskEvent(ctx context.Context, txn storage.Transaction, event *
return err
}
input, err := event.AST()
if err != nil {
return err
}
rs, err := mask.Eval(
ctx,
rego.EvalParsedInput(input),
@@ -985,7 +983,7 @@ func (p *Plugin) maskEvent(ctx context.Context, txn storage.Transaction, event *
return nil
}
func (p *Plugin) dropEvent(ctx context.Context, txn storage.Transaction, event *EventV1) (bool, error) {
func (p *Plugin) dropEvent(ctx context.Context, txn storage.Transaction, input ast.Value) (bool, error) {
drop, err := func() (rego.PreparedEvalQuery, error) {
@@ -1019,11 +1017,6 @@ func (p *Plugin) dropEvent(ctx context.Context, txn storage.Transaction, event *
return false, err
}
input, err := event.AST()
if err != nil {
return false, err
}
rs, err := drop.Eval(
ctx,
rego.EvalParsedInput(input),
+29 -36
View File
@@ -154,25 +154,22 @@ func BenchmarkMaskingNop(b *testing.B) {
}
plugin := New(cfg, manager)
var event EventV1
if err := util.UnmarshalJSON([]byte(largeEvent), &event); err != nil {
b.Fatal(err)
}
input, err := event.AST()
if err != nil {
b.Fatal(err)
}
b.ResetTimer()
for i := 0; i < b.N; i++ {
b.StopTimer()
var event EventV1
if err := util.UnmarshalJSON([]byte(largeEvent), &event); err != nil {
b.Fatal(err)
}
b.StartTimer()
if err := plugin.maskEvent(ctx, nil, &event); err != nil {
if err := plugin.maskEvent(ctx, nil, input, &event); err != nil {
b.Fatal(err)
}
}
}
func BenchmarkMaskingRuleCountsNop(b *testing.B) {
@@ -195,20 +192,20 @@ func BenchmarkMaskingRuleCountsNop(b *testing.B) {
}
plugin := New(cfg, manager)
for _, ruleCount := range numRules {
var event EventV1
if err := util.UnmarshalJSON([]byte(largeEvent), &event); err != nil {
b.Fatal(err)
}
input, err := event.AST()
if err != nil {
b.Fatal(err)
}
for _, ruleCount := range numRules {
b.Run(fmt.Sprintf("%dRules", ruleCount), func(b *testing.B) {
b.ResetTimer()
for i := 0; i < b.N; i++ {
b.StopTimer()
var event EventV1
if err := util.UnmarshalJSON([]byte(largeEvent), &event); err != nil {
b.Fatal(err)
}
b.StartTimer()
if err := plugin.maskEvent(ctx, nil, &event); err != nil {
if err := plugin.maskEvent(ctx, nil, input, &event); err != nil {
b.Fatal(err)
}
}
@@ -247,22 +244,19 @@ func BenchmarkMaskingErase(b *testing.B) {
b.Fatal(err)
}
plugin := New(cfg, manager)
var event EventV1
if err := util.UnmarshalJSON([]byte(largeEvent), &event); err != nil {
b.Fatal(err)
}
input, err := event.AST()
if err != nil {
b.Fatal(err)
}
b.ResetTimer()
for i := 0; i < b.N; i++ {
b.StopTimer()
var event EventV1
if err := util.UnmarshalJSON([]byte(largeEvent), &event); err != nil {
b.Fatal(err)
}
b.StartTimer()
if err := plugin.maskEvent(ctx, nil, &event); err != nil {
if err := plugin.maskEvent(ctx, nil, input, &event); err != nil {
b.Fatal(err)
}
@@ -270,5 +264,4 @@ func BenchmarkMaskingErase(b *testing.B) {
b.Fatal("Expected input to be erased")
}
}
}
+15 -3
View File
@@ -1877,8 +1877,12 @@ func TestPluginMasking(t *testing.T) {
Input: &tc.input,
NDBuiltinCache: &tc.ndbcache,
}
input, err := event.AST()
if err != nil {
t.Fatal(err)
}
if err := plugin.maskEvent(ctx, nil, event); err != nil {
if err := plugin.maskEvent(ctx, nil, input, event); err != nil {
t.Fatal(err)
}
@@ -1920,8 +1924,12 @@ func TestPluginMasking(t *testing.T) {
event = &EventV1{
Input: &tc.input,
}
input, err := event.AST()
if err != nil {
t.Fatal(err)
}
if err := plugin.maskEvent(ctx, nil, event); err != nil {
if err := plugin.maskEvent(ctx, nil, input, event); err != nil {
t.Fatal(err)
}
@@ -2011,8 +2019,12 @@ func TestPluginDrop(t *testing.T) {
if err := plugin.Start(ctx); err != nil {
t.Fatal(err)
}
input, err := tc.event.AST()
if err != nil {
t.Fatal(err)
}
drop, err := plugin.dropEvent(ctx, nil, tc.event)
drop, err := plugin.dropEvent(ctx, nil, input)
if err != nil {
t.Fatal(err)
}