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.

Sign up for GitHub

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

fix(php): fixed php debian recipe to accommodate recent php agent changes #1131

Merged
merged 2 commits into from
Nov 12, 2024
Merged

fix(php): fixed php debian recipe to accommodate recent php agent changes #1131

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6667702
fix(php): fixed php recipe to accommodate recent php agent changes
shashank-reddy-nr Nov 4, 2024
bbcf872
chore(php): variable name correction
shashank-reddy-nr Nov 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 23 additions & 5 deletions recipes/newrelic/apm/php/debian.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,10 +465,20 @@ install:
# Get the PHP INI Directory associated with this specific NR INI file
#
php_ini_dir=$(echo $ini | sed -n 's/\(.*\)\/conf.d/\1/p')
if [ -z "${php_ini_dir}" ]; then
php_ini_dir=$ini
if [[ -z "${php_ini_dir}" ]]; then
if [[ -z "${CLI_DIRS}" ]]; then
if [[ -d "${ini}/../apache2" ]]; then
php_ini_dir="${ini}/../apache2"
fi
if [[ -d "${ini}/../fpm" ]]; then
php_ini_dir="${ini}/../fpm"
fi
elif [[ -d "${ini}/../cli" ]]; then
php_ini_dir="${ini}/../cli"
else
php_ini_dir=$ini
fi
Comment on lines +468 to +480
Copy link
Member

@pranav-new-relic pranav-new-relic Nov 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Changes Explained:

As stated by the PHP Agent team, with the latest PHP Agent v11.3.0.16 released around two weeks ago, they've introduced a change, which would affect the functioning of the PHP tarball installer.

With the debian (package) installer using the etc/php/7.x/mods-available directory (by design) to configure newrelic.ini, customers tending to upgrade using the tarball installer would sometimes run into issues, since the installer would place a duplicate newrelic.ini in etc/php/7.x apache2 (or) fpm (or) cli, leading to duplicates of the newrelic.ini file. To solve this problem, the solution devised by the agent team was to allow for "skipping" placing a newrelic.ini file elsewhere, if a newrelic.ini is already found to have been present in the mods-available directory by the tarball installer.

This affected the recipe owing to the following reason.

The source of ${ini} here is a collection of probable directories where the php.ini file could be present, originating from pi_inidir_dso and/or pi_inidir_cli originating from this place in the agent, which the recipe scrapes.

This list earlier contained a bunch of directories such as apache2 or fpm or cli, which would contain a conf.d and php.ini; however, in order to allow for the tarball installer to check for the presence of a newrelic.ini in mods-available, this has also been in the code linked above, to override these paths when present. This wouldn't cause an issue with the agent, since the intended use is to skip populating a newrelic.ini in any of the aforementioned directories, if there is a mods-available with a newrelic.ini.

However, in the case of this recipe, since we rely on the ${ini} picked later in the code to set the target_ini_dir where the newrelic.ini would need to be moved, the value of ${ini}, which would ideally need to be one of the aforementioned directories and then allow traversing to mods-available to overwrite the newrelic.ini file there, is now mods-available already, which is why it is not able to traverse out of mods-available to mods-available; which is exactly what the error upon guided install currently states.

To avoid this problem, two things needed to be done -

  • firstly, if ${ini} is bound to be mods-available whenever a newrelic.ini is present in mods-available, reverse engineer the conditions to fix the effective ${ini} we shall use to move the newrelic.ini to, using php_ini_dir. In other words, conditionally assign ${ini} to php_ini_dir based on the path found at ../ of ${ini}, which would in turn, go back to the expected apache2 or fpm or cli based on the conditions written. If none of the conditions are satisfied, php_ini_dir takes ${ini}, which is also likely to happen when ${ini} is not mods-available, so either way, this conditional logic prevents php_ini_dir from being assigned to the wrong directory, especially since this is used later in the code.

  • the condition to configure target_ini_dir was earlier configured based on ${ini} which may now be made php_ini_dir (with one lesser folder to traverse out of, ../ instead of ../../) since php_ini_dir now takes the "effective" ${ini} and not the actual ${ini} directly, making sure that the traversal would never fail, which would prevent the error we're seeing now

fi

#
# Get the PHP Binary directory associated with this particular NR INI file.
sed_slash_ini=$(echo "${ini}" | sed 's/\//\\\//g')
Expand Down Expand Up @@ -501,6 +511,10 @@ install:
fi

sed -i "s/newrelic.appname = \"[^\"]*\"/newrelic.appname = \"${APPLICATION_NAME}\"/" $ini_full_name

LICENSE_KEY="{{.NEW_RELIC_LICENSE_KEY}}"
sed -i -E "s/newrelic.license = \"(REPLACE_WITH_REAL_KEY|)\"/newrelic.license = \"${LICENSE_KEY}\"/" $ini_full_name

Comment on lines +514 to +517
Copy link
Member

Choose a reason for hiding this comment

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

In continuation to the explanation in the above comment, a different limitation noticed because of the aforementioned change in the agent was that the newrelic.ini file populated in the mods-available directory had no license key populated.

Upon discussing this with the PHP Agent, it was obtained that the cause of this is as follows. Earlier, since a new newrelic.ini would be written to one of apache2 or fpm or cli by the recipe (as provided by the tarball installer, even in the event of a newrelic.ini existing in mods-available), the newrelic.ini file would be written with the license key, and our code in the recipe would move the newrelic.ini from one of these folders into target_ini_dir, i.e. ../../mods-available, thereby making sure that the newrelic.ini in mods-available is overwritten with the newrelic.ini in one of these directories, with the license key, facilitating instrumentation.

However, now that the newrelic.ini file is no longer generated in any of these directories if there already exists a newrelic.ini in mods-available, the newrelic.ini in mods-available is not replaced by any other version of newrelic.ini elsewhere.

Added to this is the fact that the newrelic.ini in mods-available tends to be generated without a license key.

A combination of all of these would mean we would need to sed our way into affixing the license key into the newrelic.ini that is now bound to stay in mods-available, but did not have the license key so far, and hence needs it; this is why this change is needed.

if [ "{{.NEW_RELIC_REGION}}" = "STAGING" ]; then
sed -i 's/;newrelic.daemon.collector_host = ""/newrelic.daemon.collector_host = "staging-collector.newrelic.com"/' $ini_full_name
sed -i 's/;newrelic.loglevel = "info"/newrelic.loglevel = "verbosedebug"/' $ini_full_name
Expand All @@ -520,7 +534,7 @@ install:
#
target_ini_dir="/etc/php5/conf.d/"
if [ -x /usr/sbin/phpquery ]; then
target_ini_dir="${ini}/../../mods-available/"
target_ini_dir="${php_ini_dir}/../mods-available/"
Comment on lines -523 to +537
Copy link
Member

Choose a reason for hiding this comment

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

explained in the second point here

elif [ -x /usr/sbin/php5endmod ]; then
target_ini_dir="/etc/php5/mods-available/"
fi
Expand All @@ -533,7 +547,11 @@ install:
# This ensures we always get the most up to date newrelic.ini file on the
# system.
#
mv ${ini_full_name} ${target_ini_dir}
ini_source_dir=$(dirname "$(realpath "$ini_full_name")")
ini_destination_dir=$(realpath "$target_ini_dir")
if [[ "$ini_source_dir" != "$ini_destination_dir" ]]; then
mv "${ini_full_name}" "${ini_destination_dir}"
fi
Comment on lines +550 to +554
Copy link
Member

Choose a reason for hiding this comment

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

Finally, this code block is a consequence of everything else described above. $ini_full_name holds the path to newrelic.ini in ${ini}.

With all the context above, and the model used by the recipe to first install via the package-based installer and then use tarball, with the former leading to placing a newrelic.ini in mods-available, target_ini_dir was always meant to be mods-available, prior to these changes too (to move any updated newrelic.ini to replace the newrelic.ini in mods-available).

However, now with the concept of "updating" newrelic.ini in mods-available vanishing (if it already exists, no other newrelic.ini is added to other directories), and the probability of ${ini} being mods-available when a newrelic.ini is found in mods-available, it is very likely that the path to ini_full_name and target_ini_dir would now be the same; in which case it wouldn't make sense to perform the move operation in the code block. Nevertheless, to make this check and only perform this when both of these paths are unequal, the condition has been added.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason why this piece of code is necessary is that if a user installs the PHP agent manually using a tarball installer, they will have newrelic.ini in fpm or apache2 and cli, but not in mods-available as per agent logic. If, in the future, user want to run the guided install again on same host, the agent will not add newrelic.ini to any locations. However, the agent will inform us that newrelic.ini is already present in cli/conf.d and fpm/conf.d. Therefore, we will move the newrelic.ini to mods-available using this piece of code.


#
# We rely on the package installer to enable the newrelic module.
Expand Down
Loading