feat: Send GraphQL "operationName" in HTTP breadcrumbs

[maxchuquimia]

📜 Description

This PR attempts to support sending GraphQL operation names with existing HTTP breadcrumbs.

💡 Motivation and Context

When using GraphQL requests, Sentry's HTTP breadcrumbs are not valuable as they all show the same URL. Being able to see the operation name in a HTTP breadcrumb is extremely useful when tracking down what happened leading up to an error.

The main reason I am opening this PR now is because of Sentry's move to distributing XCFrameworks. This has made my fork unusable as pointing our app's Package.swift to our fork doesn't immediately change the downloaded .binaryTarget in Sentry's Package.swift. At a glance, the GitHub Actions in my fork are failing so I thought opening a PR may be the easiest option to enable us to update Sentry (which we now need to do to get PrivacyInfo.xcprivacy).

Relates to #3494

💚 How did you test it?

• Our original fork maxchuquimia@fa0b660 has been in production for a number of months with no issues.
• Added some unit tests before opening this PR

📝 Checklist

You have to check all boxes before merging:

• I reviewed the submitted code.
• I added tests to verify the changes.
• No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
• I updated the docs if needed.
• Review from the native team if needed.
• No breaking change or entry added to the changelog.
• No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

• Perhaps there's a better way?
• Ideally someone else tests this manually
• Unit tests could probably be expanded or improved on, they were difficult to jump in and write as I have done no other work in this project 😅

[brustolin]

Hello @maxchuquimia. Thank you so much for the help.
The PR looks very good, with tests and all.

I left a comment of something that needs changing.
The PR is also missing a CHANGELOG entry.

Nice work!!

[maxchuquimia]

I left a comment of something that needs changing. The PR is also missing a CHANGELOG entry.

@brustolin updated!

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.727%. Comparing base (99ab5d0) to head (64d7a39).

Additional details and impacted files

@@              Coverage Diff              @@
##              main     #3931       +/-   ##
=============================================
- Coverage   90.767%   90.727%   -0.040%     
=============================================
  Files          586       588        +2     
  Lines        45783     45879       +96     
  Branches     16326     16356       +30     
=============================================
+ Hits         41556     41625       +69     
- Misses        4047      4183      +136     
+ Partials       180        71      -109     

Files	Coverage Δ
Sources/Sentry/SentryNetworkTracker.m	96.369% <100.000%> (+0.098%)	⬆️
Sources/Sentry/SentryNetworkTrackingIntegration.m	100.000% <100.000%> (ø)
Sources/Sentry/SentryOptions.m	99.202% <100.000%> (+0.006%)	⬆️
...es/Swift/Extensions/URLSessionTaskExtensions.swift	100.000% <100.000%> (ø)
...Network/SentryNetworkTrackerIntegrationTests.swift	69.354% <100.000%> (+1.198%)	⬆️
...erformance/Network/SentryNetworkTrackerTests.swift	98.489% <100.000%> (+0.074%)	⬆️
Tests/SentryTests/SentryOptionsTest.m	98.621% <100.000%> (+0.002%)	⬆️
...ryTests/Swift/Extensions/URLSessionTaskTests.swift	100.000% <100.000%> (ø)

... and 38 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 99ab5d0...64d7a39. Read the comment docs.

[brustolin]

This looks good to me.
I dont see a reason to not have this feature merged.

@philipphofmann, @kahest thoughts?

[philipphofmann]

Thanks a lot for this PR. I think we only need a couple of improvements then we can merge this. Most important we should align with how we add GraphQL breadcrumbs in our Java SDK, see also our develop docs. If you can't easily retrieve operation_name, operation_type and operation_id, it's also acceptable to only do operation_name.

[maxchuquimia]

Hey @philipphofmann, cheers for your comments! I have made the following changes:

• Added missing options test
• Rewrote operation name extraction in Swift (which looks fine to me but it does mean I can no longer say I have been using the exact code in this PR in production!)
• Add further tests for operation name extraction
• Renamed graphql to operation_name in breadcrumb (I'm not at all sure how to get the operation_id, and the only way I could see to get operation_type was from a proprietary Apollo header so I left it out for now)

[philipphofmann]

Two minor things to fix. Then we can merge this. Thank you @maxchuquimia 👍

[maxchuquimia]

@philipphofmann Done! 🎉

[philipphofmann]

LGTM, thank you @maxchuquimia 🥇 .

[philipphofmann]

LGTM

[maxchuquimia]

Hey @brustolin, you mentioned "We can't use Objc Categories to extend a class" -- as part of later comments I was asked to migrate the code in this PR to Swift, does this rule apply to Swift Extensions somehow? Now that this has been merged we are attempting to use it but are receiving this crash, I'm not sure how the function could not be there seeing as it compiled just fine 😅

*** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '-[__NSCFLocalDataTask getGraphQLOperationName]: unrecognized selector sent to instance 0x1047251b0'
*** First throw call stack:
(
	0   CoreFoundation                      0x0000000180491128 __exceptionPreprocess + 172
	1   libobjc.A.dylib                     0x000000018008412c objc_exception_throw + 56
	2   CoreFoundation                      0x00000001804a5f78 +[NSObject(NSObject) instanceMethodSignatureForSelector:] + 0
	3   CoreFoundation                      0x0000000180495278 ___forwarding___ + 1280
	4   CoreFoundation                      0x000000018049759c _CF_forwarding_prep_0 + 92
	5   App                                 0x00000001106cfff8 -[SentryNetworkTracker addBreadcrumbForSessionTask:] + 732
	6   App                                 0x00000001106cee50 -[SentryNetworkTracker urlSessionTask:setState:] + 516
	7   App                                 0x00000001106acf5c __57+[SentryNetworkTrackingIntegration swizzleURLSessionTask]_block_invoke_2.33 + 64
	8   CFNetwork                           0x00000001842724a4 _CFHTTPMessageSetResponseProxyURL + 6408
	9   CFNetwork                           0x0000000184270e34 _CFHTTPMessageSetResponseProxyURL + 664
	10  CFNetwork                           0x0000000184272d20 _CFHTTPMessageSetResponseProxyURL + 8580
	11  CFNetwork                           0x00000001842e1f38 CFURLDownloadCancel + 44640
	12  CFNetwork                           0x00000001842e0288 CFURLDownloadCancel + 37296
	13  CFNetwork                           0x00000001842df820 CFURLDownloadCancel + 34632
	14  CFNetwork                           0x00000001843cadf4 _CFNetworkSetATSContext + 79752
	15  CFNetwork                           0x00000001842e2f88 CFURLDownloadCancel + 48816
	16  libdispatch.dylib                   0x0000000103d9a038 _dispatch_block_async_invoke2 + 104
	17  libdispatch.dylib                   0x0000000103d8993c _dispatch_client_callout + 16
	18  libdispatch.dylib                   0x0000000103d91bd8 _dispatch_lane_serial_drain + 916
	19  libdispatch.dylib                   0x0000000103d9291c _dispatch_lane_invoke + 420
	20  libdispatch.dylib                   0x0000000103d9f2f8 _dispatch_root_queue_drain_deferred_wlh + 324
	21  libdispatch.dylib                   0x0000000103d9e754 _dispatch_workloop_worker_thread + 488
	22  libsystem_pthread.dylib             0x0000000103ce3814 _pthread_wqthread + 284
	23  libsystem_pthread.dylib             0x0000000103ce25d4 start_wqthread + 8
)
libc++abi: terminating due to uncaught exception of type NSException

[philipphofmann]

Hmm, it could be that Swift extension methods don't work. @maxchuquimia, can you try to convert the getGraphQLOperationName to a helper function that takes URLSessionTask as an argument and see if it solves the issue? Then, we could open a quick PR to address the issue.

[maxchuquimia]

PR opened @philipphofmann - #3973
I can't test if it solves the issue due to the original xcframework issue...