Skip to content

Commit

Permalink
Add some default context to logs if we know it.
Browse files Browse the repository at this point in the history
If we know the user ID and email, add it to the log context for easier
searching for a given users error.
  • Loading branch information
taylorotwell committed Apr 2, 2017
1 parent c1a33b8 commit 23b7d6b
Showing 1 changed file with 16 additions and 1 deletion.
17 changes: 16 additions & 1 deletion src/Illuminate/Foundation/Exceptions/Handler.php
Expand Up @@ -6,6 +6,7 @@
use Psr\Log\LoggerInterface;
use Illuminate\Http\Response;
use Illuminate\Routing\Router;
use Illuminate\Support\Facades\Auth;
use Illuminate\Http\RedirectResponse;
use Illuminate\Auth\AuthenticationException;
use Illuminate\Contracts\Container\Container;
Expand Down Expand Up @@ -69,7 +70,7 @@ public function report(Exception $e)
throw $e; // throw the original exception
}

$logger->error($e);
$logger->error($e, $this->context());
}

/**
Expand Down Expand Up @@ -98,6 +99,20 @@ protected function shouldntReport(Exception $e)
}));
}

/**
* Get the default context variables for logging.
*
* @return array
*/
protected function context()
{
return array_filter([
'userId' => Auth::id(),
'email' => Auth::check() && isset(Auth::user()->email)
? Auth::user()->email : null,
]);
}

/**
* Render an exception into a response.
*
Expand Down

4 comments on commit 23b7d6b

@LukeTowers
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related issue: #19164

@robclancy
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should be checking if email is a string or using an interface on the user model to expose this data.
What if email is a relationship? You will log the entire email instance.

One of our projects uses $this->primaryEmail->email. It has an accessor for $this->email so won't break from this but what if it didn't and I wanted this extra data?

Alternate news: why even bother with the email there in the first place?

@ryanwinchester
Copy link
Contributor

@ryanwinchester ryanwinchester commented on 23b7d6b Jul 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why even bother with the email there in the first place?

+1, user id should be enough context, in my opinion

@LukeTowers
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, that should be up to the consumer of the logs to use the ID as necessary.

Please sign in to comment.