feat(integrations): Add message envelope to scope context

[darthf1]

Related: #703. With this PR the message envelope is added to the scope context. Let me know what you think.

Can you give me some pointers on the e2e test to add for this feature? Should I edit this somehow?

[Jean85]

I'm not sure we should enable this automatically. The envelope could contain sensible information that should not be shared with Sentry.

[darthf1]

I agree. Would you like to see is behind a config flag?

[cleptric]

Overall, this is a valuable addition.

• We need to guard the inclusion of the envelope with send_default_pii
• There are some size concerns, as too big payloads are being dropped by Relay. Are there any size limits for an envelope enforced on the Symfony side?
• This data won't be searchable, so this will be just additional context that enriches an error event.

[darthf1]

Overall, this is a valuable addition.

• We need to guard the inclusion of the envelope with send_default_pii

Will do.

I have this option set to false (I don't want/need everything it provides), but I would very much like to have messenger data available with the issue. Is it possible to differentiate? A new flag could be created, or maybe (the better option?) an example can be given how to enable the send-default-pii flag and remove everything but messenger.envelope context data.

• There are some size concerns, as too big payloads are being dropped by Relay. Are there any size limits for an envelope enforced on the Symfony side?

If I'm correct; not on SF side but on message brokers side. I guess it's easier to add a check if the serialized envelope size does not supersede 8kB, else replace the data with a "skipped" value.

• This data won't be searchable, so this will be just additional context that enriches an error event.

Is that a bad thing? I don't necessarily need it to be searchable.

[cleptric]

Until we implement RFC #62 in the PHP SDK, you would likely need to remove all unwanted data inside before_send. Not a perfect solution, but we have to work with what we have today.

Enforcing a size limit in the SDK seems reasonable. We do similar things in our RequestIntegration.

Not being able to search this data was more of a heads-up to set expectations.

[darthf1]

@cleptric I have done all the changes I would like, but I'm struggling with the tests. I would like to add a simple E2E test; capture a hard failure, fetch the last event, and see if the expected context is equal to the recorded context. How can I fetch the recorded context in that scenario?

(Or do you want me to create a unit test? I thought this was not preferable because then I have to mock Symfony\Component\Messenger\Transport\Serialization\SerializerInterface which is a black box from this libraries perspective.)

[cleptric]

@darthf1 I'm currently not sure what the best way forward is to test this. I'll probably need some time to dig into the topic but I'm currently focusing on other topics. Maybe @Jean85 can provide some pointers in the meantime.

[Jean85]

E2E tests involving the Messenger are already present in the current test suite.

All involved classes are here: https://github.com/getsentry/sentry-symfony/tree/master/tests/End2End/App/Messenger

Messages are dispatched here, in a controller: https://github.com/getsentry/sentry-symfony/blob/master/tests/End2End/App/Controller/MessengerController.php#L23-L35

Tests are here (and below): https://github.com/getsentry/sentry-symfony/blob/master/tests/End2End/End2EndTest.php#L218-L234

[getsantry]

This pull request has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you remove the label Waiting for: Community, I will leave it alone ... forever!

---

"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀