Remove $context['user'] from LoggerChannel::log

Added the patch removing the line

+++ b/core/modules/dblog/tests/src/Kernel/DbLogControllerTest.php
@@ -76,4 +76,30 @@ public function testDbLogCorrupted() {
+   * Test that 'user' variable is no longer available by default in context array.

I have two small observations. One line function summary is exceeding 80 characters. Changing the variable => key will reduce the length.

+++ b/core/modules/syslog/tests/src/Kernel/SyslogTest.php
@@ -75,4 +75,12 @@ public function testSyslogSeverity() {
+   * Test that 'user' variable is no longer available by default in context array.

Same as above.

"Test that 'user' variable is no longer available by default in context array."

I could see this comment summary only in core/modules/syslog/tests/src/Kernel/SyslogTest.php file.

Changed accordingly.

Changing the status to 'Needs Review'

+++ b/core/modules/syslog/tests/src/Kernel/SyslogTest.php
@@ -75,4 +75,12 @@ public function testSyslogSeverity() {
 
+  /**
+   * Test that 'user' key is no longer available by default in context array.
+   */
+  public function testUserInContextArray() {
+    \Drupal::logger('my_module')->warning('Hello {user}');
+    \Drupal::logger('my_module')->warning('Hello {users_field_data}');
+  }
+

I don't think we need to add a test here. It's useful to have the failing test in this issue to show the bug, but in terms of adding the code to core, it's adding a test for something that no-longer exists in the code base.

Thanks. From #13 it looks like contrib loggers are using this? If they are then we might not be able to remove this.

In \Drupal\Core\Logger\LoggerChannel::log() we're still merging in a user default. Should we be doing that? In some ways it acts as a BC layer but, on the other hand, it is super confusing.

    // Merge in defaults.
    $context += [
      'channel' => $this->channel,
      'link' => '',
      'user' => NULL,
      'uid' => 0,
      'request_uri' => '',
      'referer' => '',
      'ip' => '',
      'timestamp' => time(),
    ];

Also, whilst we are here can we create a follow up to discuss removing the try{ } catch {}

      try {
        if ($this->currentUser) {
          $context['uid'] = $this->currentUser->id();
        }
      }
      catch (\Exception $e) {
        // An exception might be thrown if the database connection is not
        // available or due to another unexpected reason. It is more important
        // to log the error that we already have so any additional exceptions
        // are ignored.
      }

As far as I can see there is never a situation anymore when getting the ID from the current_user service can throw an exception. This was resolved in #2753733: AccountProxy can do unnecessary user loads to get an ID

I can confirm that the patch #20 applies cleanly and remove the $context['user']. After applying the patch I was not able to find any reference to user in $context.

curl https://www.drupal.org/files/issues/2018-07-28/2986294-20.patch | git apply -v
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   801  100   801    0     0   1448      0 --:--:-- --:--:-- --:--:--  1451
Checking patch core/lib/Drupal/Core/Logger/LoggerChannel.php...
Applied patch core/lib/Drupal/Core/Logger/LoggerChannel.php cleanly.

grep -rn 'user' core/lib/Drupal/Core/Logger
core/lib/Drupal/Core/Logger/LoggerChannelInterface.php:21: * of log(): request_uri, referer, ip, user, uid.
core/lib/Drupal/Core/Logger/LoggerChannelInterface.php:46:   * Sets the current user.
core/lib/Drupal/Core/Logger/LoggerChannelInterface.php:48:   * @param \Drupal\Core\Session\AccountInterface|null $current_user
core/lib/Drupal/Core/Logger/LoggerChannelInterface.php:49:   *   The current user object.
core/lib/Drupal/Core/Logger/LoggerChannelInterface.php:51:  public function setCurrentUser(AccountInterface $current_user = NULL);
core/lib/Drupal/Core/Logger/LoggerChannelFactory.php:36:      // If we have a container set the request_stack and current_user services
core/lib/Drupal/Core/Logger/LoggerChannelFactory.php:41:        $instance->setCurrentUser($this->container->get('current_user'));
core/lib/Drupal/Core/Logger/LoggerChannel.php:75:   * The current user object.
core/lib/Drupal/Core/Logger/LoggerChannel.php:150:  public function setCurrentUser(AccountInterface $current_user = NULL) {
core/lib/Drupal/Core/Logger/LoggerChannel.php:151:    $this->currentUser = $current_user;
core/lib/Drupal/Core/Logger/LogMessageParser.php:26:        // If the message is "User {username} created" the variable key will be
core/lib/Drupal/Core/Logger/LogMessageParser.php:27:        // just "username".

The follow-up: #2998727: Remove unnecessary try/catch from logger.

Committed 3fa89d6 and pushed to 8.7.x. Thanks!

Also opened #2998731: Remove $context['user'] from DbLogTest and FakeLogEntries as a followup for some test classes that still include the 'user' parameter.

publish the change record