Skip to content
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

GraphQL and HTTP client improvements #720

Merged
merged 2 commits into from
Jul 25, 2023
Merged

GraphQL and HTTP client improvements #720

merged 2 commits into from
Jul 25, 2023

Conversation

stayallive
Copy link
Collaborator

  • Set api_target on GraphQL request bodies to graphql to allow Sentry to highlight it nicely

    image

    Since we are not technically doing a HTTP client request but we are the GraphQL server I have not included the response since we have never done that in PHP yet. And since I couldn't find any docs on the GraphQL server thingies I tried this (2d35c89) and it looks very nice but not a 100% confident it is "correct".

  • Add missing response_body_size and request_body_size to HTTP client breadcrumbs (https://develop.sentry.dev/sdk/features/#http-client-integrations)

@cleptric
Copy link
Member

cleptric commented Jul 5, 2023

@stayallive Want to move this forward? We can follow up on response handling and errors in another PR.

@stayallive stayallive marked this pull request as ready for review July 12, 2023 07:06
@stayallive
Copy link
Collaborator Author

I'm pretty happy with this 👍

@cleptric cleptric merged commit e950109 into master Jul 25, 2023
@cleptric cleptric deleted the improvements branch July 25, 2023 17:07
@smeubank
Copy link
Member

do we need docs changes for PHP for this?

@cleptric
Copy link
Member

As we talked about during our daily, there are no changes needed in a user's application to enable the improved GrapQL request payload highlighting.
GraphQL errors are not part of this PR. This might come as a follow-up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants