-
-
Notifications
You must be signed in to change notification settings - Fork 446
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
Feat: Performance support for Android Apollo #1705
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1705 +/- ##
============================================
+ Coverage 75.01% 75.07% +0.06%
- Complexity 2104 2113 +9
============================================
Files 211 212 +1
Lines 7532 7575 +43
Branches 793 799 +6
============================================
+ Hits 5650 5687 +37
- Misses 1498 1503 +5
- Partials 384 385 +1
Continue to review full report at Codecov.
|
val span = startChild(request, activeSpan) | ||
val sentryTraceHeader = span.toSentryTrace() | ||
|
||
// we have no access to URI, no way to verify tracing origins |
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.
Should we address this before we ship it?
Sounds like we should bring tracing options (allowlist of outgoing URLs to add the header info) going forward
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.
looks like a limitation of apollo
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.
you can do with the response object, response.httpResponse.get().request().url()
but its too late
sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt
Outdated
Show resolved
Hide resolved
sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt
Outdated
Show resolved
Hide resolved
sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt
Outdated
Show resolved
Hide resolved
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.
LGTM
docs issue getsentry/sentry-docs#4151 |
📜 Description
Performance support for Android Apollo.
Apollo executes callback on another thread (or coroutine), which makes it impossible to set breadcrumbs, as the context is not propagated back to parent.
Since all spans are finished now automatically,
BeforeSpanCallback
does not allow dropping spans (this will likely change when we fix this for Feign and OkHttp).Generated classes for test purposes, are generated using Apollo plugin for the API: https://apollo-fullstack-tutorial.herokuapp.com/graphql
💡 Motivation and Context
Fixes #1331
💚 How did you test it?
Integration test.
📝 Checklist
🔮 Next steps