Skip to content

Commit 91c0bae

Browse files
authored
Enabling OPA AST optimizations in Skipper to store AST values in in-memory store (#3514)
**Context:** With open-policy-agent/opa#7125 Open Policy Agent introduced an option to read AST values from in-memory storage instead of interfaces/slices. When enabled it is expected to improve latency as this removes the time spent converting raw data values to AST during policy evaluation. The memory footprint of the store will increase, as processed AST values would take more space in memory than the corresponding raw data values, but overall memory usage of OPA might remain more stable over time, as pre-converted data is shared across evaluations and isn't recomputed for each evaluation. With this PR, the option `enable-open-policy-agent-data-preprocessing-optimization` is supported as a Skipper configuration at OPA registry level. In other words, when enabled it will be enabled for all the OPA systems running within the same Skipper ingress. ### Benchmark test results show huge improvments A 1000 run of the benchmark comparison. For the simple policy, <img width="805" alt="image" src="https://github.com/user-attachments/assets/b4a71b03-ed10-440f-bcd5-05a3e9d3dbaf" /> <google-sheets-html-origin><!--td {border: 1px solid #cccccc;}br {mso-data-placement:same-cell;}-->   | ns/op _default | ns/op _optimized | B/op _default | B/op _optimized | allocs/op _default | allocs/op _optimized -- | -- | -- | -- | -- | -- | -- Median | 24727.5 | 25016 | 18945 | 18961 | 308 | 309 Average | 26727.107 | 27058.068 | 18945.996 | 18961.234 | 308 | 309 P99 | 37737.5 | 39558.05 | 18951 | 18967 | 308 | 309 Max | 44914 | 49197 | 18953 | 18968 | 308 | 309 Min | 23375 | 23899 | 18945 | 18961 | 308 | 309 Deviation | 3703.249928 | 3840.160723 | 1.550609038 | 0.9110840104 | 0 | 0 For the slightly complex policy, <img width="778" alt="image" src="https://github.com/user-attachments/assets/c5cd93d2-38cc-47ef-b6b4-f31cad91c461" /> **Complex Policy** <google-sheets-html-origin><!--td {border: 1px solid #cccccc;}br {mso-data-placement:same-cell;}-->   | ns/op _default | ns/op _optimized | B/op _default | B/op _optimized | allocs/op _default | allocs/op _optimized -- | -- | -- | -- | -- | -- | -- Median | 1021329 | 28488.5 | 832732 | 20386 | 13990 | 336 Average | 1083208.684 | 29921.284 | 832734.342 | 20386.162 | 13990.147 | 336 P99 | 1612204.06 | 36141.25 | 832751 | 20388 | 13991 | 336 Max | 2908902 | 53455 | 832797 | 20394 | 13991 | 336 Min | 985625 | 26690 | 832726 | 20385 | 13990 | 336 Deviation | 140331.5701 | 3317.752934 | 7.238607147 | 1.144143813 | 0.3542831022 | 0 Signed-off-by: Pushpalanka Jayawardhana <pushpalanka.jayawardhana@zalando.de>
1 parent 0d0142d commit 91c0bae

9 files changed

Lines changed: 4716 additions & 70 deletions

File tree

config/config.go

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -297,17 +297,18 @@ type Config struct {
297297
LuaModules *listFlag `yaml:"lua-modules"`
298298
LuaSources *listFlag `yaml:"lua-sources"`
299299

300-
EnableOpenPolicyAgent bool `yaml:"enable-open-policy-agent"`
301-
EnableOpenPolicyAgentCustomControlLoop bool `yaml:"enable-open-policy-agent-custom-control-loop"`
302-
OpenPolicyAgentControlLoopInterval time.Duration `yaml:"open-policy-agent-control-loop-interval"`
303-
OpenPolicyAgentControlLoopMaxJitter time.Duration `yaml:"open-policy-agent-control-loop-max-jitter"`
304-
OpenPolicyAgentConfigTemplate string `yaml:"open-policy-agent-config-template"`
305-
OpenPolicyAgentEnvoyMetadata string `yaml:"open-policy-agent-envoy-metadata"`
306-
OpenPolicyAgentCleanerInterval time.Duration `yaml:"open-policy-agent-cleaner-interval"`
307-
OpenPolicyAgentStartupTimeout time.Duration `yaml:"open-policy-agent-startup-timeout"`
308-
OpenPolicyAgentRequestBodyBufferSize int64 `yaml:"open-policy-agent-request-body-buffer-size"`
309-
OpenPolicyAgentMaxRequestBodySize int64 `yaml:"open-policy-agent-max-request-body-size"`
310-
OpenPolicyAgentMaxMemoryBodyParsing int64 `yaml:"open-policy-agent-max-memory-body-parsing"`
300+
EnableOpenPolicyAgent bool `yaml:"enable-open-policy-agent"`
301+
EnableOpenPolicyAgentCustomControlLoop bool `yaml:"enable-open-policy-agent-custom-control-loop"`
302+
OpenPolicyAgentControlLoopInterval time.Duration `yaml:"open-policy-agent-control-loop-interval"`
303+
OpenPolicyAgentControlLoopMaxJitter time.Duration `yaml:"open-policy-agent-control-loop-max-jitter"`
304+
EnableOpenPolicyAgentDataPreProcessingOptimization bool `yaml:"enable-open-policy-agent-data-preprocessing-optimization"`
305+
OpenPolicyAgentConfigTemplate string `yaml:"open-policy-agent-config-template"`
306+
OpenPolicyAgentEnvoyMetadata string `yaml:"open-policy-agent-envoy-metadata"`
307+
OpenPolicyAgentCleanerInterval time.Duration `yaml:"open-policy-agent-cleaner-interval"`
308+
OpenPolicyAgentStartupTimeout time.Duration `yaml:"open-policy-agent-startup-timeout"`
309+
OpenPolicyAgentRequestBodyBufferSize int64 `yaml:"open-policy-agent-request-body-buffer-size"`
310+
OpenPolicyAgentMaxRequestBodySize int64 `yaml:"open-policy-agent-max-request-body-size"`
311+
OpenPolicyAgentMaxMemoryBodyParsing int64 `yaml:"open-policy-agent-max-memory-body-parsing"`
311312

312313
PassiveHealthCheck mapFlags `yaml:"passive-health-check"`
313314
}
@@ -536,6 +537,7 @@ func NewConfig() *Config {
536537
flag.BoolVar(&cfg.EnableOpenPolicyAgentCustomControlLoop, "enable-open-policy-agent-custom-control-loop", false, "when enabled skipper will use a custom control loop to orchestrate certain opa behaviour (like the download of new bundles) instead of relying on periodic plugin triggers")
537538
flag.DurationVar(&cfg.OpenPolicyAgentControlLoopInterval, "open-policy-agent-control-loop-interval", openpolicyagent.DefaultControlLoopInterval, "Interval between the execution of the control loop. Only applies if the custom control loop is enabled")
538539
flag.DurationVar(&cfg.OpenPolicyAgentControlLoopMaxJitter, "open-policy-agent-control-loop-max-jitter", openpolicyagent.DefaultControlLoopMaxJitter, "Maximum jitter to add to the control loop interval. Only applies if the custom control loop is enabled")
540+
flag.BoolVar(&cfg.EnableOpenPolicyAgentDataPreProcessingOptimization, "enable-open-policy-agent-data-preprocessing-optimization", false, "As a latency optimization, open policy agent will read values from in-memory storage as pre converted ASTs, removing conversion overhead at evaluation time. Currently experimental and if successful will be enabled by default")
539541
flag.StringVar(&cfg.OpenPolicyAgentConfigTemplate, "open-policy-agent-config-template", "", "file containing a template for an Open Policy Agent configuration file that is interpolated for each OPA filter instance")
540542
flag.StringVar(&cfg.OpenPolicyAgentEnvoyMetadata, "open-policy-agent-envoy-metadata", "", "JSON file containing meta-data passed as input for compatibility with Envoy policies in the format")
541543
flag.DurationVar(&cfg.OpenPolicyAgentCleanerInterval, "open-policy-agent-cleaner-interval", openpolicyagent.DefaultCleanIdlePeriod, "Duration in seconds to wait before cleaning up unused opa instances")
@@ -993,17 +995,18 @@ func (c *Config) ToOptions() skipper.Options {
993995
LuaModules: c.LuaModules.values,
994996
LuaSources: c.LuaSources.values,
995997

996-
EnableOpenPolicyAgent: c.EnableOpenPolicyAgent,
997-
EnableOpenPolicyAgentCustomControlLoop: c.EnableOpenPolicyAgentCustomControlLoop,
998-
OpenPolicyAgentControlLoopInterval: c.OpenPolicyAgentControlLoopInterval,
999-
OpenPolicyAgentControlLoopMaxJitter: c.OpenPolicyAgentControlLoopMaxJitter,
1000-
OpenPolicyAgentConfigTemplate: c.OpenPolicyAgentConfigTemplate,
1001-
OpenPolicyAgentEnvoyMetadata: c.OpenPolicyAgentEnvoyMetadata,
1002-
OpenPolicyAgentCleanerInterval: c.OpenPolicyAgentCleanerInterval,
1003-
OpenPolicyAgentStartupTimeout: c.OpenPolicyAgentStartupTimeout,
1004-
OpenPolicyAgentMaxRequestBodySize: c.OpenPolicyAgentMaxRequestBodySize,
1005-
OpenPolicyAgentRequestBodyBufferSize: c.OpenPolicyAgentRequestBodyBufferSize,
1006-
OpenPolicyAgentMaxMemoryBodyParsing: c.OpenPolicyAgentMaxMemoryBodyParsing,
998+
EnableOpenPolicyAgent: c.EnableOpenPolicyAgent,
999+
EnableOpenPolicyAgentCustomControlLoop: c.EnableOpenPolicyAgentCustomControlLoop,
1000+
OpenPolicyAgentControlLoopInterval: c.OpenPolicyAgentControlLoopInterval,
1001+
OpenPolicyAgentControlLoopMaxJitter: c.OpenPolicyAgentControlLoopMaxJitter,
1002+
EnableOpenPolicyAgentDataPreProcessingOptimization: c.EnableOpenPolicyAgentDataPreProcessingOptimization,
1003+
OpenPolicyAgentConfigTemplate: c.OpenPolicyAgentConfigTemplate,
1004+
OpenPolicyAgentEnvoyMetadata: c.OpenPolicyAgentEnvoyMetadata,
1005+
OpenPolicyAgentCleanerInterval: c.OpenPolicyAgentCleanerInterval,
1006+
OpenPolicyAgentStartupTimeout: c.OpenPolicyAgentStartupTimeout,
1007+
OpenPolicyAgentMaxRequestBodySize: c.OpenPolicyAgentMaxRequestBodySize,
1008+
OpenPolicyAgentRequestBodyBufferSize: c.OpenPolicyAgentRequestBodyBufferSize,
1009+
OpenPolicyAgentMaxMemoryBodyParsing: c.OpenPolicyAgentMaxMemoryBodyParsing,
10071010

10081011
PassiveHealthCheck: c.PassiveHealthCheck.values,
10091012
}

filters/openpolicyagent/opaauthorizerequest/benchmark_test.go

Lines changed: 67 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,8 @@ func BenchmarkMinimalPolicyBundle(b *testing.B) {
343343
// pre-compiled bundles, serving as a representative use case.
344344

345345
// A sample bundles for this benchmark are located at testResources/split-bundles.
346+
// To evaluate the performance of data-preprocessing optimization, use the resources at
347+
// testResources/data-pre-processing-visible-impact.
346348
// To generate a .tgz bundle, use the following command:
347349
//
348350
// opa build -b <bundle_directory> -o <output_file.tgz>
@@ -357,49 +359,72 @@ func BenchmarkMinimalPolicyBundle(b *testing.B) {
357359
// and filterOpts variables are correctly configured to match your bundle and
358360
// the roots in .manifest files in your bundles do not overlap.
359361
func BenchmarkSplitPolicyAndDataBundles(b *testing.B) {
360-
policyBundle := "policy"
361-
dataBundle := "context-data"
362-
363-
bundleFiles := map[string]string{
364-
policyBundle: "testResources/split-bundles/policy.tgz",
365-
dataBundle: "testResources/split-bundles/context-data.tgz",
362+
type benchmarkCase struct {
363+
name string
364+
createFunc func(FilterOptions) (filters.Filter, error)
366365
}
367366

368-
opaControlPlane := newOpaControlPlaneServingDataAndPolicyBundles(b, bundleFiles)
369-
defer opaControlPlane.Close()
370-
371-
filterOpts := FilterOptions{
372-
OpaControlPlaneUrl: opaControlPlane.URL,
373-
DecisionConsumerUrl: opaControlPlane.URL,
374-
DecisionPath: "policy/allow",
375-
BundleNames: []string{policyBundle, dataBundle},
376-
DecisionLogging: false,
367+
cases := []benchmarkCase{
368+
{
369+
name: "default_filter",
370+
createFunc: func(opts FilterOptions) (filters.Filter, error) {
371+
return createOpaFilterForMultipleBundles(opts)
372+
},
373+
},
374+
{
375+
name: "with_data_preprocessing_optimization",
376+
createFunc: func(opts FilterOptions) (filters.Filter, error) {
377+
return createOpaFilterWithDataProcessingOptimization(opts)
378+
},
379+
},
377380
}
378381

379-
f, err := createOpaFilterForMultipleBundles(filterOpts)
380-
require.NoError(b, err)
382+
for _, bc := range cases {
383+
b.Run(bc.name, func(b *testing.B) {
384+
bundleFiles := map[string]string{
385+
"policy": "testResources/split-bundles/policy.tgz",
386+
"context-data": "testResources/split-bundles/context-data.tgz",
387+
}
381388

382-
requestUrl, err := url.Parse("http://opa-authorized.test/allow/alice")
383-
require.NoError(b, err)
389+
opaControlPlane := newOpaControlPlaneServingDataAndPolicyBundles(b, bundleFiles)
390+
defer opaControlPlane.Close()
384391

385-
b.ResetTimer()
386-
b.RunParallel(func(pb *testing.PB) {
387-
for pb.Next() {
388-
ctx := &filtertest.Context{
389-
FStateBag: map[string]interface{}{},
390-
FResponse: &http.Response{},
391-
FRequest: &http.Request{
392-
URL: requestUrl,
393-
Method: "GET",
394-
},
395-
FMetrics: &metricstest.MockMetrics{},
392+
filterOpts := FilterOptions{
393+
OpaControlPlaneUrl: opaControlPlane.URL,
394+
DecisionConsumerUrl: opaControlPlane.URL,
395+
DecisionPath: "policy/allow",
396+
BundleNames: []string{"policy", "context-data"},
397+
DecisionLogging: false,
396398
}
397-
f.Request(ctx)
398-
assert.False(b, ctx.FServed)
399-
assert.NotEqual(b, 403, ctx.FResponse.StatusCode, "Expected 403 Forbidden response")
400-
}
401-
})
402399

400+
f, err := bc.createFunc(filterOpts)
401+
require.NoError(b, err)
402+
403+
requestURL, err := url.Parse("http://opa-authorized.test/allow/alice")
404+
require.NoError(b, err)
405+
406+
b.ReportAllocs()
407+
b.ResetTimer()
408+
409+
b.RunParallel(func(pb *testing.PB) {
410+
for pb.Next() {
411+
ctx := &filtertest.Context{
412+
FStateBag: map[string]interface{}{},
413+
FResponse: &http.Response{},
414+
FRequest: &http.Request{
415+
URL: requestURL,
416+
Method: "GET",
417+
},
418+
FMetrics: &metricstest.MockMetrics{},
419+
}
420+
421+
f.Request(ctx)
422+
assert.False(b, ctx.FServed)
423+
assert.NotEqual(b, 403, ctx.FResponse.StatusCode, "Expected 403 Forbidden response")
424+
}
425+
})
426+
})
427+
}
403428
}
404429

405430
func newOpaControlPlaneServingDataAndPolicyBundles(b *testing.B, bundleFiles map[string]string) *httptest.Server {
@@ -506,6 +531,13 @@ func createOpaFilterForMultipleBundles(opts FilterOptions) (filters.Filter, erro
506531
return spec.CreateFilter([]interface{}{opts.BundleNames[0], opts.ContextExtensions})
507532
}
508533

534+
func createOpaFilterWithDataProcessingOptimization(opts FilterOptions) (filters.Filter, error) {
535+
config := generateConfigForMultipleBundles(opts.BundleNames, opts.OpaControlPlaneUrl, opts.DecisionConsumerUrl, opts.DecisionPath, opts.DecisionLogging)
536+
registry := openpolicyagent.NewOpenPolicyAgentRegistry(openpolicyagent.WithEnableDataPreProcessingOptimization(true))
537+
spec := NewOpaAuthorizeRequestSpec(registry, openpolicyagent.WithConfigTemplate(config))
538+
return spec.CreateFilter([]interface{}{opts.BundleNames[0], opts.ContextExtensions})
539+
}
540+
509541
func generateConfigForMultipleBundles(bundlesNames []string, opaControlPlane string, decisionLogConsumer string, decisionPath string, decisionLogging bool) []byte {
510542
var decisionPlugin string
511543
if decisionLogging {
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"roots": ["roles"]
3+
}

0 commit comments

Comments
 (0)