Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Improve server side GraphQL support for spring-graphql and Nextflix DGS #2856
Improve server side GraphQL support for spring-graphql and Nextflix DGS #2856
Changes from 11 commits
426da00
10194c8
2ec2bbb
c7ab0eb
65fd0a3
0f2e929
53e1486
3ca6c27
d9ed5ec
86e964a
2864d0c
797e46c
2ce76e0
eada4d3
34e560b
dbd68dc
ff29b47
2e01876
e50321b
ab741eb
8cf8f57
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Can you set all the information besides the
data
?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 mean statuscode, headers and cookies?
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.
I can probably attach some of it for Spring in an event processor but since the response isn't finished yet it might very well be wrong / incomplete. Would you still like me to add it?
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.
Can you add it after the request is made? This does not need to be GraphQL-specific since it makes sense for every integration.
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.
I don't understand. Do you mean after the response is finished? We'd have to delay sending an event to Sentry, probably put the response stuff on the scope and pick it up from there once ready. Sounds brittle and complicated.
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.
Yes, after the request is "finished".
I don't know the internals to tell how complicated is, but we do that with transactions and spans right, we collect all the information and "capture" at the end once everything is figured out, maybe we need something similar.
It's not a must but wishable.
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.
Can we do this in a follow up PR?
Check warning on line 120 in sentry-graphql/src/main/java/io/sentry/graphql/ExceptionReporter.java
Codecov / codecov/patch
sentry-graphql/src/main/java/io/sentry/graphql/ExceptionReporter.java#L115-L120
Check warning on line 127 in sentry-graphql/src/main/java/io/sentry/graphql/ExceptionReporter.java
Codecov / codecov/patch
sentry-graphql/src/main/java/io/sentry/graphql/ExceptionReporter.java#L127
Check warning on line 129 in sentry-graphql/src/main/java/io/sentry/graphql/ExceptionReporter.java
Codecov / codecov/patch
sentry-graphql/src/main/java/io/sentry/graphql/ExceptionReporter.java#L129
Check warning on line 137 in sentry-graphql/src/main/java/io/sentry/graphql/ExceptionReporter.java
Codecov / codecov/patch
sentry-graphql/src/main/java/io/sentry/graphql/ExceptionReporter.java#L137
Check warning on line 139 in sentry-graphql/src/main/java/io/sentry/graphql/ExceptionReporter.java
Codecov / codecov/patch
sentry-graphql/src/main/java/io/sentry/graphql/ExceptionReporter.java#L139
Check warning on line 13 in sentry-graphql/src/main/java/io/sentry/graphql/GraphqlStringUtils.java
Codecov / codecov/patch
sentry-graphql/src/main/java/io/sentry/graphql/GraphqlStringUtils.java#L13
Check warning on line 23 in sentry-graphql/src/main/java/io/sentry/graphql/NoOpSubscriptionHandler.java
Codecov / codecov/patch
sentry-graphql/src/main/java/io/sentry/graphql/NoOpSubscriptionHandler.java#L22-L23