From c6a341baa516280a2b6fb98e3b0a5032f92dfd9a Mon Sep 17 00:00:00 2001 From: Stephan Renatus Date: Fri, 17 Mar 2023 10:31:43 +0100 Subject: [PATCH] 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 --- plugins/logs/plugin.go | 27 +++++------ plugins/logs/plugin_benchmark_test.go | 65 ++++++++++++--------------- plugins/logs/plugin_test.go | 18 ++++++-- 3 files changed, 54 insertions(+), 56 deletions(-) diff --git a/plugins/logs/plugin.go b/plugins/logs/plugin.go index f83365a4ca..7450cb8d10 100644 --- a/plugins/logs/plugin.go +++ b/plugins/logs/plugin.go @@ -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), diff --git a/plugins/logs/plugin_benchmark_test.go b/plugins/logs/plugin_benchmark_test.go index 80d46bd755..1087592301 100644 --- a/plugins/logs/plugin_benchmark_test.go +++ b/plugins/logs/plugin_benchmark_test.go @@ -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") } } - } diff --git a/plugins/logs/plugin_test.go b/plugins/logs/plugin_test.go index 0a1134157a..feb727941a 100644 --- a/plugins/logs/plugin_test.go +++ b/plugins/logs/plugin_test.go @@ -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) }