refact: avoid mem alloc with disabled decision log (#632)

Signed-off-by: Anthony Regeda <regedaster@gmail.com>

Makefile

@@ -175,7 +175,7 @@ deploy-ci: docker-login ensure-release-dir start-builder ci-build-linux ci-build
 
 .PHONY: test
 test: generate
-	$(DISABLE_CGO) $(GO) test -v -bench=. $(PACKAGES)
+	$(DISABLE_CGO) $(GO) test -v -bench=. -benchmem $(PACKAGES)
 
 .PHONY: test-e2e
 test-e2e:

envoyauth/response.go

@@ -34,6 +34,10 @@ type StopFunc = func()
 // TransactionCloser should be called to abort the transaction
 type TransactionCloser func(ctx context.Context, err error) error
 
+func noopTransactionCloser(context.Context, error) error {
+	return nil // no-op default
+}
+
 // NewEvalResult creates a new EvalResult and a StopFunc that is used to stop the timer for metrics
 func NewEvalResult(opts ...func(*EvalResult)) (*EvalResult, StopFunc, error) {
 	var err error
@@ -67,13 +71,9 @@ func NewEvalResult(opts ...func(*EvalResult)) (*EvalResult, StopFunc, error) {
 func (result *EvalResult) GetTxn(ctx context.Context, store storage.Store) (storage.Transaction, TransactionCloser, error) {
 	params := storage.TransactionParams{}
 
-	noopCloser := func(ctx context.Context, err error) error {
-		return nil // no-op default
-	}
-
 	txn, err := store.NewTransaction(ctx, params)
 	if err != nil {
-		return nil, noopCloser, err
+		return nil, noopTransactionCloser, err
 	}
 
 	// Setup a closer function that will abort the transaction.

go.mod

@@ -1,6 +1,7 @@
 module github.com/open-policy-agent/opa-envoy-plugin
 
 go 1.22.0
+
 toolchain go1.23.1
 
 require (

internal/internal.go

@@ -33,11 +33,13 @@ import (
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/reflection"
 	"google.golang.org/protobuf/reflect/protoregistry"
+	"google.golang.org/protobuf/types/known/structpb"
 
 	"github.com/open-policy-agent/opa/ast"
 	"github.com/open-policy-agent/opa/config"
 	"github.com/open-policy-agent/opa/logging"
 	"github.com/open-policy-agent/opa/plugins"
+	"github.com/open-policy-agent/opa/plugins/logs"
 	"github.com/open-policy-agent/opa/rego"
 	"github.com/open-policy-agent/opa/server"
 	"github.com/open-policy-agent/opa/storage"
@@ -48,8 +50,6 @@ import (
 
 	"go.opentelemetry.io/otel/trace"
 
-	_structpb "github.com/golang/protobuf/ptypes/struct"
-
 	"github.com/open-policy-agent/opa-envoy-plugin/envoyauth"
 	internal_util "github.com/open-policy-agent/opa-envoy-plugin/internal/util"
 	"github.com/open-policy-agent/opa-envoy-plugin/opa/decisionlog"
@@ -385,7 +385,7 @@ func (p *envoyExtAuthzGrpcServer) check(ctx context.Context, req interface{}) (*
 	var evalErr error
 	var internalErr *Error
 	start := time.Now()
-	logger := p.manager.Logger()
+	logger := p.Logger()
 
 	result, stopeval, err := envoyauth.NewEvalResult()
 	if err != nil {
@@ -417,10 +417,10 @@ func (p *envoyExtAuthzGrpcServer) check(ctx context.Context, req interface{}) (*
 				p.metricErrorCounter.With(prometheus.Labels{"reason": internalErr.Code}).Inc()
 			}
 		}
-		logErr := p.log(ctx, input, result, err)
+		logErr := p.logDecision(ctx, input, result, err)
 		if logErr != nil {
 			_ = txnClose(ctx, logErr) // Ignore error
-			p.Logger().WithFields(map[string]interface{}{"err": logErr}).Debug("Error when logging event")
+			logger.WithFields(map[string]interface{}{"err": logErr}).Debug("Error when logging event")
 			if p.cfg.EnablePerformanceMetrics {
 				p.metricErrorCounter.With(prometheus.Labels{"reason": "unknown_log_error"}).Inc()
 			}
@@ -472,8 +472,6 @@ func (p *envoyExtAuthzGrpcServer) check(ctx context.Context, req interface{}) (*
 		return nil, stop, internalErr
 	}
 
-	resp := &ext_authz_v3.CheckResponse{}
-
 	var allowed bool
 	allowed, err = result.IsAllowed()
 	if err != nil {
@@ -482,6 +480,8 @@ func (p *envoyExtAuthzGrpcServer) check(ctx context.Context, req interface{}) (*
 		return nil, stop, internalErr
 	}
 
+	resp := &ext_authz_v3.CheckResponse{}
+
 	status := int32(code.Code_PERMISSION_DENIED)
 	if allowed {
 		status = int32(code.Code_OK)
@@ -498,7 +498,7 @@ func (p *envoyExtAuthzGrpcServer) check(ctx context.Context, req interface{}) (*
 			return nil, stop, internalErr
 		}
 
-		var dynamicMetadata *_structpb.Struct
+		var dynamicMetadata *structpb.Struct
 		dynamicMetadata, err = result.GetDynamicMetadata()
 		if err != nil {
 			err = errors.Wrap(err, "failed to get dynamic metadata")
@@ -578,7 +578,7 @@ func (p *envoyExtAuthzGrpcServer) check(ctx context.Context, req interface{}) (*
 			Observe(float64(totalDecisionTime.Seconds()))
 	}
 
-	p.manager.Logger().WithFields(map[string]interface{}{
+	logger.WithFields(map[string]interface{}{
 		"query":               p.cfg.parsedQuery.String(),
 		"dry-run":             p.cfg.DryRun,
 		"decision":            result.Decision,
@@ -601,21 +601,22 @@ func (p *envoyExtAuthzGrpcServer) check(ctx context.Context, req interface{}) (*
 
 	// Add decision_id to dynamic metadata
 	if resp.DynamicMetadata == nil {
-		resp.DynamicMetadata = &_structpb.Struct{
-			Fields: map[string]*_structpb.Value{},
+		resp.DynamicMetadata = &structpb.Struct{
+			Fields: make(map[string]*structpb.Value, 1),
 		}
 	}
 
-	resp.DynamicMetadata.Fields["decision_id"] = &_structpb.Value{
-		Kind: &_structpb.Value_StringValue{
-			StringValue: result.DecisionID,
-		},
-	}
+	resp.DynamicMetadata.Fields["decision_id"] = structpb.NewStringValue(result.DecisionID)
 
 	return resp, stop, nil
 }
 
-func (p *envoyExtAuthzGrpcServer) log(ctx context.Context, input interface{}, result *envoyauth.EvalResult, err error) error {
+func (p *envoyExtAuthzGrpcServer) logDecision(ctx context.Context, input interface{}, result *envoyauth.EvalResult, err error) error {
+	plugin := logs.Lookup(p.manager)
+	if plugin == nil {
+		return nil
+	}
+
 	info := &server.Info{
 		Timestamp: time.Now(),
 		Input:     &input,
@@ -643,7 +644,7 @@ func (p *envoyExtAuthzGrpcServer) log(ctx context.Context, input interface{}, re
 		info.NDBuiltinCache = &x
 	}
 
-	return decisionlog.LogDecision(ctx, p.manager, info, result, err)
+	return decisionlog.LogDecision(ctx, plugin, info, result, err)
 }
 
 func stringPathToDataRef(s string) (r ast.Ref) {

internal/internal_bench_test.go

@@ -17,7 +17,28 @@ import (
 func BenchmarkCheck(b *testing.B) {
 	var req ext_authz.CheckRequest
 	if err := util.Unmarshal([]byte(exampleAllowedRequest), &req); err != nil {
-		panic(err)
+		b.Fatal(err)
+	}
+
+	server := testAuthzServer(nil)
+	ctx := context.Background()
+
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		output, err := server.Check(ctx, &req)
+		if err != nil {
+			b.Fatal(err)
+		}
+		if output.Status.Code != int32(code.Code_OK) {
+			b.Fatal("Expected request to be allowed but got:", output)
+		}
+	}
+}
+
+func BenchmarkCheck_withCustomLogger(b *testing.B) {
+	var req ext_authz.CheckRequest
+	if err := util.Unmarshal([]byte(exampleAllowedRequest), &req); err != nil {
+		b.Fatal(err)
 	}
 
 	server := testAuthzServer(nil, withCustomLogger(&testPlugin{}))

internal/internal_test.go

@@ -2138,7 +2138,7 @@ func TestPrometheusMetrics(t *testing.T) {
 
 func TestLogWithASTError(t *testing.T) {
 	server := testAuthzServer(nil, withCustomLogger(&testPlugin{}))
-	err := server.log(context.Background(), nil, &envoyauth.EvalResult{}, &ast.Error{Code: "foo"})
+	err := server.logDecision(context.Background(), nil, &envoyauth.EvalResult{}, &ast.Error{Code: "foo"})
 	if err != nil {
 		panic(err)
 	}
@@ -2149,7 +2149,7 @@ func TestLogWithCancelError(t *testing.T) {
 	customLogger := &testPlugin{}
 
 	server := testAuthzServer(nil, withCustomLogger(customLogger))
-	err := server.log(context.Background(), nil, &envoyauth.EvalResult{}, &topdown.Error{
+	err := server.logDecision(context.Background(), nil, &envoyauth.EvalResult{}, &topdown.Error{
 		Code:    topdown.CancelErr,
 		Message: "caller cancelled query execution",
 	})

opa/decisionlog/decision_log.go

@@ -5,7 +5,6 @@ import (
 
 	"github.com/open-policy-agent/opa-envoy-plugin/envoyauth"
 	"github.com/open-policy-agent/opa/ast"
-	"github.com/open-policy-agent/opa/plugins"
 	"github.com/open-policy-agent/opa/plugins/logs"
 	"github.com/open-policy-agent/opa/server"
 	"github.com/open-policy-agent/opa/storage"
@@ -21,15 +20,10 @@ func (e *internalError) Error() string {
 }
 
 // LogDecision - Logs a decision log event
-func LogDecision(ctx context.Context, manager *plugins.Manager, info *server.Info, result *envoyauth.EvalResult, err error) error {
-	plugin := logs.Lookup(manager)
-	if plugin == nil {
-		return nil
-	}
-
+func LogDecision(ctx context.Context, plugin *logs.Plugin, info *server.Info, result *envoyauth.EvalResult, err error) error {
 	info.Revision = result.Revision
 
-	bundles := map[string]server.BundleInfo{}
+	bundles := make(map[string]server.BundleInfo, len(result.Revisions))
 	for name, rev := range result.Revisions {
 		bundles[name] = server.BundleInfo{Revision: rev}
 	}