tests: fix integration_runner opcache module loading (#1097)

With recent official PHP docker images the Zend OPCache extension is
loaded by default. This is causing some test failures as PHP will output
a warning message when the `integration_runner` attempts to load this
extension. As a result any `EXPECT` statements in tests will fail due to
this extra output.

The solution chosen was to use the “-m” option for the `php` and
`php-cgi` binaries to probe if the Zend OPCache module is loaded by
default in the PHP environment. Both binaries are used in testing so the
defaults of both are considered. The value of the integration_runner
command line option `opcache_off` is also noted. When a test is going to
be run this information is used to determine if the
`zend_extension=opcache.so` is needed as well as setting the values of
`opcache.enable` and `opcache.enable_cli`. These INI values needed
special handling because it is possible these are overwritten by values
in the INI stanza of a given test case. Also consideration is needed to
if the `PHPMODULES` stanza exists in the test case and specifies loading
the `opcache.so` module.

---------

Co-authored-by: Michal Nowacki <[email protected]>

Author: 2 people

daemon/cmd/integration_runner/main.go

@@ -51,6 +51,7 @@ var (
 	flagMaxCustomEvents = flag.Int("max_custom_events", 30000, "value for newrelic.custom_events.max_samples_stored")
 	flagWarnIsFail      = flag.Bool("warnisfail", false, "warn result is treated as a fail")
 	flagOpcacheOff      = flag.Bool("opcacheoff", false, "run without opcache. Some tests are intended to fail when run this way")
+	flagDebug           = flag.Bool("debug", false, "enable debug logging for integration_runner")
 
 	// externalPort is the port on which we start a server to handle
 	// external calls.
@@ -362,15 +363,8 @@ func main() {
 		ctx.Settings["newrelic.loglevel"] = *flagLoglevel
 	}
 
-	if false == *flagOpcacheOff {
-		// PHP Modules common to all tests
-		ctx.Settings["zend_extension"] = "opcache.so"
-
-		// PHP INI values common to all tests
-		// These settings can be overwritten by adding new values to the INI block
-		ctx.Settings["opcache.enable"] = "1"
-		ctx.Settings["opcache.enable_cli"] = "1"
-	}
+	ctx.OPCacheModuleLoaded = integration.GetOPCacheModuleLoaded(*flagPHP, *flagCGI)
+	ctx.UseOPCache = !*flagOpcacheOff
 
 	// If the user provided a custom agent extension, use it.
 	if len(*flagAgent) > 0 {
@@ -459,8 +453,14 @@ func main() {
 	if numFailed > 0 {
 		os.Exit(1)
 	}
-	if *flagWarnIsFail && numWarned > 0 {
-		os.Exit(2)
+
+	if numWarned > 0 {
+		if *flagWarnIsFail {
+			fmt.Println("WARNING: some tests were warned, but are treated as failures because --warnisfail is true")
+			os.Exit(2)
+		} else {
+			fmt.Printf("WARNING: some tests were warned, but are not treated as failures because --warnisfail is false\n")
+		}
 	}
 }
 
@@ -534,6 +534,10 @@ func runTest(t *integration.Test) {
 	if skipIf != nil {
 		_, body, err := skipIf.Execute()
 
+		if *flagDebug {
+			fmt.Printf("SkipIf output:\n%s\n", body)
+		}
+
 		if err != nil {
 			t.Output = body
 			t.Fatal(fmt.Errorf("error executing skipif: %v", err))
@@ -545,6 +549,12 @@ func runTest(t *integration.Test) {
 			t.Skip(reason)
 			return
 		}
+
+		if warnRE.Match(body) && *flagWarnIsFail {
+			reason := string(bytes.TrimSpace(head(body)))
+			t.Warn(reason)
+			return
+		}
 	}
 
 	// Reset global response headers before the test is run. This feature
@@ -564,6 +574,10 @@ func runTest(t *integration.Test) {
 		t.Duration = 0
 	}
 
+	if *flagDebug {
+		fmt.Printf("Test output:\n%s\n", body)
+	}
+
 	// Always save the test output. If an error occurred it may contain
 	// critical information regarding the cause. Currently, it may also
 	// contain valgrind commentary which we want to display.

daemon/internal/newrelic/integration/context.go

@@ -12,12 +12,14 @@ import (
 )
 
 type Context struct {
-	PHP      string            // path to the PHP CLI executable
-	CGI      string            // path to the PHP CGI executable
-	Valgrind string            // path to the Valgrind executable, or empty if disabled
-	Env      map[string]string // environment variables to pass to each test
-	Settings map[string]string // settings to pass to each test
-	Timeout  time.Duration     // maximum test duration
+	PHP                 string            // path to the PHP CLI executable
+	CGI                 string            // path to the PHP CGI executable
+	Valgrind            string            // path to the Valgrind executable, or empty if disabled
+	Env                 map[string]string // environment variables to pass to each test
+	Settings            map[string]string // settings to pass to each test
+	Timeout             time.Duration     // maximum test duration
+	OPCacheModuleLoaded map[string]bool   // map of PHP and CGI to OPcache default loaded status
+	UseOPCache          bool              // whether to use OPcache in tests
 }
 
 func NewContext(php, cgi string) *Context {

daemon/internal/newrelic/integration/test.go

@@ -201,7 +201,34 @@ func merge(a, b map[string]string) map[string]string {
 	return merged
 }
 
+// checks the context to see if opcache is loaded by default
+// and then handles the PHP modules requests to make sure
+// we don't load opcache.so if it is already loaded by default
+func (t *Test) HandlePHPModules(php_executable string, ctx *Context) map[string]string {
+	// two cases:
+	// 1. Web test and php-cgi has opcache.so loaded by default - remove any PHPMODULE spec for opcache.so
+	// 2. PHP test and php has opcache.so loaded by default - remove any PHPMODULE spec for opcache.so
+	phpModulesCopy := make(map[string]string)
+	if ctx.OPCacheModuleLoaded[php_executable] {
+		for k, v := range t.PhpModules {
+			if !strings.Contains(v, "opcache.so") {
+				phpModulesCopy[k] = v
+			}
+		}
+	}
+
+	return phpModulesCopy
+}
+
 func (t *Test) MakeRun(ctx *Context) (Tx, error) {
+
+	// we don't support running C tests - assert this so we can
+	// troubleshoot if we try to run a C test
+	if t.IsC() {
+		fmt.Printf("ERROR - UNEXPECTED - Running C test: %s\n", t.Path)
+		os.Exit(1)
+	}
+
 	t.Env = merge(ctx.Env, t.Env)
 	settings := merge(ctx.Settings, t.Settings)
 	settings["newrelic.appname"] = t.Name
@@ -214,11 +241,18 @@ func (t *Test) MakeRun(ctx *Context) (Tx, error) {
 		}
 	}
 
-	settings = merge(settings, t.PhpModules)
-
-	if t.IsC() {
-		return CTx(ScriptFile(t.Path), t.Env, settings, headers, ctx)
+	var php_executable string
+	if t.IsWeb() {
+		php_executable = ctx.CGI
+	} else if t.IsPHP() {
+		php_executable = ctx.PHP
+	} else {
+		return nil, fmt.Errorf("unknown test type for %s", t.Path)
 	}
+
+	phpModulesCopy := t.HandlePHPModules(php_executable, ctx)
+	settings = merge(settings, phpModulesCopy)
+
 	if t.IsWeb() {
 		return CgiTx(ScriptFile(t.Path), t.Env, settings, headers, ctx)
 	}
@@ -241,6 +275,10 @@ func (t *Test) MakeSkipIf(ctx *Context) (Tx, error) {
 		data: t.rawSkipIf,
 	}
 
+	// handle the PHPMODULES directive
+	phpModulesCopy := t.HandlePHPModules(ctx.PHP, ctx)
+	settings = merge(settings, phpModulesCopy)
+
 	return PhpTx(src, t.Env, settings, ctx)
 }
 

daemon/internal/newrelic/integration/transaction.go

@@ -41,12 +41,61 @@ func flatten(x map[string]string) []string {
 	return s
 }
 
+// fixup settings to handle opcache module loading gracefully
+//
+// php_executable is the name to the PHP executable
+// env is the environment variables to pass to the PHP process
+// settings is the PHP settings to apply to the process
+// ctx is the context containing configuration options
+func fixupSettings(php_executable string, env, settings map[string]string, ctx *Context) map[string]string {
+
+	// Make a copy of settings to avoid mutating the original map
+	// Need to adjust settings to opcache
+	phpSettings := make(map[string]string, len(settings))
+	setOPCacheEnable := true
+	setOPCacheEnableCLI := true
+
+	for k, v := range settings {
+		phpSettings[k] = v
+
+		// see if settings affect opcache config
+		// if so then we will not set config below
+		if k == "opcache.enable" {
+			setOPCacheEnable = false
+		} else if k == "opcache.enable_cli" {
+			setOPCacheEnableCLI = false
+		}
+	}
+	if ctx.UseOPCache {
+		if !ctx.OPCacheModuleLoaded[php_executable] {
+			phpSettings["zend_extension"] = "opcache.so"
+		}
+		if setOPCacheEnable {
+			phpSettings["opcache.enable"] = "1"
+		}
+		if setOPCacheEnableCLI {
+			phpSettings["opcache.enable_cli"] = "1"
+		}
+	} else {
+		if setOPCacheEnable {
+			phpSettings["opcache.enable"] = "0"
+		}
+		if setOPCacheEnableCLI {
+			phpSettings["opcache.enable_cli"] = "0"
+		}
+	}
+
+	return phpSettings
+}
+
 // PhpTx constructs non-Web transactions to be executed by PHP.
 func PhpTx(src Script, env, settings map[string]string, ctx *Context) (Tx, error) {
 	// Note: file path must be relative to the working directory.
 	var txn Tx
 
-	args := phpArgs(nil, filepath.Base(src.Name()), false, settings)
+	newSettings := fixupSettings(ctx.PHP, env, settings, ctx)
+
+	args := phpArgs(nil, filepath.Base(src.Name()), false, newSettings)
 
 	if ctx.Valgrind != "" && settings["newrelic.appname"] != "skipif" {
 		txn = &ValgrindCLI{
@@ -83,6 +132,8 @@ func CgiTx(src Script, env, settings map[string]string, headers http.Header, ctx
 	var err error
 	var txn Tx
 
+	newSettings := fixupSettings(ctx.CGI, env, settings, ctx)
+
 	req := &http.Request{
 		Method:     env["REQUEST_METHOD"],
 		RequestURI: "/" + filepath.Base(src.Name()) + "?" + env["QUERY_STRING"],
@@ -113,7 +164,7 @@ func CgiTx(src Script, env, settings map[string]string, headers http.Header, ctx
 				handler: &cgi.Handler{
 					Path: ctx.CGI,
 					Dir:  src.Dir(),
-					Args: phpArgs(nil, "", false, settings),
+					Args: phpArgs(nil, "", false, newSettings),
 				},
 			},
 			Valgrind: ctx.Valgrind,
@@ -144,7 +195,7 @@ func CgiTx(src Script, env, settings map[string]string, headers http.Header, ctx
 			handler: &cgi.Handler{
 				Path: ctx.CGI,
 				Dir:  src.Dir(),
-				Args: phpArgs(nil, "", false, settings),
+				Args: phpArgs(nil, "", false, newSettings),
 			},
 		}
 		tx.handler.Env = append(tx.handler.Env,

daemon/internal/newrelic/integration/util.go

@@ -6,12 +6,14 @@
 package integration
 
 import (
+	"bytes"
 	"fmt"
+	"os"
 	"os/exec"
 )
 
 func GetPHPVersion() string {
-	cmd := exec.Command("php", "-r", "echo PHP_VERSION;")
+	cmd := exec.Command("php", "-d", "newrelic.daemon.dont_launch=3", "-r", "echo PHP_VERSION;")
 
 	output, err := cmd.Output()
 	if err != nil {
@@ -23,11 +25,37 @@ func GetPHPVersion() string {
 }
 
 func GetAgentVersion(agent_extension string) string {
-	cmd := exec.Command("php", "-d", "extension="+agent_extension, "-r", "echo phpversion('newrelic');")
+	cmd := exec.Command("php", "-d", "newrelic.daemon.dont_launch=3", "-d", "extension="+agent_extension, "-r", "echo phpversion('newrelic');")
 
 	output, err := cmd.Output()
 	if err != nil {
 		return fmt.Errorf("Failed to get agent version: %v", err).Error()
 	}
 	return string(output)
 }
+
+func IsOPcacheLoaded(php_executable string) bool {
+	fmt.Printf("Checking if OPcache is loaded using %s\n", php_executable)
+	cmd := exec.Command(php_executable, "-d", "newrelic.daemon.dont_launch=3", "-m")
+
+	output, err := cmd.Output()
+
+	if err != nil {
+		fmt.Printf("Failed to check if OPcache is loaded: %v\n", err)
+		os.Exit(1)
+	}
+
+	// Check if "Zend OPcache" is in the output
+	return bytes.Contains(output, []byte("Zend OPcache"))
+}
+
+func GetOPCacheModuleLoaded(php, cgi string) map[string]bool {
+	result := make(map[string]bool)
+
+	result[php] = IsOPcacheLoaded(php)
+	result[cgi] = IsOPcacheLoaded(cgi)
+
+	fmt.Printf("OPcache default loaded status: %+v\n", result)
+
+	return result
+}