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

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 performance with concurrency)
  • Include benchmarks for minimal policy with/without OPA decision logging
  • Add reference benchmark to run with a given policy bundle

@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

@mefarazath mefarazath force-pushed the opa-benchmark-restructure branch from 63534fe to d01b9cf Compare February 6, 2025 11:59
@mjungsbluth
Copy link
Collaborator

👍

Refactor: Improve OPA authorization filter benchmarks
f553645 
- Move benchmarks to a separate file
- Add parallel execution to benchmarks
- Includ benchmarks for minimal policy with/without decision logging
- Add reference benchmark running with a policy bundle

Signed-off-by: Farasath Ahamed <[email protected]>
@mefarazath mefarazath force-pushed the opa-benchmark-restructure branch from 8d1b96f to f553645 Compare February 7, 2025 13:44
@MustafaSaber
Copy link
Member

👍

1 similar comment
@Pushpalanka
Copy link
Collaborator

👍

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

@Pushpalanka Pushpalanka Awaiting requested review from Pushpalanka

@mjungsbluth mjungsbluth Awaiting requested review from mjungsbluth

@AlexanderYastrebov AlexanderYastrebov Awaiting requested review from AlexanderYastrebov

@szuecs szuecs Awaiting requested review from szuecs

@MustafaSaber MustafaSaber Awaiting requested review from MustafaSaber

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.
5 participants
@mefarazath @mjungsbluth @MustafaSaber @Pushpalanka @szuecs