fix(service): Propagate Sentry Hub correctly#368
Conversation
|
Maybe worth to add something about this to AGENTS.md or similar? |
| let _ = tx.send(result); | ||
| drop(guard); | ||
| } | ||
| .bind_hub(Hub::current()), |
There was a problem hiding this comment.
I see two potential problems with this:
- Consider how this works for batch requests, which also call into
spawn_metered. They would all share the same span stack - in this case I think it would be better to useHub::new_from_top(Hub::current())instead. - The task can outlive the web request, at which point the transaction is sent. We will be losing spans from the remaining operations that continue on the server.
There was a problem hiding this comment.
- Using new from top now
- Fixed this scenario as well. It's not beautiful but it works. I've tested it and this way you get the correct parent->child span relationship as well as the correct trace id association for errors and logs
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
|
It works now, you can see the tests here https://sentry-sdks.sentry.io/issues/?project=4508694563782656&statsPeriod=1h |
jan-auer
left a comment
There was a problem hiding this comment.
I would suggest to deduplicate the boilerplate and move it into spawn_metered.
Currently, most of our Sentry traces and logs are mixed up, errors are not associated with the correct traces and logs, and errors/warnings logged inside the backends are missing the
usecaseandscopestags.This fixes that by creating a new hub, transaction and scope, and binding the new hub to the spawned tasks' future.
The new hub and transaction are necessary to handle cases where the execution of the task may outlive the web request.