Reducing Noise in Remote Logging (#51357)

* Refactor remote logging in WooCommerce

This commit refactors the remote logging functionality in WooCommerce to improve its efficiency and flexibility.

- In the `class-woocommerce.php` file, the `context` array now includes a new key `remote-logging` to indicate whether the error should be logged remotely if remote logging is enabled.

- In the `RemoteLogger.php` file, the `file` key in the `context['error']` array is now assigned to the `$log_data` array, and then unset from the `context['error']` array.

- The `should_handle` method in the `RemoteLogger` class now checks for the presence of the `remote-logging` key in the `context` array. If it is not set or set to `false`, the log is ignored.

- The `RemoteLoggerTest.php` file includes new test cases to ensure that the `should_handle` method returns `false` when the `remote-logging` key is not present in the `context` array.

These changes improve the remote logging functionality in WooCommerce and make it more robust and efficient.

* Revert log format change

* Set remote-logging context to true in log remote event method

* Add changelog

* revert change

* revert change

plugins/woocommerce-beta-tester/api/remote-logging/remote-logging.php

@@ -100,7 +100,10 @@ function log_remote_event() {
 		time(),
 		'critical',
 		'Test PHP event from WC Beta Tester',
-		array( 'source' => 'wc-beta-tester' )
+		array(
+			'source'         => 'wc-beta-tester',
+			'remote-logging' => true,
+		)
 	);
 
 	if ( $result ) {

plugins/woocommerce-beta-tester/changelog/enhance-reduce-remote-logging-noise

@@ -0,0 +1,4 @@
+Significance: patch
+Type: tweak
+
+Set remote-logging context to true in log remote event method

plugins/woocommerce/changelog/enhance-reduce-remote-logging-noise

@@ -0,0 +1,4 @@
+Significance: patch
+Type: enhancement
+
+Reducing noise in remote logging

plugins/woocommerce/includes/class-woocommerce.php

@@ -383,8 +383,10 @@ final class WooCommerce {
 			unset( $error_copy['message'] );
 
 			$context = array(
-				'source' => 'fatal-errors',
+				'source'         => 'fatal-errors',
-				'error'  => $error_copy,
+				'error'          => $error_copy,
+				// Indicate that this error should be logged remotely if remote logging is enabled.
+				'remote-logging' => true,
 			);
 
 			if ( false !== strpos( $message, 'Stack trace:' ) ) {

plugins/woocommerce/src/Internal/Logging/RemoteLogger.php

@@ -103,6 +103,8 @@ class RemoteLogger extends \WC_Log_Handler {
 
 		$extra_attrs = $context['extra'] ?? array();
 		unset( $context['extra'] );
+		unset( $context['remote-logging'] );
+
 		// Merge the extra attributes with the remaining context since we can't send arbitrary fields to Logstash.
 		$log_data['extra'] = array_merge( $extra_attrs, $context );
 
@@ -162,9 +164,15 @@ class RemoteLogger extends \WC_Log_Handler {
 	 * @return bool True if the log should be handled.
 	 */
 	protected function should_handle( $level, $message, $context ) {
+		// Ignore logs that are not opted in for remote logging.
+		if ( ! isset( $context['remote-logging'] ) || false === $context['remote-logging'] ) {
+			return false;
+		}
+
 		if ( ! $this->is_remote_logging_allowed() ) {
 			return false;
 		}
+
 		// Ignore logs that are less severe than critical. This is temporary to prevent sending too many logs to the remote logging service. We can consider remove this if the remote logging service can handle more logs.
 		if ( WC_Log_Levels::get_level_severity( $level ) < WC_Log_Levels::get_level_severity( WC_Log_Levels::CRITICAL ) ) {
 			return false;

plugins/woocommerce/tests/php/src/Internal/Logging/RemoteLoggerTest.php

@@ -348,7 +348,7 @@ namespace Automattic\WooCommerce\Tests\Internal\Logging {
 
 			$setup( $this );
 
-			$result = $this->invoke_private_method( $this->sut, 'should_handle', array( $level, 'Test message', array() ) );
+			$result = $this->invoke_private_method( $this->sut, 'should_handle', array( $level, 'Test message', array( 'remote-logging' => true ) ) );
 			$this->assertEquals( $expected, $result );
 		}
 
@@ -377,6 +377,14 @@ namespace Automattic\WooCommerce\Tests\Internal\Logging {
 			);
 		}
 
+		/**
+		 * @testdox Test should_handle returns false without remote-logging context
+		 */
+		public function test_should_handle_no_remote_logging_context() {
+			$result = $this->invoke_private_method( $this->sut, 'should_handle', array( 'error', 'Test message', array() ) );
+			$this->assertFalse( $result, 'should_handle should return false without remote-logging context' );
+		}
+
 		/**
 		 * @testdox handle method applies filter and doesn't send logs when filtered to null
 		 */
@@ -390,7 +398,7 @@ namespace Automattic\WooCommerce\Tests\Internal\Logging {
 			add_filter( 'woocommerce_remote_logger_formatted_log_data', fn() => null, 10, 4 );
 			add_filter( 'pre_http_request', fn() => $this->fail( 'wp_safe_remote_post should not be called' ), 10, 3 );
 
-			$this->assertFalse( $this->sut->handle( time(), 'error', 'Test message', array() ) );
+			$this->assertFalse( $this->sut->handle( time(), 'error', 'Test message', array( 'remote-logging' => true ) ) );
 		}
 
 		/**
@@ -404,7 +412,7 @@ namespace Automattic\WooCommerce\Tests\Internal\Logging {
 			$this->sut->set_is_dev_or_local( true );
 			$this->sut->method( 'is_remote_logging_allowed' )->willReturn( true );
 
-			$this->assertFalse( $this->sut->handle( time(), 'error', 'Test message', array() ) );
+			$this->assertFalse( $this->sut->handle( time(), 'error', 'Test message', array( 'remote-logging' => true ) ) );
 		}
 
 		/**
@@ -435,7 +443,7 @@ namespace Automattic\WooCommerce\Tests\Internal\Logging {
 				3
 			);
 
-			$this->assertTrue( $this->sut->handle( time(), 'critical', 'Test message', array() ) );
+			$this->assertTrue( $this->sut->handle( time(), 'critical', 'Test message', array( 'remote-logging' => true ) ) );
 			$this->assertTrue( WC_Rate_Limiter::retried_too_soon( RemoteLogger::RATE_LIMIT_ID ) );
 		}
 
@@ -462,7 +470,7 @@ namespace Automattic\WooCommerce\Tests\Internal\Logging {
 				3
 			);
 
-			$this->assertFalse( $this->sut->handle( time(), 'critical', 'Test message', array() ) );
+			$this->assertFalse( $this->sut->handle( time(), 'critical', 'Test message', array( 'remote-logging' => true ) ) );
 			$this->assertTrue( WC_Rate_Limiter::retried_too_soon( RemoteLogger::RATE_LIMIT_ID ) );
 		}