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

feat(agent): add support for CakePHP 4.x and 5.x #983

Open
wants to merge 23 commits into
base: dev
Choose a base branch
from

Conversation

hahuja2
Copy link
Contributor

@hahuja2 hahuja2 commented Oct 30, 2024

This PR does the following:

  • Modifies how the action name is retrieved, so that it is compatible with CakePHP 4.x and 5.x
  • Adds correct detection file for CakePHP 4.x and 5.x
  • Adds call to nr_txn_suggest_package_supportability_metric in nr_cakephp_enable

@newrelic-php-agent-bot
Copy link

newrelic-php-agent-bot commented Oct 30, 2024

Test Suite Status Result
Multiverse 7/7 passing
SOAK 54/56 passing

@codecov-commenter
Copy link

codecov-commenter commented Oct 30, 2024

Codecov Report

Attention: Patch coverage is 11.42857% with 31 lines in your changes missing coverage. Please review.

Project coverage is 78.68%. Comparing base (727812d) to head (6540e70).
Report is 1 commits behind head on dev.

Files with missing lines Patch % Lines
agent/fw_cakephp.c 11.42% 31 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #983      +/-   ##
==========================================
+ Coverage   78.54%   78.68%   +0.14%     
==========================================
  Files         196      196              
  Lines       27103    27047      -56     
==========================================
- Hits        21287    21281       -6     
+ Misses       5816     5766      -50     
Flag Coverage Δ
agent-for-php-7.2 78.69% <11.42%> (+0.14%) ⬆️
agent-for-php-7.3 78.71% <11.42%> (+0.14%) ⬆️
agent-for-php-7.4 78.41% <11.42%> (+0.14%) ⬆️
agent-for-php-8.0 78.43% <11.42%> (+0.14%) ⬆️
agent-for-php-8.1 78.42% <11.42%> (+0.14%) ⬆️
agent-for-php-8.2 78.01% <11.42%> (+0.14%) ⬆️
agent-for-php-8.3 78.01% <11.42%> (+0.14%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

Comment on lines 321 to 322
NR_PSTR("Cake\\Core\\Exception\\CakeException::__construct"),
nr_cakephp_problem_4, NULL, NULL);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is needed. When Observer API is used to hook into Zend Engine, the agent should do the right thing and record the error on the transaction even though framework installs its own exception handler. See Yii's exception instrumentation here for reference.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have experimented with the above and noticed that the error is not recorded as a traced error. Instead, I have added code that retrieves the exception caught by CakePHP's error handler and reports it as a traced error: 86108b5

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving information for posterity:

The agent correctly detects that CakePHP registers its default exception handler Cake\Error\ExceptionTrap::handleException (CakePHP 4.x, CakePHP 5.x). However, that exception handler is used only for uncaught exceptions. In a CakePHP application, exceptions thrown in controller actions are caught in Cake\Error\Middleware\ErrorHandlerMiddleware::process method (CakePHP 4.x, CakePHP 5.x). Cake\Error\Middleware\ErrorHandlerMiddleware is part of CakePHP application's default middleware stack (CakePHP 4.x, CakePHP 5.x) so wrapping Cake\Error\Middleware\ErrorHandlerMiddleware::handleException is the right approach for the agent to record error events for exceptions thrown by controller actions in standard CakePHP applications.

@lavarou lavarou added this to the next-release milestone Nov 12, 2024
agent/fw_hooks.h Outdated Show resolved Hide resolved
agent/fw_cakephp.c Outdated Show resolved Hide resolved
agent/fw_cakephp.c Outdated Show resolved Hide resolved
agent/fw_cakephp.c Outdated Show resolved Hide resolved
agent/fw_cakephp.c Outdated Show resolved Hide resolved
agent/fw_cakephp.c Outdated Show resolved Hide resolved
agent/fw_cakephp.c Outdated Show resolved Hide resolved
agent/fw_cakephp.c Outdated Show resolved Hide resolved
ZNeumann
ZNeumann previously approved these changes Nov 20, 2024
agent/fw_cakephp.c Outdated Show resolved Hide resolved
agent/fw_cakephp.c Outdated Show resolved Hide resolved
Co-authored-by: Michal Nowacki <[email protected]>
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