-
-
Notifications
You must be signed in to change notification settings - Fork 532
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
Remove the deprecated sentry tracing extension #3672
base: main
Are you sure you want to change the base?
Remove the deprecated sentry tracing extension #3672
Conversation
Reviewer's Guide by SourceryThis pull request removes the deprecated Class diagram for the removal of SentryTracingExtensionclassDiagram
class TracingExtensions {
+ApolloTracingExtension
+ApolloTracingExtensionSync
+DatadogTracingExtension
+DatadogTracingExtensionSync
+OpenTelemetryExtension
+OpenTelemetryExtensionSync
}
%% Removed classes
class SentryTracingExtension {
- Removed
}
class SentryTracingExtensionSync {
- Removed
}
note for SentryTracingExtension "This class was removed in favor of the official Sentry SDK integration."
note for SentryTracingExtensionSync "This class was removed in favor of the official Sentry SDK integration."
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @DoctorJohn - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟡 Documentation: 1 issue found
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Thanks for adding the Here's a preview of the changelog: After a year-long deprecation period, the To migrate, remove the Here's the tweet text:
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3672 +/- ##
==========================================
+ Coverage 96.70% 97.02% +0.31%
==========================================
Files 503 501 -2
Lines 33457 33287 -170
Branches 5618 5584 -34
==========================================
- Hits 32355 32296 -59
+ Misses 880 788 -92
+ Partials 222 203 -19 |
c885868
to
e76e43c
Compare
CodSpeed Performance ReportMerging #3672 will not alter performanceComparing Summary
|
Description
After a one-year deprecation period, this PR removes the
SentryTracingExtension
provided by Strawberry in favor of the official Sentry SDK integration.We might need to coordinate this with Sentry. It looks like their integration will assume Strawberry is not installed at all when our (deprecated) extensions cannot be imported:
https://github.com/getsentry/sentry-python/blob/1e73ce9fa12ea04250a708c14531d94827501a1d/sentry_sdk/integrations/strawberry.py#L28-L39
I created an upstream PR addressing this: getsentry/sentry-python#3649
Issues Fixed or Closed by This PR
strawberry.extensions.tracing.sentry
will be impossible to import withsentry-sdk>=3.0.0
#3590Summary by Sourcery
Remove the deprecated SentryTracingExtension in favor of the official Sentry SDK integration after a one-year deprecation period. Update documentation and remove related tests and files.
Documentation:
Tests:
Chores: