Add Strawberry GraphQL integration

[sentrivana]

Capture GraphQL errors and spans when using Strawberry server side.

This is a port of the existing Sentry tracing extension in Strawberry with added error monitoring.

The integration has an option called async_execution which controls whether to hook into Strawberry sync or async. If not provided, we try to guess based on whether an async web framework is installed.

Example event in Sentry: https://sentry-sdks.sentry.io/issues/4507792283/events/c517f630fef54a378affa9602ccf3ff8/

Strawberry is one of three integrations for #2257, split from #2381.

[sentrivana]

Hey @patrick91, here's a shot at porting the 🍓 Sentry tracing extension to the Sentry SDK and adding error monitoring. Any feedback is welcome!

Some notes:

• The integration, if enabled, will automatically remove the existing Strawberry Sentry tracing extension, if present, to avoid conflicts.
• The error monitoring will only work if folks don't override the default process_result. This is not optimal but it was the most straightforward place to hook into.
• I've made some changes to the tracing extension, mostly renaming things (e.g. span ops) to comply with our and/or OTel conventions. Also changed all span.set_tags to span.set_data since the SDK ideally shouldn't be setting span tags.

[patrick91]

we spoke on discord about this, I'm very excited to see this landing! From our side we'll add some code to make it easier to hook into the process result code (so users can still override it)

I'll also make sure to have an integration test on our side once this lands, so we don't break the integration in future ☺️

[patrick91]

Made the PR here: strawberry-graphql/strawberry#3127 @sentrivana let me know if that looks ok to you!

[sentrivana]

Thanks @patrick91 for being awesome and helping us integrate with Strawberry better! This PR now utilizes a new hook released with Strawberry 0.209.5.

[szokeasaurusrex]

I have suggested a few changes. But, generally looks good!

[szokeasaurusrex]

Maybe we could wrap it in a try/except block like so? Your choice, I think either way is good

Suggested change

	elif event.get("request", {}).get("data"):
	del event["request"]["data"]
	else:
	try:
	del event["request"]["data"]
	except KeyError:
	pass

[sentrivana]

I know that PEP8 prefers what you suggested here but from a personal taste point of view I prefer the original because I find it more concise and there is less visual clutter. So if you're fine with it I'd keep the original.

[sentrivana]

Made one slight change to it though: 7831520 So that, if for some reason event["request"] is there and is None, we don't get an AttributeError.

[szokeasaurusrex]

I have thought about this a bit more, and I think I would really prefer to switch to the try/except syntax, since I think it is much more readable for people looking at this code for the first time.

With the try/except block, the code reads in plain English as "If we want to send PII, send it, otherwise ensure we don't send any request data by deleting it if it's there." It is immediately obvious what the code does.

With the elif statement, the code reads in plain English as "If we want to send PII, send it, otherwise check whether the event has a request (treat the request as an empty dictionary if it doesn't) and check whether the event's request has any data, and if it does, delete the event request data." In this situation, the code's objective is less obvious; only after thoroughly reading the contents of the elif condition and the statement it contains is it clear that the code here is designed to ensure the request has no data when we send the event.

Although the elif version is more concise, this concision comes at the expense of readability and understandability, so I think the try/except version would be preferable here.

[sentrivana]

Ok, changed: 2ccd1eb
(The TypeError is there for the event["request"] == None case -- it'd throw that instead of the AttributeError I mentioned above.)

[sentrivana]

Hope I've addressed everything @szokeasaurusrex -- please have another look when you have time.

[szokeasaurusrex]

Added one comment to your tests, also please address my reply to our try/except thread. Then, the code looks good to me! @antonpirker should review before merge to double-check that I didn't miss anything

[antonpirker]

🔥 🚀