Replace laravel-stackdriver package
#731
Conversation
via `composer require`
| * Register the exception handling callbacks for the application. | ||
| */ | ||
| public function report(Throwable $e) | ||
| public function register() |
There was a problem hiding this comment.
We talked about this; we just wanted to check that this will behave in the same way as before but is the new recommended way to do this.
There was a problem hiding this comment.
Looking at the docs this seems to be recommended, although I now realized a reason to override the report function may be if you want to call it yourself when there is no Exception (see section The report helper)
There was a problem hiding this comment.
I don't think we do this anywhere though. edit: not true, there are in fact 3 occurences of this in our code.
There was a problem hiding this comment.
Okay I have to correct myself again, after testing locally I could verify that the handler still gets called this way, so we should be good.
tarrow
left a comment
tarrow
left a comment
There was a problem hiding this comment.
Assuming this one comment is "fine" and that the error reports coming out look the same then I'm happy for this to me merged :)
|
This is an example of the reported errors that I triggered on monday on staging for testing this: Unfortunately it seems there is no matching naturally ocurring instance of this exact error in the last 30 days but what we still can see here is that it gets reported just as other errors before and it also turns up in the console logs edit: actually looking at the occurences graph it seems like it also appeared last weekend but I struggle to find it in the console UI |
| * Register the exception handling callbacks for the application. | ||
| */ | ||
| public function report(Throwable $e) | ||
| public function register() |
| 'logName' => env('STACKDRIVER_LOGGING_NAME', 'error-log'), | ||
| 'serviceId' => env('STACKDRIVER_ERROR_REPORTING_SERVICE_ID', env('APP_NAME')), | ||
| 'versionId' => env('STACKDRIVER_ERROR_REPORTING_VERSION_ID', '1.0.0'), | ||
| 'LoggingClient' => [ |
There was a problem hiding this comment.
As we mentioned in person it could be good to take 5mins to check if there is a way to make this logging appears as though its from a k8s container and add all the appropriate metadata to make it good; if not then no worries!
There was a problem hiding this comment.
I dug a bit into this and summarized my findings in the ticket. Seems like it's possible but also couldnt make it work: https://phabricator.wikimedia.org/T344377#9518149
There was a problem hiding this comment.
Cool, let's just go ahead and merge this so we're unblocked on the laravel 10 update. I think we should then (not immediately) revisit our whole strategy and consider "just" logging to stdout/stderr similar to mediawiki
There was a problem hiding this comment.
I think we should then (not immediately) revisit our whole strategy and consider "just" logging to stdout/stderr similar to mediawiki
sounds interesting, happy to talk about and improve this
* wip composer.json * WIP - departure to integrate https://github.com/googleapis/google-cloud-php-errorreporting * replace `gluedev/laravel-stackdriver` * configure `absszero/laravel-stackdriver-error-reporting` see #731 * remove call to `registerPolicies` see https://laravel.com/docs/10.x/upgrade#register-policies * schedule redis stale tag pruning * use `$casts` property see https://laravel.com/docs/10.x/upgrade#model-dates-property * run `composer update --ignore-platform-reqs` Composer version 2.6.5 2023-10-06 10:11:52 * rename `$routeMiddleware` see https://laravel.com/docs/10.x/upgrade#middleware-aliases * Apply curated changes from laravel/laravel@9.x...10.x * test: php 8.2 * update php version in composer * remove custom stackdriver repo * upgrade to phpunit 10 * migrate phpunit configuration * re-add "absszero/laravel-stackdriver-error-reporting" * replace CORS Handler * refactor data providers * refactor `$this->dispatch` & `dispatchNow` * fix test class name * refactor some tests * update phpunit to 10.5, run composer update * migrate ElasticSearchIndexDeleteTest.php * fix deprecated string interpolation * tests: replace job to queue dispatching with calling handle() * implement ForceSearchIndex::uniqueId * move to `dispatch` as mostly recommended example
https://phabricator.wikimedia.org/T344377
gluedev/laravel-stackdriverabsszero/laravel-stackdriver-error-reporting