DataDog / dd-trace-php

Datadog PHP Clients
https://docs.datadoghq.com/tracing/setup/php
Other
501 stars 155 forks source link

Fix appsec tests in 8.1-8.3 #2974

Open cataphract opened 12 hours ago

cataphract commented 12 hours ago

Description

The use of pipe() creates an extra file descriptor which makes the helper unable to find the correct pipe file descriptor. While this could perhaps be improved by telling the helper explicitly the id of the correct file descriptor, it's probably better to swap write() calls of reading invalid addresses with mincore().

See https://github.com/DataDog/dd-trace-php/pull/2942 , which broke the tests

Reviewer checklist

codecov-commenter commented 12 hours ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 74.82%. Comparing base (35d1665) to head (9fd6b9e). Report is 1 commits behind head on master.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/DataDog/dd-trace-php/pull/2974/graphs/tree.svg?width=650&height=150&src=pr&token=eXio8H7vwF&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DataDog)](https://app.codecov.io/gh/DataDog/dd-trace-php/pull/2974?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DataDog) ```diff @@ Coverage Diff @@ ## master #2974 +/- ## ========================================= Coverage 74.82% 74.82% Complexity 2741 2741 ========================================= Files 110 110 Lines 10863 10863 ========================================= Hits 8128 8128 Misses 2735 2735 ``` | [Flag](https://app.codecov.io/gh/DataDog/dd-trace-php/pull/2974/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DataDog) | Coverage Δ | | |---|---|---| | [tracer-php](https://app.codecov.io/gh/DataDog/dd-trace-php/pull/2974/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DataDog) | `74.82% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DataDog#carryforward-flags-in-the-pull-request-comment) to find out more. ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/DataDog/dd-trace-php/pull/2974?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DataDog). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DataDog) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/DataDog/dd-trace-php/pull/2974?dropdown=coverage&src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DataDog). Last update [35d1665...9fd6b9e](https://app.codecov.io/gh/DataDog/dd-trace-php/pull/2974?dropdown=coverage&src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DataDog). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DataDog).

🚨 Try these New Features:

Anilm3 commented 11 hours ago

The use of pipe() creates an extra file descriptor which makes the helper unable to find the correct pipe file descriptor. While this could perhaps be improved by telling the helper explicitly the id of the correct file descriptor, it's probably better to swap write() calls of reading invalid addresses with mincore().

I suppose you meant mock helper.

pr-commenter[bot] commented 11 hours ago

Benchmarks [ tracer ]

Benchmark execution time: 2024-11-22 16:45:22

Comparing candidate commit 9fd6b9e4d9fdb70e6d52ce2676ab5a15144ac75c in PR branch glopes/fix-appsec-test-pipe with baseline commit 35d1665166dde8c5017bccb7f7d7233614b27dda in branch master.

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

scenario:TraceSerializationBench/benchSerializeTrace