Add appsec benchmarks #2791

estringana · 2024-08-14T14:58:52Z

Description

This pull requests adds:

Benchmarking with appsec loaded
Logs of all benchmarking executions

APPSEC-54443

Reviewer checklist

Test coverage seems ok.
Appropriate labels assigned.

codecov-commenter · 2024-08-14T15:07:02Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.07%. Comparing base (31382ec) to head (95aa3f3).
Report is 15 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #2791      +/-   ##
============================================
+ Coverage     81.05%   81.07%   +0.01%     
  Complexity     2517     2517              
============================================
  Files           146      146              
  Lines         14654    14654              
============================================
+ Hits          11878    11880       +2     
+ Misses         2776     2774       -2

Flag	Coverage Δ
tracer-extension	`78.18% <ø> (ø)`
tracer-php	`82.27% <ø> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 1 file with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 31382ec...95aa3f3. Read the comment docs.

pr-commenter · 2024-09-02T14:10:26Z

Benchmarks [ profiler ]

Benchmark execution time: 2024-09-02 14:10:17

Comparing candidate commit 9db79ba in PR branch estringana/add-appsec-benchmarks with baseline commit 05ae627 in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 27 metrics, 9 unstable metrics.

pr-commenter · 2024-09-02T14:52:08Z

Benchmarks [ tracer ]

Benchmark execution time: 2024-09-25 09:08:18

Comparing candidate commit 95aa3f3 in PR branch estringana/add-appsec-benchmarks with baseline commit 31382ec in branch master.

Found 2 performance improvements and 9 performance regressions! Performance is the same for 167 metrics, 0 unstable metrics.

scenario:EmptyFileBench/benchEmptyFileBaseline-opcache

🟥 execution_time [+103.079µs; +235.101µs] or [+3.494%; +7.968%]

scenario:EmptyFileBench/benchEmptyFileOverhead

🟩 execution_time [-185.295µs; -67.985µs] or [-5.855%; -2.148%]

scenario:LaravelBench/benchLaravelBaseline

🟥 execution_time [+7.138ms; +7.310ms] or [+230.113%; +235.670%]

scenario:LaravelBench/benchLaravelBaseline-opcache

🟥 execution_time [+7.287ms; +7.432ms] or [+233.886%; +238.513%]

scenario:LaravelBench/benchLaravelOverhead

🟥 execution_time [+7.781ms; +7.992ms] or [+235.894%; +242.277%]

scenario:LaravelBench/benchLaravelOverhead-opcache

🟥 execution_time [+7.837ms; +8.052ms] or [+231.866%; +238.216%]

scenario:LogsInjectionBench/benchLogsInfoBaseline-opcache

🟩 execution_time [-215.290ns; -166.310ns] or [-12.663%; -9.782%]

scenario:WordPressBench/benchWordPressBaseline

🟥 mem_peak [+512.544KB; +512.544KB] or [+7.068%; +7.068%]

scenario:WordPressBench/benchWordPressBaseline-opcache

🟥 mem_peak [+198.464KB; +198.464KB] or [+4.415%; +4.415%]

scenario:WordPressBench/benchWordPressOverhead

🟥 mem_peak [+2.944MB; +2.944MB] or [+61.087%; +61.087%]

scenario:WordPressBench/benchWordPressOverhead-opcache

🟥 mem_peak [+2.688MB; +2.688MB] or [+134.034%; +134.034%]

pr-commenter · 2024-09-02T15:04:18Z

Benchmarks [ appsec ]

Benchmark execution time: 2024-09-11 09:53:43

Comparing candidate commit ccd86d5 in PR branch estringana/add-appsec-benchmarks with baseline commit 05ae627 in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 178 metrics, 0 unstable metrics.

ddyurchenko · 2024-09-02T15:17:58Z

About - Benchmarks [ tracer ]. Something is completely not right with WordPressBench/benchWordPressOverhead, should not experience such big perf difference. Other 4 scenarios seems to be what usually flaky / not super precise benchmarks look like.

I can increase flakiness threshold for you from 2% to 5%, so the small flaky results are hidden. For most teams we already use 5%, ensuring 2% threshold is possible, but would require quite some engineering time (and in general not sure if it is useful for you -- will you address a regression of size 2% anyhow?).

ddyurchenko · 2024-09-10T15:57:38Z

I keep wondering what is happening with scenario:WordPressBench/benchWordPressOverhead. It is consistently 80% faster across 5 last runs. It can be that we do something wrong in estringana/add-appsec branch in benchmarking-platform repo during candidate or baseline run.

.gitlab/benchmarks.yml

pr-commenter · 2024-09-17T11:24:57Z

Benchmarks

Benchmark execution time: 2024-09-20 08:45:44

Comparing candidate commit 339adfc in PR branch estringana/add-appsec-benchmarks with baseline commit 31382ec in branch master.

Found 1 performance improvements and 8 performance regressions! Performance is the same for 168 metrics, 1 unstable metrics.

scenario:ContextPropagationBench/benchInject64Bit

🟥 execution_time [+158.425ns; +471.575ns] or [+2.014%; +5.994%]

scenario:EmptyFileBench/benchEmptyFileOverhead-opcache

🟩 execution_time [-236.273µs; -105.307µs] or [-7.039%; -3.137%]

scenario:LaravelBench/benchLaravelBaseline

🟥 execution_time [+7.176ms; +7.392ms] or [+226.330%; +233.156%]

scenario:LaravelBench/benchLaravelOverhead

🟥 execution_time [+7.756ms; +7.986ms] or [+230.602%; +237.461%]

scenario:LaravelBench/benchLaravelOverhead-opcache

🟥 execution_time [+7.787ms; +8.009ms] or [+226.349%; +232.819%]

scenario:WordPressBench/benchWordPressBaseline

🟥 mem_peak [+512.896KB; +512.896KB] or [+7.073%; +7.073%]

scenario:WordPressBench/benchWordPressBaseline-opcache

🟥 mem_peak [+198.672KB; +198.672KB] or [+4.419%; +4.419%]

scenario:WordPressBench/benchWordPressOverhead

🟥 mem_peak [+2.945MB; +2.945MB] or [+61.093%; +61.093%]

scenario:WordPressBench/benchWordPressOverhead-opcache

🟥 mem_peak [+2.689MB; +2.689MB] or [+134.039%; +134.039%]

estringana · 2024-09-20T15:35:06Z

I keep wondering what is happening with scenario:WordPressBench/benchWordPressOverhead. It is consistently 80% faster across 5 last runs. It can be that we do something wrong in estringana/add-appsec branch in benchmarking-platform repo during candidate or baseline run.

This is fixed. Basically, there was error connecting to the wordpress database. I think this problem was there before but can't understand why it was not spotted

estringana · 2024-09-20T15:37:24Z

Makefile

@@ -1112,7 +1133,7 @@ test_distributed_tracing_coverage:
 test_metrics: global_test_run_dependencies
 	$(call run_tests,--testsuite=metrics $(TESTS))

-benchmarks_run_dependencies: global_test_run_dependencies tests/Frameworks/Symfony/Version_5_2/composer.lock-php$(PHP_MAJOR_MINOR) tests/Frameworks/Laravel/Version_8_x/composer.lock-php$(PHP_MAJOR_MINOR) tests/Benchmarks/composer.lock-php$(PHP_MAJOR_MINOR)
+benchmarks_run_dependencies: global_test_run_dependencies tests/Frameworks/Symfony/Version_5_2/composer.lock-php$(PHP_MAJOR_MINOR) tests/Frameworks/Laravel/Version_10_x/composer.lock-php$(PHP_MAJOR_MINOR) tests/Benchmarks/composer.lock-php$(PHP_MAJOR_MINOR)


This is the reason why Laravel performance has increased. Basically, the autoloader didn't exists and all responses were fast because they were 500x

estringana · 2024-09-23T07:52:18Z

.gitlab/benchmarks.yml

@@ -13,7 +13,7 @@ variables:
  timeout: 1h
  script:
    - export ARTIFACTS_DIR="$(pwd)/reports" && (mkdir "${ARTIFACTS_DIR}" || :)
-    - git clone --branch dd-trace-php https://gitlab-ci-token:${CI_JOB_TOKEN}@gitlab.ddbuild.io/DataDog/benchmarking-platform /platform && cd /platform


I will change this once the other branch is merged and this approved

estringana · 2024-09-23T07:53:18Z

docker-compose.yml

@@ -83,6 +83,7 @@ services:
  '8.3-buster': { <<: *linux_php_service, image: 'datadog/dd-trace-ci:php-8.3_buster' }
  'php-master-buster': { <<: *linux_php_service, image: 'datadog/dd-trace-ci:php-master_buster' }
  # --- Bookworm ---
+  '8.2-bookworm': { <<: *linux_php_service, image: 'datadog/dd-trace-ci:php-8.2_bookworm-3' }


Not really needed but I added it here because that's the image I use to test benchmarks. Appsec is not compiling on buster versions

estringana · 2024-09-23T07:54:06Z

tests/Benchmarks/Integrations/WordPressBench.php

    }

    public function afterMethod()
    {
        $this->TearDownAfterClass();
    }

-    public function enableWordPressTracing()


I have changed this so now we ensure the database is created before any test

estringana · 2024-09-23T07:54:49Z

tests/bootstrap_common.php

@@ -51,3 +51,67 @@ function missing_ddtrace_class_fatal_autoloader($class)

 spl_autoload_register('\DDTrace\Tests\missing_ddtrace_class_fatal_autoloader', true);

+$phpunitVersionParts = class_exists('\PHPUnit\Runner\Version')


The next three files related with bootstrap are changed because benchmarks were loading appsec Mock. This changes avoid that happening

Basically I copied/pasted code from one file to another. All the code was already there but in different files

tests/bootstrap_phpunit.php

Makefile

PROFeNoM

LGTM

Reminder: Don't forget to change the branch in .gitlab/benchmarks 👍

estringana force-pushed the estringana/add-appsec-benchmarks branch 2 times, most recently from 7955e0b to 979c3ad Compare August 28, 2024 14:13

estringana force-pushed the estringana/add-appsec-benchmarks branch from 979c3ad to bf6e1ef Compare August 30, 2024 13:49

estringana marked this pull request as ready for review September 2, 2024 12:40

estringana requested review from a team as code owners September 2, 2024 12:40

estringana requested review from PROFeNoM and ddyurchenko September 2, 2024 12:40

ddyurchenko approved these changes Sep 2, 2024

View reviewed changes

estringana force-pushed the estringana/add-appsec-benchmarks branch 2 times, most recently from 0813939 to 19a0904 Compare September 12, 2024 10:00

estringana commented Sep 16, 2024

View reviewed changes

.gitlab/benchmarks.yml Outdated Show resolved Hide resolved

estringana and others added 12 commits September 16, 2024 12:41

Add basic benchmarks skeleton

ebd0ab7

Point to my ongoing branch on benchmarks

c0e0ac5

Add some changes so benchmarks can run

24dab61

tweak: Ensure that appsec and tracer benchmarks have separate filenames

a93a97d

Install appsec extension and helper

bd11d00

Check debbuging logging files

be9e053

Output php modules installed

5883ce0

Enable appsec cli on benchmarks

3e88984

Update rules configuration

8ad0e7c

Remove debug mode

e8b8547

Avoid failure when no logs generated

d2bffd1

Add 8.2 bookworm

71c27b9

estringana force-pushed the estringana/add-appsec-benchmarks branch from 46f2665 to 9ecca43 Compare September 16, 2024 10:42

Point to temporary branch

7f15058

estringana force-pushed the estringana/add-appsec-benchmarks branch from f89eed7 to 7f15058 Compare September 16, 2024 15:58

estringana added 2 commits September 17, 2024 09:31

Fix tea tests

89c08fb

Remove log files after printing them

620b5d4

estringana added 4 commits September 18, 2024 09:51

Display php errors if any

9122e08

TEMP

da4f67a

Generate logs

415a3ae

Point benchmarks to the right version of Laravel

f38dddc

estringana force-pushed the estringana/add-appsec-benchmarks branch from 6b533a9 to 8293cfd Compare September 19, 2024 15:10

Fix wordpress database setup in benchmark tests

339adfc

estringana force-pushed the estringana/add-appsec-benchmarks branch from 8293cfd to 339adfc Compare September 20, 2024 08:20

Revert changes to makefile

aaec12b

estringana force-pushed the estringana/add-appsec-benchmarks branch from 5d2d179 to aaec12b Compare September 20, 2024 11:02

Avoid loading appsec mock on benchmarks

0b8f053

estringana commented Sep 20, 2024

View reviewed changes

estringana commented Sep 23, 2024

View reviewed changes

PROFeNoM reviewed Sep 23, 2024

View reviewed changes

tests/bootstrap_phpunit.php Show resolved Hide resolved

PROFeNoM reviewed Sep 23, 2024

View reviewed changes

Makefile Show resolved Hide resolved

Fix namespaces on bootstrap files

ffb0d10

PROFeNoM approved these changes Sep 24, 2024

View reviewed changes

Revert target benchmark branch

95aa3f3

estringana merged commit f117e22 into master Sep 25, 2024
642 of 646 checks passed

estringana deleted the estringana/add-appsec-benchmarks branch September 25, 2024 09:49

github-actions bot added this to the 1.4.0 milestone Sep 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add appsec benchmarks #2791

Add appsec benchmarks #2791

estringana commented Aug 14, 2024 •

edited

Loading

codecov-commenter commented Aug 14, 2024 •

edited

Loading

pr-commenter bot commented Sep 2, 2024

pr-commenter bot commented Sep 2, 2024 •

edited

Loading

pr-commenter bot commented Sep 2, 2024 •

edited

Loading

ddyurchenko commented Sep 2, 2024 •

edited

Loading

ddyurchenko commented Sep 10, 2024

pr-commenter bot commented Sep 17, 2024 •

edited

Loading

estringana commented Sep 20, 2024

estringana Sep 20, 2024

estringana Sep 23, 2024

estringana Sep 23, 2024

estringana Sep 23, 2024

estringana Sep 23, 2024

estringana Sep 23, 2024

PROFeNoM left a comment

		@@ -51,3 +51,67 @@ function missing_ddtrace_class_fatal_autoloader($class)

		spl_autoload_register('\DDTrace\Tests\missing_ddtrace_class_fatal_autoloader', true);

		$phpunitVersionParts = class_exists('\PHPUnit\Runner\Version')

Add appsec benchmarks #2791

Add appsec benchmarks #2791

Conversation

estringana commented Aug 14, 2024 • edited Loading

Description

Reviewer checklist

codecov-commenter commented Aug 14, 2024 • edited Loading

Codecov Report

pr-commenter bot commented Sep 2, 2024

Benchmarks [ profiler ]

pr-commenter bot commented Sep 2, 2024 • edited Loading

Benchmarks [ tracer ]

scenario:EmptyFileBench/benchEmptyFileBaseline-opcache

scenario:EmptyFileBench/benchEmptyFileOverhead

scenario:LaravelBench/benchLaravelBaseline

scenario:LaravelBench/benchLaravelBaseline-opcache

scenario:LaravelBench/benchLaravelOverhead

scenario:LaravelBench/benchLaravelOverhead-opcache

scenario:LogsInjectionBench/benchLogsInfoBaseline-opcache

scenario:WordPressBench/benchWordPressBaseline

scenario:WordPressBench/benchWordPressBaseline-opcache

scenario:WordPressBench/benchWordPressOverhead

scenario:WordPressBench/benchWordPressOverhead-opcache

pr-commenter bot commented Sep 2, 2024 • edited Loading

Benchmarks [ appsec ]

ddyurchenko commented Sep 2, 2024 • edited Loading

ddyurchenko commented Sep 10, 2024

pr-commenter bot commented Sep 17, 2024 • edited Loading

Benchmarks

scenario:ContextPropagationBench/benchInject64Bit

scenario:EmptyFileBench/benchEmptyFileOverhead-opcache

scenario:LaravelBench/benchLaravelBaseline

scenario:LaravelBench/benchLaravelOverhead

scenario:LaravelBench/benchLaravelOverhead-opcache

scenario:WordPressBench/benchWordPressBaseline

scenario:WordPressBench/benchWordPressBaseline-opcache

scenario:WordPressBench/benchWordPressOverhead

scenario:WordPressBench/benchWordPressOverhead-opcache

estringana commented Sep 20, 2024

estringana Sep 20, 2024

Choose a reason for hiding this comment

estringana Sep 23, 2024

Choose a reason for hiding this comment

estringana Sep 23, 2024

Choose a reason for hiding this comment

estringana Sep 23, 2024

Choose a reason for hiding this comment

estringana Sep 23, 2024

Choose a reason for hiding this comment

estringana Sep 23, 2024

Choose a reason for hiding this comment

PROFeNoM left a comment

Choose a reason for hiding this comment

estringana commented Aug 14, 2024 •

edited

Loading

codecov-commenter commented Aug 14, 2024 •

edited

Loading

pr-commenter bot commented Sep 2, 2024 •

edited

Loading

pr-commenter bot commented Sep 2, 2024 •

edited

Loading

ddyurchenko commented Sep 2, 2024 •

edited

Loading

pr-commenter bot commented Sep 17, 2024 •

edited

Loading