GlueDev / laravel-stackdriver

Enables logging, tracing and error reporting to Google Stackdriver for Laravel
MIT License
47 stars 26 forks source link

Error reporting is missing request? #16

Closed cerw closed 3 years ago

cerw commented 4 years ago

Hello there

I have read the documentation on https://cloud.google.com/error-reporting/docs/viewing-errors and it seem they do capture request (HTTP status, URL etc) but in our implementation we missing that info?

Ours:

image

Googles: image

Are I am missing something?

THanks

DiederikvandenB commented 4 years ago

I took a look, and it seems like Google's error reporting class for PHP does not support this out of the box. This package uses Google's own Bootstrap.php for setting up the necessary error/exception handlers. You can find the file here.

As you can see, the context array simply does not include the HttpRequestContext.

The reason why you do see a screenshot of this working is because of some nice Google magic which they do behind the screens only when you use Google App Service.

So there's two solutions for this issue in my opinion:

  1. We fix this ourselves by overwriting the Bootstrap.php.
  2. It is fixed upstream by Google, simply by allowing an extra parameter ($context?) to the handler functions of Bootstrap.php.

My preference is option 2 so we do not have to modify/overwrite Google's own classes.

cerw commented 4 years ago

Hello

thanks for looking into this, I think this google product is missing a lots of features compete to bugsnag and Sentry. Not sure if its worth implementing it.. atm its very basic.

I love the tracing bit the most! Do you use it for production?

DiederikvandenB commented 4 years ago

As far as I know, neither Bugsnag nor Sentry offers tracing, right? And I don’t think either platform is ideal for storing logs.

But granted, their error reporting SDKs are much better, and so is their UI. Therefore, it can make perfect sense to only use this package for tracing and logging and for example Sentry for error reporting.

And yes, we do use this in production.

cerw commented 4 years ago

Yes will disable the error tracking, but logging and tracing work well! Need to get the batch demon working was getting some permission issue running as a user.

How many traces do you have per month? Is the cost manageable?

thank you.

P

vincentrolfs commented 4 years ago

@DiederikvandenB I think a fix for this on our end would be useful. It doesn't look like Google is fixing this anytime soon. I can provide a PR if you want.

It looks to me like all we need to do is modifiy StackdriverExceptionHandler::report by changing the call Bootstrap::exceptionHandler($exception) to

$message = sprintf('PHP Notice: %s', (string)$exception);

// This was tweaked to include more data in the logging
$psrLogger->error($message, [
    'context' => [
        'reportLocation' => [
            'filePath' => $exception->getFile(),
            'lineNumber' => $exception->getLine(),
            'functionName' =>
                self::getFunctionNameForReport($exception->getTrace()),
        ],
        'user' => (string) (Auth::user()->id ?? ''),
        'httpRequest' => [
            'url' => Request::url(),
            'method' => Request::method(),
            'userAgent' => Request::userAgent(),
        ],
    ],
    'serviceContext' => [
        'service' => $psrLogger->getMetadataProvider()->serviceId(),
        'version' => $psrLogger->getMetadataProvider()->versionId(),
    ],
]);

Annoyingly, Bootstrap::getFunctionNameForReport is private static, so we would have to copy that as well (but the function is small). It is not so nice, but error reporting without the request data is crippling. What do you think?

DiederikvandenB commented 4 years ago

Thanks @vincentrolfs for your input. It would be great if you could open a PR for this!

I'll have a look at the Bootstrap class function then.

vincentrolfs commented 4 years ago

@DiederikvandenB I created pull request #20 . I also took care of Bootstrap::getFunctionNameForReport:)