Skip to content

Conversation

@marc-gr
Copy link
Contributor

@marc-gr marc-gr commented Nov 8, 2022

  • This adds a report benchmark subcommand that will allow to compare benchmark results between benchmark runs.
  • It also changes the format from xUnit to JSON to leverage kibana reporting (xunit was initially used in hopes to use a jenkins plugin to parse it, but we decided to not use it at the end because lack of support)

Relates to #992

It will generate a Markdown report that will be used as part of CI results in integrations:

elastic-package report --fail-on-missing benchmark --source ./build/benchmark-results --target ./build/benchmark-results1 --report-output=stdout
elastic-package has been installed.
Generate benchmark reports

🚀 Benchmarks report

Package aws 👍(0) 💚(0) 💔(13)

Expand to view
Data stream Previous EPS New EPS Diff (%) Result
securityhub_findings 805.15 681.66 -123.49 (-15.34%) 💔
securityhub_insights 1250 961.54 -288.46 (-23.08%) 💔
vpcflow 8130.08 6369.43 -1760.65 (-21.66%) 💔
waf 11363.64 6666.67 -4696.97 (-41.33%) 💔
cloudfront_logs 8000 3322.26 -4677.74 (-58.47%) 💔
cloudtrail 4291.85 2652.52 -1639.33 (-38.2%) 💔
cloudwatch_logs 1e+06 500000 -500000 (-50%) 💔
ec2_logs 76923.08 55555.56 -21367.52 (-27.78%) 💔
elb_logs 10309.28 6369.43 -3939.85 (-38.22%) 💔
firewall_logs 3731.34 2958.58 -772.76 (-20.71%) 💔
route53_public_logs 18867.92 10752.69 -8115.23 (-43.01%) 💔
route53_resolver_logs 12195.12 4950.5 -7244.62 (-59.41%) 💔
s3access 6993.01 5102.04 -1890.97 (-27.04%) 💔

To see the full report comment with /test benchmark fullreport

@elasticmachine
Copy link
Collaborator

elasticmachine commented Nov 8, 2022

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-11-11T10:52:20.230+0000

  • Duration: 32 min 35 sec

Test stats 🧪

Test Results
Failed 0
Passed 863
Skipped 0
Total 863

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

@marc-gr marc-gr requested review from jsoriano and v1v November 8, 2022 13:24
@elasticmachine
Copy link
Collaborator

elasticmachine commented Nov 8, 2022

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (35/35) 💚
Files 66.923% (87/130) 👍 0.256
Classes 61.622% (114/185) 👍 0.209
Methods 47.865% (370/773) 👍 0.203
Lines 30.726% (3319/10802) 👍 0.003
Conditionals 100.0% (0/0) 💚

Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

In general it LGTM, added some small suggestions. Thanks!

- Keep an xml format
- Rename source -> new, target -> old
- Add report destination path flag
- Generate report on benchmark test
@marc-gr marc-gr requested a review from jsoriano November 11, 2022 09:52
Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

A couple of questions/suggestions, but LGTM.

Comment on lines 70 to 78
rm -rf "${OLDPWD}/build/benchmark-results"
elastic-package benchmark -v --report-format xml --report-output file --fail-on-missing

rm -rf "${OLDPWD}/build/benchmark-results-old"
mv "${OLDPWD}/build/benchmark-results" "${OLDPWD}/build/benchmark-results-old"

elastic-package benchmark -v --report-format json --report-output file --fail-on-missing

elastic-package report --fail-on-missing benchmark --new ${OLDPWD}/build/benchmark-results --old ${OLDPWD}/build/benchmark-results-old --threshold 1
Copy link
Member

@jsoriano jsoriano Nov 11, 2022

Choose a reason for hiding this comment

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

Nit. Could we also take the opportunity to test --report-output-path here?

Suggested change
rm -rf "${OLDPWD}/build/benchmark-results"
elastic-package benchmark -v --report-format xml --report-output file --fail-on-missing
rm -rf "${OLDPWD}/build/benchmark-results-old"
mv "${OLDPWD}/build/benchmark-results" "${OLDPWD}/build/benchmark-results-old"
elastic-package benchmark -v --report-format json --report-output file --fail-on-missing
elastic-package report --fail-on-missing benchmark --new ${OLDPWD}/build/benchmark-results --old ${OLDPWD}/build/benchmark-results-old --threshold 1
rm -rf "${OLDPWD}/build/benchmark-results" "${OLDPWD}/build/benchmark-results-old"
elastic-package benchmark -v --report-format xml --report-output file --report-output-path "${OLDPWD}/build/benchmark-results-old" --fail-on-missing
elastic-package benchmark -v --report-format json --report-output file --fail-on-missing
elastic-package report --fail-on-missing benchmark --new ${OLDPWD}/build/benchmark-results --old ${OLDPWD}/build/benchmark-results-old --threshold 1

Copy link
Member

Choose a reason for hiding this comment

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

Umm, tests still don't use --report-output-path? 🤔

@marc-gr marc-gr requested a review from jsoriano November 11, 2022 10:52
@marc-gr marc-gr merged commit b2e3cac into elastic:main Nov 11, 2022
@marc-gr marc-gr deleted the bench-comparisons branch November 11, 2022 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants