Skip to content

Conversation

@cataphract
Copy link
Contributor

@cataphract cataphract commented Jan 16, 2026

Description

Passing integration and system-tests.

Further integration into sidecar and protocol changes pending.

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

@cataphract cataphract requested review from a team as code owners January 16, 2026 16:47
@cataphract cataphract marked this pull request as draft January 16, 2026 16:47
@datadog-datadog-prod-us1
Copy link

datadog-datadog-prod-us1 bot commented Jan 16, 2026

⚠️ Tests

Fix all issues with Cursor

⚠️ Warnings

❄️ 9 New flaky tests detected

    match against json request body() from com.datadog.appsec.php.integration.FrankenphpWorkerTests (Fix with Cursor)

match against json response body() from com.datadog.appsec.php.integration.FrankenphpWorkerTests (Datadog) (Fix with Cursor)
java.lang.AssertionError: 
Expected: {
  "triggers" : [ {
    "rule" : {
      "id" : "poison-in-json",
      "name" : "poison-in-json",
      "tags" : {
        "category" : "attack_attempt",
        "type" : "security_scanner"
      }
...
match against xml request body() from com.datadog.appsec.php.integration.FrankenphpWorkerTests (Datadog) (Fix with Cursor)
java.lang.AssertionError: 
Expected: {
  "triggers" : [ {
    "rule" : {
      "id" : "poison-in-xml",
      "name" : "poison-in-xml",
      "tags" : {
        "category" : "attack_attempt",
        "type" : "security_scanner"
      }
...
View all

🧪 1022 Tests failed

    testSearchPhpBinaries from integration.DDTrace\Tests\Integration\PHPInstallerTest (Fix with Cursor)

    testSimplePushAndProcess from laravel-58-test.DDTrace\Tests\Integrations\Laravel\V5_8\QueueTest (Fix with Cursor)

testSimplePushAndProcess from laravel-8x-test.DDTrace\Tests\Integrations\Laravel\V8_x\QueueTest (Datadog) (Fix with Cursor)
DDTrace\Tests\Integrations\Laravel\V8_x\QueueTest::testSimplePushAndProcess
Test code or tested code printed unexpected output: spanLinksTraceId: 697b5ced00000000efb0c65e29dc7984
tid: 697b5ced00000000
hexProcessTraceId: efb0c65e29dc7984
hexProcessSpanId: b801573f5675b3ea
processTraceId: 17271522678697392516
processSpanId: 13258974707500561386
View all
This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 718f800 | Docs | Datadog PR Page | Was this helpful? Give us feedback!

@codecov-commenter
Copy link

codecov-commenter commented Jan 16, 2026

Codecov Report

❌ Patch coverage is 49.72884% with 2132 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.72%. Comparing base (ed3089e) to head (718f800).
⚠️ Report is 35 commits behind head on master.

Files with missing lines Patch % Lines
appsec/helper-rust/src/client.rs 0.00% 575 Missing ⚠️
appsec/helper-rust/src/service.rs 51.02% 240 Missing ⚠️
appsec/helper-rust/src/service/waf_diag.rs 20.33% 192 Missing ⚠️
appsec/helper-rust/src/lib.rs 0.00% 186 Missing ⚠️
appsec/helper-rust/src/client/metrics.rs 0.00% 138 Missing ⚠️
appsec/helper-rust/src/rc.rs 73.73% 125 Missing ⚠️
appsec/helper-rust/src/telemetry/sidecar.rs 0.00% 94 Missing ⚠️
appsec/helper-rust/src/rc_notify.rs 0.00% 86 Missing ⚠️
appsec/helper-rust/src/service/limiter.rs 8.33% 66 Missing ⚠️
appsec/helper-rust/src/telemetry.rs 25.31% 59 Missing ⚠️
... and 11 more

❌ Your patch status has failed because the patch coverage (49.72%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage.

❗ There is a different number of reports uploaded between BASE (ed3089e) and HEAD (718f800). Click for more details.

HEAD has 16 uploads less than BASE
Flag BASE (ed3089e) HEAD (718f800)
16 0
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #3581       +/-   ##
===========================================
- Coverage   62.02%   49.72%   -12.30%     
===========================================
  Files         140       21      -119     
  Lines       13309     4241     -9068     
  Branches     1762        0     -1762     
===========================================
- Hits         8255     2109     -6146     
+ Misses       4265     2132     -2133     
+ Partials      789        0      -789     
Flag Coverage Δ
helper-rust-unit 49.72% <49.72%> (?)

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

Files with missing lines Coverage Δ
appsec/helper-rust/src/client/attributes.rs 97.76% <97.76%> (ø)
appsec/helper-rust/src/client/log.rs 0.00% <0.00%> (ø)
appsec/helper-rust/src/config.rs 74.39% <74.39%> (ø)
appsec/helper-rust/src/ffi.rs 79.69% <79.69%> (ø)
appsec/helper-rust/src/service/updateable_waf.rs 84.15% <84.15%> (ø)
appsec/helper-rust/src/server.rs 0.00% <0.00%> (ø)
appsec/helper-rust/src/service/config_manager.rs 67.93% <67.93%> (ø)
appsec/helper-rust/src/service/sampler.rs 88.31% <88.31%> (ø)
appsec/helper-rust/src/client/protocol.rs 89.72% <89.72%> (ø)
appsec/helper-rust/src/lock.rs 59.45% <59.45%> (ø)
... and 11 more

... and 140 files 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 ed3089e...718f800. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pr-commenter
Copy link

pr-commenter bot commented Jan 16, 2026

Benchmarks [ appsec ]

Benchmark execution time: 2026-01-28 20:08:34

Comparing candidate commit fc9100c in PR branch glopes/helper-rust with baseline commit 192a4a1 in branch glopes/appsec-curl.

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

@pr-commenter
Copy link

pr-commenter bot commented Jan 16, 2026

Benchmarks [ tracer ]

Benchmark execution time: 2026-01-28 22:06:48

Comparing candidate commit fc9100c in PR branch glopes/helper-rust with baseline commit 192a4a1 in branch glopes/appsec-curl.

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

scenario:ComposerTelemetryBench/benchTelemetryParsing

  • 🟩 execution_time [-1.653µs; -0.747µs] or [-13.116%; -5.931%]

scenario:ComposerTelemetryBench/benchTelemetryParsing-opcache

  • 🟩 execution_time [-1.702µs; -0.498µs] or [-12.247%; -3.580%]

scenario:EmptyFileBench/benchEmptyFileDdprof

  • 🟥 execution_time [+98.883µs; +248.277µs] or [+2.696%; +6.769%]

scenario:PDOBench/benchPDOOverhead

  • 🟥 execution_time [+8.045µs; +11.810µs] or [+3.681%; +5.403%]

scenario:PDOBench/benchPDOOverheadWithDBM

  • 🟥 execution_time [+6.999µs; +10.994µs] or [+3.206%; +5.036%]

scenario:PHPRedisBench/benchRedisOverhead

  • 🟥 execution_time [+31.840µs; +63.834µs] or [+3.756%; +7.531%]

@cataphract cataphract force-pushed the glopes/helper-rust branch 9 times, most recently from fb4432d to 8d1029e Compare January 18, 2026 03:49
@morrisonlevi
Copy link
Collaborator

morrisonlevi commented Jan 18, 2026

This PR is so large that github will only permit me to review it one file at a time. I didn't even know that was a thing! You're going to need to break it down into a series of smaller PRs, probably.

@cataphract cataphract force-pushed the glopes/helper-rust branch 2 times, most recently from a218cd6 to 332fd93 Compare January 20, 2026 12:39
@bwoebi
Copy link
Collaborator

bwoebi commented Jan 20, 2026

@morrisonlevi I've had success for very big PRs with the PHPStorm/CLion github integrations in the past. Doesn't matter for small PRs, but can definitely recommend it for extra-large PRs :-)

cataphract and others added 3 commits January 29, 2026 09:55
Changes the request_exec message format from [rasp_rule, data] to
[data, options_map]. The options map supports:
- rasp_rule: string (same as before)
- subctx_id: optional string (accepted but ignored)
- subctx_last_call: optional bool (accepted but ignored)

This prepares the protocol for curl/subcontext support while maintaining
backwards compatibility. The subcontext fields are accepted by the
protocol but not implemented in the business logic.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Upgrade testcontainers
- Add some assertions
- Better debug output for metrics
@cataphract cataphract changed the base branch from glopes/appsec-curl to master January 29, 2026 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants