fix(strawberry): Remove autodetection, always use sync extension

[sentrivana]

Description

Problem

There are two ways to instrument strawberry-graphl apps: via a sync or an async extension. We enable one or the other based on what web framework is installed (async framework -> async extension, sync framework -> sync extension). This auto-detection can be overridden via an integration option.

At some point (SDK 2.0?), we added StrawberryIntegration to auto-enabling integrations, which means the brittle auto-detection kicks in as soon as someone has strawberry-graphl installed. This can lead to issues, most notably when we auto-enable the async version of the extension even though the user's Strawberry app is actually sync.

Options

1. Removing the auto-detection, always enabling the sync version if not specified otherwise.
   This way we'll never mistakenly enable async code in a sync app. We also had a report at some point that the sync extension actually performs better than async, so enabling sync by default shouldn't be a big problem in async apps.
2. Removing StrawberryIntegration from auto-enabling integrations.
   Breaking change. People might just lose their traces and errors.
3. Improving the auto-detection.
   Best option, but out of ideas how to do this.

Went with 1), all things considered it's the least breaking change (unless there's a way to do 3). Needs a big callout in the changelog anyway though.

Issues

Closes #4980

Reminders

• Please add tests to validate your changes, and lint your code using tox -e linters.
• Add GH Issue ID & Linear ID (if applicable)
• PR title should use conventional commit style (feat:, fix:, ref:, meta:)
• For external contributors: CONTRIBUTING.md, Sentry SDK development docs, Discord community

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.97%. Comparing base (e9738f6) to head (e47d930).
⚠️ Report is 7 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #4984   +/-   ##
=======================================
  Coverage   83.96%   83.97%           
=======================================
  Files         165      165           
  Lines       17085    17089    +4     
  Branches     3001     3002    +1     
=======================================
+ Hits        14346    14351    +5     
  Misses       1839     1839           
+ Partials      900      899    -1     

Files with missing lines	Coverage Δ
sentry_sdk/integrations/strawberry.py	88.44% <100.00%> (+0.23%)	⬆️

... and 1 file with indirect coverage changes

[alexander-alderman-webb]

I have two thoughts:

• Right now we patch the schema creation which seems to be agnostic between ASGI and WSGI, so there's no easy change to improve the detection logic.
• Enabling the sync integration in async apps should be okay in most cases; however, because Strawberry relies on sync_to_async from asgiref to run the sync resolvers asynchronously the scope will not be propagated in two cases: the user does not have the ThreadingIntegration enabled or the user has set propagate_scope=False in the ThreadingIntegration. See https://strawberry.rocks/docs/django/guide/resolvers.

I am worried that we would break some users by merging this because of reliance on the ThreadingIntegration.

I'll take a look if there is a way to improve the auto-detection. If not, I think we should add a warning if the ThreadingIntegration is not enabled or propagate_scope=False in the ThreadingIntegration when using the StrawberryIntegration.

[sentrivana]

Let me know if you can think of anything wrt improving auto detection; that'd be my preferred way of doing things, but since we integrate too early to know anything useful about the user's app, I don't see a way forward there.

The interop with ThreadingIntegration is a good point, but propagate_scope=False is a very obscure setting that probably only a small fraction of users have opted in to. My hunch is that the intersection of those users and users who currently have StrawberryIntegration auto-enabled in async mode is very small. We can add a warning though, doesn't hurt.

(Also, all of this is just a band-aid. The proper solution is to make the whole integration opt-in so that folks have power over which extension should be used. But we can only do that in a major.)

[alexander-alderman-webb]

Just recording: core of strawberry is agnostic between sync and async server frameworks, so I agree that there's no easy way to figure out if the async extension is safe to run.

Regarding interaction with the threading integration through sync_to_async, it would only be a problem on old asgiref versions that were needed on Python 3.6. So it's even less likely for users to be affected.

So looks good to me as is!