Skip to content
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

Jump to bottom

Refactor: Improve OPA authorization filter benchmarks #3391

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Refactor: Improve OPA authorization filter benchmarks #3391

wants to merge 1 commit into from
+459 −242

Conversation

mefarazath
Copy link
Collaborator

@mefarazath mefarazath commented Jan 31, 2025 •
edited
Loading

Changes,

  • Move OPA filter benchmarks to a separate file
  • Add parallel execution to benchmarks (to detect contention issues and overall performance under load)
  • Include benchmarks for minimal policy with/without decision logging
  • Add reference benchmark to with a policy bundle

Refactor: Improve OPA authorization filter benchmarks
0216434 
- Moved benchmarks to a separate file
- Added parallel execution to benchmarks
- Included benchmarks for minimal policy with/without decision logging
- Added reference benchmark running with a policy bundle

Signed-off-by: Farasath Ahamed <[email protected]>
@mefarazath mefarazath force-pushed the opa-benchmark-restructure branch from 2ef7407 to 0216434 Compare January 31, 2025 09:11
@mefarazath mefarazath added refactor minor no risk changes, for example new filters labels Jan 31, 2025
@mefarazath
Copy link
Collaborator Author

mefarazath commented Jan 31, 2025 •
edited
Loading

Sample benchmark output

go test -run=NONE -bench="." "./filters/openpolicyagent/opaauthorizerequest" -benchtime="1s" -benchmem
 
goarch: arm64
pkg: github.com/zalando/skipper/filters/openpolicyagent/opaauthorizerequest
cpu: Apple M1 Max
BenchmarkMinimalPolicy-10                          97986             10520 ns/op           24960 B/op        470 allocs/op
BenchmarkMinimalPolicyWithDecisionLogs-10          21968             60172 ns/op           75001 B/op       1252 allocs/op
BenchmarkAllowWithReqBody-10                      107958             23321 ns/op           34581 B/op        507 allocs/op
BenchmarkJwtValidation-10                          49458             22695 ns/op           60834 B/op        976 allocs/op
BenchmarkMinimalPolicyBundle-10                   136844              8104 ns/op           25278 B/op        482 allocs/op
PASS
ok      github.com/zalando/skipper/filters/openpolicyagent/opaauthorizerequest  11.050s

benchstat output for 10 runs

                                 │ test_run.txt │
                                │    sec/op    │
MinimalPolicy-10                    10.58µ ± 3%
MinimalPolicyWithDecisionLogs-10    54.60µ ± 8%
AllowWithReqBody-10                 24.04µ ± 5%
JwtValidation-10                    23.99µ ± 7%
MinimalPolicyBundle-10              8.386µ ± 3%
geomean                             19.46µ

                                │ test_run.txt │
                                │     B/op     │
MinimalPolicy-10                   24.37Ki ± 0%
MinimalPolicyWithDecisionLogs-10   73.42Ki ± 0%
AllowWithReqBody-10                33.77Ki ± 0%
JwtValidation-10                   59.40Ki ± 0%
MinimalPolicyBundle-10             24.69Ki ± 0%
geomean                            38.86Ki

                                │ test_run.txt │
                                │  allocs/op   │
MinimalPolicy-10                     470.0 ± 0%
MinimalPolicyWithDecisionLogs-10    1.254k ± 0%
AllowWithReqBody-10                  507.0 ± 0%
JwtValidation-10                     976.0 ± 0%
MinimalPolicyBundle-10               482.0 ± 0%
geomean                              675.4

mjungsbluth
mjungsbluth reviewed Feb 5, 2025
View reviewed changes
filters/openpolicyagent/opaauthorizerequest/benchmark_test.go
// To generate a bundle in .tar.gz format, use the following command:
// ./opa build -b <bundle_directory> -o <output_file.tar.gz>
// For example:
// ./opa build -b simple-opa-bundle -o simple-opa-bundle.tar.gz
Copy link
Collaborator

Choose a reason for hiding this comment

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

You should mention that you have a pre-made directory in this repo. I.e. if you include a line before that goes to that directory it would help

mjungsbluth
mjungsbluth reviewed Feb 5, 2025
View reviewed changes
filters/openpolicyagent/opaauthorizerequest/benchmark_test.go
}

b.ResetTimer()
b.RunParallel(func(pb *testing.PB) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Might be redundant to general docs for benchmarks but I'd recommend to add the command line with which you run this benchmark and how you control the parallelism. This would help anybody to create reproducible results

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mjungsbluth mjungsbluth mjungsbluth left review comments

@Pushpalanka Pushpalanka Awaiting requested review from Pushpalanka

Assignees
No one assigned
Labels
minor no risk changes, for example new filters refactor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mefarazath @mjungsbluth