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

GrapheneIntegration: Enhance Transaction Naming for Errors in Graphene GraphQL Operations #2765

Closed
czyber opened this issue Feb 26, 2024 · 6 comments · Fixed by #2788
Closed
Labels

Comments

@czyber
Copy link
Contributor

czyber commented Feb 26, 2024

Problem Statement

Currently when an exception or error occurs during the processing of a GraphQL operation by Graphene, the transaction name assigned to the reported issue is often not as descriptive as it could be. This lack of specificity in the transaction naming makes it challenging to quickly identify and address the source of the problem, and there are already existing workarounds such as described in this article.

image

Solution Brainstorm

Setting the transaction name of the current scope to the operation type and the operation name, would add more meaning to the issues, and one could see which operation was called at first glance.

image

This should be just a small change - i'm happy to do it 😄

@czyber czyber added the Enhancement New feature or request label Feb 26, 2024
@antonpirker
Copy link
Member

Hey @czyber thanks for writing in. What Version of the SDK are you using?

@czyber
Copy link
Contributor Author

czyber commented Feb 27, 2024

I used 1.40.0 for the above example

@sentrivana
Copy link
Contributor

Hey @czyber, I'm not sure if we should rename the transaction -- the transaction is coming from the WSGI integration. (We should also not create a separate transaction because nested transactions are not supported.) What we do in a similar case (strawberry-graphql) is leave the WSGI transaction as is, but create a child span with all the important GraphQL data:

self.graphql_span = scope.span.start_child(
What do you think about that approach?

czyber added a commit to czyber/sentry-python that referenced this issue Mar 5, 2024
This commit adds a span for a graphql operation to the graphene integration
@czyber
Copy link
Contributor Author

czyber commented Mar 5, 2024

Hi @sentrivana, i think creating a child span should be sufficient for most cases. After thinking a bit about it, setting the transaction name of the wsgi integration is really not the way 😅

I added a child span, analogous to the way it is done for strawberry-graphql.

@sentrivana
Copy link
Contributor

Thanks @czyber, we will take a look!

czyber added a commit to czyber/sentry-python that referenced this issue Mar 8, 2024
This commit adds a span for a graphql operation to the graphene integration
czyber added a commit to czyber/sentry-python that referenced this issue Mar 8, 2024
This commit adds a span for a graphql operation to the graphene integration
@antonpirker
Copy link
Member

Hey @czyber !

We are really not the fastest at looking at contributions right now. (didnt know you also did some graphene contributions)

Is this the change you want to contribute: master...czyber:sentry-python:feature/add-graphene-child-span

(could you update your branch to the lastest sentry master and create a PR? or if there is already one, please link me to it)
Thanks!

antonpirker added a commit that referenced this issue Jul 25, 2024
This commit adds a span for a GraphQL operation to the graphene integration.

Fixes #2765

---------

Co-authored-by: Anton Pirker <anton@ignaz.at>
Co-authored-by: Daniel Szoke <7881302+szokeasaurusrex@users.noreply.github.com>
arjennienhuis pushed a commit to arjennienhuis/sentry-python that referenced this issue Sep 30, 2024
This commit adds a span for a GraphQL operation to the graphene integration.

Fixes getsentry#2765

---------

Co-authored-by: Anton Pirker <anton@ignaz.at>
Co-authored-by: Daniel Szoke <7881302+szokeasaurusrex@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants