Skip to content

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

Merged
realFlowControl merged 6 commits intomasterfrom
florian/frankenphp
Mar 1, 2024
Merged

Add FrankenPHP to the list of recognised/supported SAPIs#2523
realFlowControl merged 6 commits intomasterfrom
florian/frankenphp

Conversation

@realFlowControl
Copy link
Copy Markdown
Member

@realFlowControl realFlowControl commented Feb 19, 2024 •
edited
Loading

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
Copy Markdown

pr-commenter Bot commented Feb 19, 2024 •
edited
Loading

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%]

@realFlowControl realFlowControl mentioned this pull request Feb 20, 2024
Closed
@realFlowControl realFlowControl marked this pull request as ready for review February 22, 2024 16:33
@realFlowControl realFlowControl requested a review from a team as a code owner February 22, 2024 16:33
@bwoebi
Copy link
Copy Markdown
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
@bwoebi bwoebi force-pushed the florian/frankenphp branch from 9bfadaa to 60b8ded Compare February 29, 2024 13:14
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Feb 29, 2024 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.55%. Comparing base (552b0ef) to head (ac127a6).
⚠️ Report is 933 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #2523   +/-   ##
=========================================
  Coverage     76.55%   76.55%           
  Complexity      267      267           
=========================================
  Files           138      138           
  Lines         17457    17458    +1     
  Branches        976      976           
=========================================
+ Hits          13364    13365    +1     
  Misses         3573     3573           
  Partials        520      520
Flag Coverage Δ
appsec-extension 69.13% <ø> (ø)
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 with missing lines Coverage Δ
components/sapi/sapi.c 80.00% <ø> (ø)
ext/ddtrace.c 73.03% <100.00%> (+0.01%) ⬆️

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...ac127a6. 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.

morrisonlevi
morrisonlevi approved these changes Feb 29, 2024
View reviewed changes
Comment thread ext/ddtrace.c
Comment thread ext/ddtrace.c Outdated
realFlowControl and others added 2 commits March 1, 2024 11:20
@realFlowControl realFlowControl mentioned this pull request Mar 1, 2024
Closed
@realFlowControl realFlowControl merged commit d954ded into master Mar 1, 2024
@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

Reviewers

@morrisonlevi morrisonlevi morrisonlevi approved these changes

Assignees

No one assigned

Labels

profiling Relates to the Continuous Profiler tracing

Projects

None yet

Milestone

0.99.0

Development

Successfully merging this pull request may close these issues.

4 participants

@realFlowControl @bwoebi @codecov-commenter @morrisonlevi