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.

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

Add FrankenPHP to the list of recognised/supported SAPIs #2523

Merged
merged 6 commits into from
Mar 1, 2024

Conversation

realFlowControl
Copy link
Member

@realFlowControl realFlowControl commented Feb 19, 2024

Description

PROF-9184 / #2070

This PR aimes to add support for FrankenPHP SAPI, now that ZTS support exists. In detail this means:

  • adding FrankenPHP to the list of detected SAPIs
  •  adding frankenphp_handle_request() (worker mode) to the idle times in timeline (analogue to rshutdown -> rinit in PHP-FPM)
image

Note: Screenshot is from the FrankenPHP Symfony Demo application in the reliability environment using worker mode

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

@github-actions github-actions bot added profiling Relates to the Continuous Profiler tracing labels Feb 19, 2024
@pr-commenter
Copy link

pr-commenter bot commented Feb 19, 2024

Benchmarks

Benchmark execution time: 2024-03-01 10:55:50

Comparing candidate commit ac127a6 in PR branch florian/frankenphp with baseline commit 552b0ef in branch master.

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

scenario:TraceSerializationBench/benchSerializeTrace

  • 🟥 execution_time [+4.501µs; +8.399µs] or [+2.447%; +4.566%]

@bwoebi
Copy link
Collaborator

bwoebi commented Feb 29, 2024

If we're going to support frankenphp in the profiler, the tracer probably should too.

I think it's not too different from other sapis, so probably the tracer should just support it out of the box once added to components/sapi/sapi.c and dd_is_compatible_sapi in ddtrace.c.

@realFlowControl realFlowControl requested a review from a team as a code owner February 29, 2024 10:06
@bwoebi bwoebi changed the title feat(profiling) add FrankenPHP to the list of recognised/supported SAPIs Add FrankenPHP to the list of recognised/supported SAPIs Feb 29, 2024
Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>
@codecov-commenter
Copy link

codecov-commenter commented Feb 29, 2024

Codecov Report

Merging #2523 (60b8ded) into master (552b0ef) will increase coverage by 2.18%.
Report is 1 commits behind head on master.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2523      +/-   ##
============================================
+ Coverage     76.55%   78.74%   +2.18%     
  Complexity      267      267              
============================================
  Files           138      112      -26     
  Lines         17457    13486    -3971     
  Branches        976        0     -976     
============================================
- Hits          13364    10619    -2745     
+ Misses         3573     2867     -706     
+ Partials        520        0     -520     
Flag Coverage Δ
appsec-extension ?
tracer-extension 78.69% <100.00%> (+<0.01%) ⬆️
tracer-integrations 79.49% <ø> (ø)

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

Files Coverage Δ
components/sapi/sapi.c 80.00% <ø> (ø)
ext/ddtrace.c 73.03% <100.00%> (+0.01%) ⬆️

... and 26 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 552b0ef...60b8ded. Read the comment docs.

ext/ddtrace.c Show resolved Hide resolved
ext/ddtrace.c Outdated Show resolved Hide resolved
realFlowControl and others added 2 commits March 1, 2024 11:20
Co-authored-by: Levi Morrison <levi.morrison@datadoghq.com>
@realFlowControl realFlowControl merged commit d954ded into master Mar 1, 2024
629 of 630 checks passed
@realFlowControl realFlowControl deleted the florian/frankenphp branch March 1, 2024 15:39
@github-actions github-actions bot added this to the 0.99.0 milestone Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
profiling Relates to the Continuous Profiler tracing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants