Merge branch 'main' into 1.x

elastic · Feb 17, 2022 · 0742ce7 · 0742ce7
2 parents d6c2e31 + 2a3b639
commit 0742ce7
Show file tree

Hide file tree

Showing 7 changed files with 94 additions and 14 deletions.
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
@@ -21,6 +21,10 @@ endif::[]
 // Using the template above, release notes go here.
 // CHANGELOG_AUTOMATION_KEYWORD
 
+[[release-notes-v1.4.2]]
+=== v1.4.2 - 2022/02/17
+* Create error events only for PHP error types included in https://www.php.net/manual/en/function.error-reporting.php[`error_reporting()`]: {pull}625[#625]
+
 [[release-notes-v1.4.1]]
 === v1.4.1 - 2022/02/14
 * Fixed error events not being created for PHP errors: {pull}619[#619]

diff --git a/docs/release-notes.asciidoc b/docs/release-notes.asciidoc
@@ -3,6 +3,7 @@
 [[release-notes]]
 == Release notes
 
+* <<release-notes-v1.4.2>>
 * <<release-notes-v1.4.1>>
 * <<release-notes-v1.4>>
 * <<release-notes-v1.3.1>>

diff --git a/src/ElasticApm/ElasticApm.php b/src/ElasticApm/ElasticApm.php
@@ -35,7 +35,7 @@ final class ElasticApm
 {
     use StaticClassTrait;
 
-    public const VERSION = '1.4.1';
+    public const VERSION = '1.4.2';
 
     /**
      * Begins a new transaction and sets it as the current transaction.

diff --git a/src/ElasticApm/Impl/Tracer.php b/src/ElasticApm/Impl/Tracer.php
@@ -274,10 +274,24 @@ public function onPhpError(
                 '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)

diff --git a/src/ext/elastic_apm_version.h b/src/ext/elastic_apm_version.h
@@ -18,4 +18,4 @@
  */
 #pragma once
 
-#define PHP_ELASTIC_APM_VERSION "1.4.1"
+#define PHP_ELASTIC_APM_VERSION "1.4.2"
diff --git a/tests/ElasticApmTests/ComponentTests/ErrorTest.php b/tests/ElasticApmTests/ComponentTests/ErrorTest.php
@@ -23,6 +23,7 @@
 
 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 @@ private static function verifyAppCodeStacktraceTop(array $expectedStacktraceTop,
         }
     }
 
-    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
@@ -25,14 +25,11 @@
 
 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