feat(agent): add support for Yii v2 #848

lavarou · 2024-03-11T13:56:12Z

Implement Yii v2 instrumentation for auto transaction naming.

Original work by @razvanphp in #823. Fixes #821.

codecov-commenter · 2024-03-11T14:20:29Z

Codecov Report

Attention: Patch coverage is 55.10204% with 22 lines in your changes are missing coverage. Please review.

Project coverage is 78.73%. Comparing base (eecccd5) to head (0b892a6).

Files	Patch %	Lines
agent/fw_yii.c	55.10%	22 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #848      +/-   ##
==========================================
- Coverage   78.76%   78.73%   -0.04%     
==========================================
  Files         193      193              
  Lines       27129    27175      +46     
==========================================
+ Hits        21369    21396      +27     
- Misses       5760     5779      +19

Flag	Coverage Δ
agent-for-php-7.0	`77.79% <53.33%> (-0.04%)`	⬇️
agent-for-php-7.1	`77.54% <48.83%> (-0.04%)`	⬇️
agent-for-php-7.2	`78.11% <48.83%> (-0.04%)`	⬇️
agent-for-php-7.3	`78.13% <48.83%> (-0.04%)`	⬇️
agent-for-php-7.4	`77.84% <48.83%> (-0.04%)`	⬇️
agent-for-php-8.0	`77.90% <68.96%> (-0.01%)`	⬇️
agent-for-php-8.1	`77.88% <68.96%> (-0.01%)`	⬇️
agent-for-php-8.2	`77.48% <68.96%> (-0.01%)`	⬇️
agent-for-php-8.3	`77.48% <68.96%> (-0.01%)`	⬇️

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.

razvanphp · 2024-03-11T14:26:16Z

tests/integration/frameworks/yii/test_basic_cli.php

+
+/*EXPECT_ERROR_EVENTS null */
+
+/* WordPress mock app */


This should've been Yii 🙂 but no bigie, thanks for adding the tests for me!

Good catch!

Addressed with d059d1c.

razvanphp · 2024-03-11T14:42:51Z

agent/fw_yii.c

 /*
 * Enable Yii2 instrumentation.
 */
 void nr_yii2_enable(TSRMLS_D) {
+  NRPRG(framework_version) = 2;


is this still useful?

Not after c4172d8.

razvanphp · 2024-03-11T14:50:55Z

One more question: should the error catching be tested too?

lavarou · 2024-03-11T14:56:10Z

One more question: should the error catching be tested too?

It should be tested and is tested, just with a tool different than integration tests. That tool is using the actual framework, not the mock. I'll think some more about adding a mock to test error handling in integration tests.

agent/fw_yii.c

lavarou · 2024-03-13T14:31:58Z

One more question: should the error catching be tested too?

It should be tested and is tested, just with a tool different than integration tests. That tool is using the actual framework, not the mock. I'll think some more about adding a mock to test error handling in integration tests.

Further testing uncovered some issues with exception handling when Yii's Exception Handler is disabled. This requires further investigation.

agent/fw_yii.c

newrelic-php-agent-bot · 2024-04-11T15:53:57Z

Test Suite	Status	Result
Multiverse	✅	9/9 passing
SOAK	❌	49/51 passing

razvanphp · 2024-04-11T17:45:25Z

Further testing uncovered some issues with exception handling when Yii's Exception Handler is disabled. This requires further investigation.

Can you elaborate on this? I tried it and default PHP error handler was used and errors were catched by the default newrelic implementation.

Agent for PHPs 8.0+ uses observer API to hook into Zend Engine, which allows offers better out-of-the-box instrumentation of user exception handler. When observer API is used, there's no need to wrap exception handlers to record errors.

Yii 1.x and Yii 2.x are detected as two distinct frameworks and therefore need their own values in nrframework_t enum. This helps with tracking framework usage through supportability metrics. This approach obsoletes the need to store framework_version.

Agent for PHPs 8.0+ uses observer API to hook into Zend Engine, which allows offers better out-of-the-box instrumentation of user exception handler. When observer API is used, there's no need to wrap exception handlers to record errors.

agent/fw_yii.c

* nr_php_is_zval_valid_object already checks for NULL * NR_PHP_WRAPPER_END already ensures NR_PHP_WRAPPER_CALL is called and there's no need to call it earlier

lavarou · 2024-04-19T16:17:48Z

Further testing uncovered some issues with exception handling when Yii's Exception Handler is disabled. This requires further investigation.

Can you elaborate on this? I tried it and default PHP error handler was used and errors were catched by the default newrelic implementation.

There were two problems which caused the default newrelic implementation not to record errors correctly. One has been resolved with #851 and another with #877.

lavarou added the ready for review label Mar 11, 2024

lavarou requested review from ZNeumann and zsistla March 11, 2024 13:56

lavarou self-assigned this Mar 11, 2024

lavarou mentioned this pull request Mar 11, 2024

feat(agent): Add Yii v2 framework instrumentation #823

Closed

5 tasks

razvanphp reviewed Mar 11, 2024

View reviewed changes

ZNeumann reviewed Mar 11, 2024

View reviewed changes

agent/fw_yii.c Show resolved Hide resolved

ZNeumann approved these changes Mar 12, 2024

View reviewed changes

lavarou modified the milestone: next-release Mar 12, 2024

lavarou removed the ready for review label Mar 13, 2024

lavarou changed the title ~~feat(agent): add support for Yii v2~~ [WIP]: feat(agent): add support for Yii v2 Mar 13, 2024

lavarou marked this pull request as draft March 13, 2024 14:33

zsistla reviewed Mar 14, 2024

View reviewed changes

agent/fw_yii.c Outdated Show resolved Hide resolved

lavarou force-pushed the feat/agent/add-support-for-yii2 branch from dc5fa2c to 25c800e Compare March 15, 2024 01:19

lavarou force-pushed the feat/agent/add-support-for-yii2 branch from 25c800e to 3bddfdf Compare April 11, 2024 15:53

lavarou force-pushed the feat/agent/add-support-for-yii2 branch from 3bddfdf to 3d90b20 Compare April 12, 2024 17:13

lavarou changed the base branch from dev to fix/agent/oapi-exception-handler-instrumentation April 12, 2024 17:14

Base automatically changed from fix/agent/oapi-exception-handler-instrumentation to dev April 19, 2024 13:19

razvanphp and others added 3 commits April 19, 2024 09:22

feat(agent): Add Yii v2 framework instrumentation

1dc4b7c

feat(agent): Add Yii v2 ErrorHandler wrapper

46fd442

lavarou added 4 commits April 19, 2024 09:22

add integration tests

ec4ebf3

typo

86cb90e

free zvals returned by nr_php_call

8e5cd7c

remove ErrorHandler wrapper for PHPs 8.0+

cb19ba7

Agent for PHPs 8.0+ uses observer API to hook into Zend Engine, which allows offers better out-of-the-box instrumentation of user exception handler. When observer API is used, there's no need to wrap exception handlers to record errors.

lavarou force-pushed the feat/agent/add-support-for-yii2 branch from 3d90b20 to cb19ba7 Compare April 19, 2024 13:22

lavarou marked this pull request as ready for review April 19, 2024 13:22

lavarou changed the title ~~[WIP]: feat(agent): add support for Yii v2~~ feat(agent): add support for Yii v2 Apr 19, 2024

enhance check for return value of getUniqueId

ca81b4d

zsistla reviewed Apr 19, 2024

View reviewed changes

agent/fw_yii.c Show resolved Hide resolved

simplify nr_yii2_error_handler_wrapper

0b892a6

* nr_php_is_zval_valid_object already checks for NULL * NR_PHP_WRAPPER_END already ensures NR_PHP_WRAPPER_CALL is called and there's no need to call it earlier

zsistla approved these changes Apr 19, 2024

View reviewed changes

lavarou merged commit 539dd4f into dev Apr 19, 2024
62 checks passed

lavarou deleted the feat/agent/add-support-for-yii2 branch April 19, 2024 16:41

lavarou added this to the next-release milestone Apr 19, 2024

lavarou linked an issue Apr 19, 2024 that may be closed by this pull request

Support for PHP Framework Yii2 #821

Closed

razvanphp mentioned this pull request Oct 24, 2024

Update php-agent-compatibility-requirements.mdx for Yii 1 & 2 newrelic/docs-website#19018

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(agent): add support for Yii v2 #848

feat(agent): add support for Yii v2 #848

lavarou commented Mar 11, 2024

codecov-commenter commented Mar 11, 2024 •

edited

Loading

razvanphp Mar 11, 2024

lavarou Mar 11, 2024

lavarou Mar 11, 2024

razvanphp Mar 11, 2024

lavarou Mar 11, 2024

razvanphp commented Mar 11, 2024

lavarou commented Mar 11, 2024

lavarou commented Mar 13, 2024

newrelic-php-agent-bot commented Apr 11, 2024 •

edited

Loading

razvanphp commented Apr 11, 2024

lavarou commented Apr 19, 2024

feat(agent): add support for Yii v2 #848

feat(agent): add support for Yii v2 #848

Conversation

lavarou commented Mar 11, 2024

codecov-commenter commented Mar 11, 2024 • edited Loading

Codecov Report

razvanphp Mar 11, 2024

Choose a reason for hiding this comment

lavarou Mar 11, 2024

Choose a reason for hiding this comment

lavarou Mar 11, 2024

Choose a reason for hiding this comment

razvanphp Mar 11, 2024

Choose a reason for hiding this comment

lavarou Mar 11, 2024

Choose a reason for hiding this comment

razvanphp commented Mar 11, 2024

lavarou commented Mar 11, 2024

lavarou commented Mar 13, 2024

newrelic-php-agent-bot commented Apr 11, 2024 • edited Loading

razvanphp commented Apr 11, 2024

lavarou commented Apr 19, 2024

codecov-commenter commented Mar 11, 2024 •

edited

Loading

newrelic-php-agent-bot commented Apr 11, 2024 •

edited

Loading