Get request data from event

[baijunyao]

It might be better to get the requested data from the event, otherwise, the src/Integration/RequestIntegration.php class will overwrite the previous data.

For example, I create a CustomRequestIntegration.php.

final class CustomRequestIntegration implements IntegrationInterface
{
    public function setupOnce(): void
    {
        Scope::addGlobalEventProcessor(function (Event $event): Event {
            $event->setRequest([
                'data' => [
                    'custom_key' => 'value'
                ]
            ]);

            return $event;
        });
    }
}

In Laravel, the order of src/Integration/RequestIntegration.php will be executed last.

https://github.com/getsentry/sentry-laravel/blob/c2494c16335da24029b73b9358cd843ec7c82620/src/Sentry/Laravel/ServiceProvider.php#L191-L203

The src/Integration/RequestIntegration.php will overwrite the custom key.

[ste93cry]

If you don't have a particular need, I would make things simpler by making the integration a noop if the request data of the event is already filled. WDYT?

[stayallive]

That the request integration does nothing when a request is already set on the event seems like a great idea.

We can also make sure that in Laravel user supplied integrations are called last to make sure they have the "greates" priority, that looks like an oversight from me having them run first (but that's offtopic for this particular issue).

---

Edit: Also keep in mind that regardless of what we change doing this results in all previous requests data being overwritten, request data is not merged. setRequest is just a setter, it overwrites any previous values which might not be what you wanted.

$event->setRequest([
    'data' => [
        'custom_key' => 'value'
    ]
]);

[Jean85]

👍 on the no-op if data is already there... As @stayallive said, the setter overwrites, so if the user provides anything, it would be completely lost now due to that.

[baijunyao]

If the user enables send_default_pii, then the user assumes that the PPI field has been filtered, and if the user customizes RequestIntegration, the user assumes that custom data can be added to the request, both of which should be allowed.

[stayallive]

There are many ways to add data to an event. You could use setTags or setExtra or maybe even use breadcrumbs. Maybe the data you are trying to add is related to the request but maybe that is not the best place to put it.

If the user wanted to keep any data set they can use getRequest and merge the data in any way the like so I don't think that is the issue here.

So we can do 2 things here. We can make sure the RequestIntegration does not overwrite the request data on an event if there is already request data set (no-op).

Or we can change the order integrations run in Laravel.

I think we should do both but at the least the last one. But both will solve your issue one way or another.

[baijunyao]

Yes, I think it would be better to change the integration order in Laravel, and the user's integration priority should be higher.
I opened a new PR getsentry/sentry-laravel#474
thanks.