Repository: elastic/apm-agent-php (php)

Issue:
Allow users to control for which PHP errors agent creates APM error events
Follow up from [this comment](https://github.com/elastic/apm-agent-php/pull/619#issuecomment-1041392233)

It seems the most natural for the agent to abide by [`error_reporting()`](https://www.php.net/manual/en/function.error-reporting.php) and create error event only for PHP error types included in [`error_reporting()`](https://www.php.net/manual/en/function.error-reporting.php)

Pull request:
Create error event for PHP error only when in error_reporting()
Closes #624




Patch:
diff --git a/src/ElasticApm/Impl/Tracer.php b/src/ElasticApm/Impl/Tracer.php
index 43d8337a..be5df189 100644
--- a/src/ElasticApm/Impl/Tracer.php
+++ b/src/ElasticApm/Impl/Tracer.php
@@ -274,10 +274,24 @@ final class Tracer implements TracerInterface, LoggableInterface
                 'fileName' => $fileName,
                 'lineNumber' => $lineNumber,
                 'message' => $message,
-                'relatedThrowable' => $relatedThrowable,
+                'relatedThrowable' => $relatedThrowable
             ]
         );
 
+        if ((error_reporting() & $type) === 0) {
+            ($loggerProxy = $this->logger->ifDebugLevelEnabled(__LINE__, __FUNCTION__))
+            && $loggerProxy->log(
+                'Not creating error event because error_reporting() does not include its type',
+                ['type' => $type, 'error_reporting()' => error_reporting()]
+            );
+            return;
+        }
+        ($loggerProxy = $this->logger->ifDebugLevelEnabled(__LINE__, __FUNCTION__))
+        && $loggerProxy->log(
+            'Creating error event because error_reporting() includes its type...',
+            ['type' => $type, 'error_reporting()' => error_reporting()]
+        );
+
         $customErrorData = new CustomErrorData();
         $customErrorData->code = $type;
         $customErrorData->message = TextUtil::contains($message, $fileName)


Test patch:
diff --git a/tests/ElasticApmTests/ComponentTests/ErrorTest.php b/tests/ElasticApmTests/ComponentTests/ErrorTest.php
index fcf712ad..460a9170 100644
--- a/tests/ElasticApmTests/ComponentTests/ErrorTest.php
+++ b/tests/ElasticApmTests/ComponentTests/ErrorTest.php
@@ -23,6 +23,7 @@ declare(strict_types=1);
 
 namespace ElasticApmTests\ComponentTests;
 
+use Elastic\Apm\ElasticApm;
 use Elastic\Apm\Impl\ErrorData;
 use Elastic\Apm\Impl\Log\LoggableToString;
 use Elastic\Apm\Impl\StacktraceFrame;
@@ -44,6 +45,8 @@ final class ErrorTest extends ComponentTestCaseBase
     private const STACK_TRACE_FUNCTION = 'STACK_TRACE_FUNCTION';
     private const STACK_TRACE_LINE_NUMBER = 'STACK_TRACE_LINE_NUMBER';
 
+    private const INCLUDE_IN_ERROR_REPORTING = 'INCLUDE_IN_ERROR_REPORTING';
+
     private function verifyError(DataFromAgent $dataFromAgent): ErrorData
     {
         $tx = $this->verifyTransactionWithoutSpans($dataFromAgent);
@@ -103,27 +106,88 @@ final class ErrorTest extends ComponentTestCaseBase
         }
     }
 
-    public static function appCodeForTestPhpErrorUndefinedVariableWrapper(): void
+    private static function undefinedVariablePhpErrorCode(): int
+    {
+        // From PHP 7.4.x to PHP 8.0.x attempting to read an undefined variable
+        // was converted from notice to warning
+        // https://www.php.net/manual/en/migration80.incompatible.php
+        return (version_compare(PHP_VERSION, '8.0.0') < 0) ? E_NOTICE : E_WARNING;
+    }
+
+    private static function buildExceptionForSubstituteError(): DummyExceptionForTests
+    {
+        return new DummyExceptionForTests('Exception for substitute error', /* code: */ 123);
+    }
+
+    private static function verifySubstituteError(ErrorData $err): void
+    {
+        self::assertNotNull($err->exception);
+        self::assertSame('ElasticApmTests\\Util', $err->exception->module);
+        self::assertSame('DummyExceptionForTests', $err->exception->type);
+        self::assertSame('Exception for substitute error', $err->exception->message);
+        self::assertSame(123, $err->exception->code);
+    }
+
+    /**
+     * @param array<string, mixed> $appCodeArgs
+     */
+    public static function appCodeForTestPhpErrorUndefinedVariableWrapper(array $appCodeArgs): void
     {
+        /** @var bool $includeInErrorReporting */
+        $includeInErrorReporting = self::getMandatoryAppCodeArg($appCodeArgs, self::INCLUDE_IN_ERROR_REPORTING);
+        self::printMessage(
+            __METHOD__,
+            'Before changing error_reporting(): '
+            . '$includeInErrorReporting: ' . LoggableToString::convert($includeInErrorReporting)
+            . '; ' . 'error_reporting(): ' . error_reporting()
+            . '; ' . 'undefinedVariablePhpErrorCode: ' . self::undefinedVariablePhpErrorCode()
+            . '; ' . 'error_reporting() includes undefinedVariablePhpErrorCode: '
+            . LoggableToString::convert((error_reporting() & self::undefinedVariablePhpErrorCode()) !== 0)
+        );
+        if ($includeInErrorReporting) {
+            error_reporting(error_reporting() | self::undefinedVariablePhpErrorCode());
+        } else {
+            error_reporting(error_reporting() & ~self::undefinedVariablePhpErrorCode());
+        }
+        self::printMessage(
+            __METHOD__,
+            'After changing error_reporting(): ' . error_reporting()
+            . '; ' . 'error_reporting() includes undefinedVariablePhpErrorCode: '
+            . LoggableToString::convert((error_reporting() & self::undefinedVariablePhpErrorCode()) !== 0)
+        );
+
         appCodeForTestPhpErrorUndefinedVariable();
+
+        if (!$includeInErrorReporting) {
+            ElasticApm::createErrorFromThrowable(self::buildExceptionForSubstituteError());
+        }
     }
 
-    public function testPhpErrorUndefinedVariable(): void
+    /**
+     * @dataProvider boolDataProvider
+     *
+     * @param bool $includeInErrorReporting
+     */
+    public function testPhpErrorUndefinedVariable(bool $includeInErrorReporting): void
     {
         $this->sendRequestToInstrumentedAppAndVerifyDataFromAgent(
             (new TestProperties())
-                ->withRoutedAppCode([__CLASS__, 'appCodeForTestPhpErrorUndefinedVariableWrapper']),
-            function (DataFromAgent $dataFromAgent): void {
+                ->withRoutedAppCode([__CLASS__, 'appCodeForTestPhpErrorUndefinedVariableWrapper'])
+                ->withAppCodeArgs([self::INCLUDE_IN_ERROR_REPORTING => $includeInErrorReporting]),
+            function (DataFromAgent $dataFromAgent) use ($includeInErrorReporting): void {
                 $err = $this->verifyError($dataFromAgent);
-                // TODO: Sergey Kleyman: COMMENT
                 // self::printMessage(__METHOD__, '$err: ' . LoggableToString::convert($err));
+                if (!$includeInErrorReporting) {
+                    self::verifySubstituteError($err);
+                    return;
+                }
 
                 $appCodeFile = dirname(__FILE__) . DIRECTORY_SEPARATOR . 'appCodeForTestPhpErrorUndefinedVariable.php';
                 self::assertNotNull($err->exception);
                 // From PHP 7.4.x to PHP 8.0.x attempting to read an undefined variable
                 // was converted from notice to warning
                 // https://www.php.net/manual/en/migration80.incompatible.php
-                $expectedCode = (version_compare(PHP_VERSION, '8.0.0') < 0) ? E_NOTICE : E_WARNING;
+                $expectedCode = self::undefinedVariablePhpErrorCode();
                 $expectedType = PhpErrorUtil::getTypeName($expectedCode);
                 self::assertNotNull($expectedType, '$expectedCode: ' . $expectedCode);
                 self::assertSame($expectedType, $err->exception->type);
diff --git a/tests/ElasticApmTests/ComponentTests/appCodeForTestPhpErrorUndefinedVariable.php b/tests/ElasticApmTests/ComponentTests/appCodeForTestPhpErrorUndefinedVariable.php
index 69b8b7bd..24472941 100644
--- a/tests/ElasticApmTests/ComponentTests/appCodeForTestPhpErrorUndefinedVariable.php
+++ b/tests/ElasticApmTests/ComponentTests/appCodeForTestPhpErrorUndefinedVariable.php
@@ -25,14 +25,11 @@ namespace ElasticApmTests\ComponentTests;
 
 use PHPUnit\Framework\TestCase;
 
-const APP_CODE_FOR_TEST_PHP_ERROR_UNDEFINED_VARIABLE_ERROR_LINE_NUMBER = 38;
-const APP_CODE_FOR_TEST_PHP_ERROR_UNDEFINED_VARIABLE_CALL_TO_IMPL_LINE_NUMBER = 44;
+const APP_CODE_FOR_TEST_PHP_ERROR_UNDEFINED_VARIABLE_ERROR_LINE_NUMBER = 35;
+const APP_CODE_FOR_TEST_PHP_ERROR_UNDEFINED_VARIABLE_CALL_TO_IMPL_LINE_NUMBER = 41;
 
 function appCodeForTestPhpErrorUndefinedVariableImpl(): void
 {
-    // Ensure E_NOTICE is included in error_reporting
-    error_reporting(error_reporting() | E_NOTICE);
-
     TestCase::assertSame(APP_CODE_FOR_TEST_PHP_ERROR_UNDEFINED_VARIABLE_ERROR_LINE_NUMBER, __LINE__ + 2);
     /** @noinspection PhpUndefinedVariableInspection */
     $undefinedVariable = $undefinedVariable + 1; // @phpstan-ignore-line