Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Let HubInterface implementation implement ResetInterface to not leak state on background workers #702

Closed
rvanlaak opened this issue Mar 16, 2023 · 3 comments

Comments

@rvanlaak
Copy link
Contributor

Environment

When running Symfony Messenger in background, the Sentry HubInterface is a great way of marking up the hub with breadcrumbs to provide additional debugging context around the userland code. This works especially well when using the Symfony Messenger middleware, and checking the envelope for certain stamps.

The problem is, background jobs typically run for more than one event (contrary to http requests), and thereby by default state is shared across events: https://symfony.com/doc/current/messenger.html#stateless-worker

The ResetInterface was introduced to support in that, so the container can collect services that need to be resetted after events are handled.

Steps to Reproduce

  1. add middleware or handler that adds breadcrumb to the HubInterface
  2. run messenger in background
  3. consume messages that lead to exceptions
  4. second Sentry issue report will contain the breadcrumbs of the previous event

Expected Result

One breadcrumb should be added per event.

Actual Result

As many breadcrumbs are added as events that were processed by the worker, as state is shared and not resetted by the container. So, the first issue event in Sentry has one breadcrumb, the second issue event has two (read: unintentionally includes the first breadcrumb).

@Jean85
Copy link
Collaborator

Jean85 commented Mar 17, 2023

IIRC this was intentionally left like that, and the breadcrumb limit would truncate too much data if needed. If you want a different behavior, you can probably use scopes? And push/pop them when needed.

@rvanlaak
Copy link
Contributor Author

Given that the most of the container services do get resetted when the next messenger message gets handled; I'm not sure whether I can come up with a situation where you would want breadcrumbs to last across requests/messages.

Or, did I implement adding breadcrumbs by requiring the HubInterface instance incorrectly? Is there another way how messenger middleware should add breadcrumbs that solely should last within the scope of that message?
To me the scope of a request sound comparable to the scope of a message, meaning I'd expect the state to be clean on the next request/message.

@Jean85
Copy link
Collaborator

Jean85 commented Mar 17, 2023

I searched a bit, and I found this comment: #672 (comment)

Even, this issue is a duplicate of #672. I'm closing this and cross-referencing it, please refer to that one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants