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

Release 10.20 #884

Merged
merged 15 commits into from
Apr 29, 2024
Merged

Release 10.20 #884

merged 15 commits into from
Apr 29, 2024

Conversation

lavarou
Copy link
Member

@lavarou lavarou commented Apr 29, 2024

No description provided.

lavarou and others added 15 commits March 18, 2024 13:23
Bump version to 10.20 for next release and fix code name in code names'
history.

---------

Co-authored-by: Niklas <[email protected]>
Added additional tests to exercise a variety of return values from a
custom error handler.
This PR 
1)checks if EH_THROW(which signals to PHP to throw an exception from the
error) is toggled and if so does not record an error because if uncaught
it will be handled by the exception handler and if caught there's no
need to record it.
2) add tests to verify fix

More details:  
/*
   * For a the following error codes:
   * E_WARNING || E_CORE_WARNING || E_COMPILE_WARNING || E_USER_WARNING
* PHP triggers an exception if EH_THROW is toggled on and then
immediately
   * returns after throwing the exception. See for more info:
* https://github.com/php/php-src/blob/master/main/main.c In that case,
we
* should not handle it, but we should exist and let the exception
handler
* deal with it; otherwise, we could record an error even if an exception
is
   * caught. */

In the case where a php E_WARNING error was triggered.  We intercept the
error from PHP and record it then pass execution back to PHP.

But, for instance, in the particular case that deals with a filehandling
which toggles EH_THROW on
here: https://github.com/php/php-src/blob/master/ext/fileinfo/fileinfo.c#L176
PHP then handles the error but for a certain number of error codes, PHP
triggers an exception if EH_THROW is toggled
on: https://github.com/php/php-src/blob/master/main/main.c#L1254C1-L1259C24
and then immediately returns after throwing the exception.

This leads to the case in the issue where the exception that was
triggered is caught, but we recorded the error....
adds a filter for package data already sent during the current
connection instance to prevent sending duplicate data between harvests.
…877)

When user exception handler is installed it can call restore_exception_handler,
and that will reset is_exception_handler flag in the wraprec which will cause
the agent not to use the correct code path after user exception handler returns.
The information that nr_php_instrument_func_end is handling return from user
exception handler needs to be stored in the segment associated with the user
exception handler.
---------

Co-authored-by: ZNeumann <[email protected]>
Implement Yii v2 instrumentation for auto transaction naming.

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

---------

Co-authored-by: Razvan Grigore <[email protected]>
…or version (#868)

This PR creates a new supportability metric that will provide
information on what major version of the package is being used. The
packages that provide the major version and will send up this
supportability metric are:
 - Drupal
 - Guzzle
 - Laravel
 - Monolog
 - Predis
 - Slim
 - Wordpress.
To reduce agent's overhead, by default the agent will only instrument hook
callbacks from plugins/themes and will not instrument hook callbacks from
wordpress core.
…s (GH issue 875) (#881)

In GH issue #875 it was reported a warning is generated when using
prepared statements with `mysqli`. The issue included excellent
instructions on how to reproduce the issue. This PR addresses the cause
of the warning.

To understand the cause of the warning requires understanding how the
agent handles `mysqli::stmt` objects. When a SQL statement is prepared
via mysqli the agent will store the SQL string in a global hashmap
called `mysqli_queries` *if* the SQL is considered explainable
(determined via `nr_php_explain_mysql_query_is_explainable()`),
otherwise it is just ignored. Later when a slow SQL query is detected
the SQL string is retrieved and used to explain the query.

An additional factor is when a `mysqli::stmt` object is released it goes
into a free pool PHP keeps which allows it to quickly hand out an
allocated section of memory when a new object is created. Each object
has a `object ID` (referenced as a `handle` in the agent for the
relevant code). When an the allocated object is reused from the free
pool it retains the same `object ID`.

The agent used the `object ID` (`handle`) as the key for storing the
prepared SQL string for explainable SQL strings. Now what if an
explainable query comes through, the string is stored, and then that
object is released. A new query is prepared with a string that is NOT
explainable and gets the object container with the same `object ID` as
the explainable one just released. The string stored in `mysqli_queries`
using that `object ID` as the key is now stale. If the new,
unexplainable query is slow then the agent will pull this stale string
and try to run an explain with it - leading to the warning.

The fix is to clear any query string for a given `object ID` if the
query string is unexplainable.

Integration tests were added (based on the reproduction case) that test
for a regression of this fix.
A EXPECT_SLOW_SQLS specification was left out of one test from PR #881 .
Adds a workflow to enable automated responses via the
newrelic-php-agent-bot to newly opened GitHub issues as well as issues
determined to require additional support engagement.

---------

Co-authored-by: Amber Sistla <[email protected]>
Co-authored-by: Hitesh Ahuja <[email protected]>
Co-authored-by: Michal Nowacki <[email protected]>
Co-authored-by: Hitesh Ahuja <[email protected]>
@newrelic-php-agent-bot
Copy link

newrelic-php-agent-bot commented Apr 29, 2024

Test Suite Status Result
Multiverse 9/9 passing
SOAK 56/56 passing

@lavarou lavarou self-assigned this Apr 29, 2024
@codecov-commenter
Copy link

codecov-commenter commented Apr 29, 2024

Codecov Report

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

Project coverage is 78.72%. Comparing base (2c9bf20) to head (85d74db).

Files Patch % Lines
agent/fw_yii.c 55.10% 22 Missing ⚠️
agent/fw_slim.c 0.00% 5 Missing ⚠️
agent/fw_drupal8.c 25.00% 3 Missing ⚠️
agent/fw_wordpress.c 25.00% 3 Missing ⚠️
agent/fw_laravel.c 0.00% 2 Missing ⚠️
agent/php_agent.c 75.00% 1 Missing ⚠️
agent/php_mysqli.c 88.88% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #884      +/-   ##
==========================================
- Coverage   78.76%   78.72%   -0.05%     
==========================================
  Files         193      193              
  Lines       27125    27214      +89     
==========================================
+ Hits        21365    21423      +58     
- Misses       5760     5791      +31     
Flag Coverage Δ
agent-for-php-7.0 77.45% <56.48%> (-0.37%) ⬇️
agent-for-php-7.1 77.21% <54.71%> (-0.38%) ⬇️
agent-for-php-7.2 78.13% <65.09%> (-0.02%) ⬇️
agent-for-php-7.3 78.15% <65.09%> (-0.02%) ⬇️
agent-for-php-7.4 77.86% <65.09%> (-0.02%) ⬇️
agent-for-php-8.0 77.92% <74.73%> (+0.02%) ⬆️
agent-for-php-8.1 77.91% <74.73%> (+0.02%) ⬆️
agent-for-php-8.2 77.51% <74.73%> (+0.02%) ⬆️
agent-for-php-8.3 77.51% <74.73%> (+0.02%) ⬆️

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.

@lavarou lavarou merged commit 4f470da into main Apr 29, 2024
120 checks passed
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.

7 participants