Automattic / Automattic-Tracks-iOS

Client library for tracking user events for later analysis
GNU General Public License v2.0
42 stars 12 forks source link

Update Sentry to v6 #166

Closed jkmassel closed 3 years ago

jkmassel commented 3 years ago

Updates the Sentry library to v6, which enables:

Unfortunately, the v6 library takes away our ability to manually send events to Sentry, so this PR adds that ability back by using the API directly.

Previously to this PR, testing Sentry integration was always a bit tricky because we'd need to integrate into a host app and make weird changes to it in order to test the integration.

This PR adds the ability to test Sentry functionality from the example project.

To Test:

Additional M1-specific tests:

I'd love one review on this from a mobile infrastructure engineer as well as a review from someone who has M1 hardware to test with.

AliSoftware commented 3 years ago

Podfile.lock changes / Sentry 6.1.3

While trying to test this, bundle exec pod install failed to resolve the dependency for Sentry:

$ be pod install
Analyzing dependencies
[!] CocoaPods could not find compatible versions for pod "Sentry":
  In snapshot (Podfile.lock):
    Sentry (= 6.1.2, ~> 6)

  In Podfile:
    Automattic-Tracks-iOS (from `../`) was resolved to 0.7.0, which depends on
      Sentry (~> 6)

You have either:
 * out-of-date source repos which you can update with `pod repo update` or with `pod install --repo-update`.
 * changed the constraints of dependency `Sentry` inside your development pod `Automattic-Tracks-iOS`.
   You should run `pod update Sentry` to apply changes you've made.

I had to bundle exec pod update Sentry first, which led to a bump from Sentry 6.1.2 to 6.1.3 and left the Podfile.lock with a diff accordingly.

Testing and the Secrets.swift file

Also, regarding the Shared/Secrets.swift, for anyone who might have been as confused as I was (cc @mokagio) trying to follow the testing instructions in PR description:

[EDIT: I took the liberty to edit the PR description to take those remarks ☝️ into account]

jkmassel commented 3 years ago

@mokagio and @AliSoftware, thanks for the feedback – it should all be addressed now – ready for another look when you're ready 😃

One last change – I didn't realize the schemes weren't shared – I've added them to the repo in 901bf1d3a3aa82ef4201e223892323efd400db91. That should fix the issue where in testing the "Send Test Event" wouldn't work (it needs to override the debug mode).

AliSoftware commented 3 years ago

Tested on iPhone Simulator (iPhone 11 Pro, iOS 13.5) and on Intel Mac. Was able to see both events in Sentry ("Send Test Event" + "Send Error and Wait") in both cases

I got this log in Xcode console each time I clicked the button when testing with the iPhone Simulator app. I'm not entirely sure it's expected to especially get the "Cancelling log file attachment"? 🤔

2021-01-22 16:25:22:594 TracksDemo[77872:5993852] 📜 Firing `beforeSend`
2021-01-22 16:25:22.594496+0100 TracksDemo[77872:5994015] 📜 Firing `beforeSend`
2021-01-22 16:25:22:594 TracksDemo[77872:5993852] 📜 This is a debug build
2021-01-22 16:25:22.594710+0100 TracksDemo[77872:5994015] 📜 This is a debug build
2021-01-22 16:25:22:594 TracksDemo[77872:5993852] 📜 Cancelling log file attachment – Event Logging is not initialized
2021-01-22 16:25:22.594901+0100 TracksDemo[77872:5994015] 📜 Cancelling log file attachment – Event Logging is not initialized

I also still got a diff on the Podfile.lock even after cleaning my Pods/ and working copy and redoing a pod install… but this time a different one.

git clean + pod install + git diff ``` $ git clean -dxf Removing Pods/ Removing Shared/Secrets.swift Removing TracksDemo.xcodeproj/xcuserdata/ Removing TracksDemo.xcworkspace/xcuserdata/ $ cp ~/.mobile-secrets/iOS/tracks-demo/Secrets.swift Shared/Secrets.swift $ be pod install Analyzing dependencies Downloading dependencies Installing Automattic-Tracks-iOS (0.7.1) Installing CocoaLumberjack (3.4.2) Installing Reachability (3.2) Installing Sentry (6.1.2) Installing Sodium (0.9.1) Installing UIDeviceIdentifier (1.1.4) Generating Pods project Integrating client project Pod installation complete! There is 1 dependency from the Podfile and 6 total pods installed. $ gst On branch update/sentry-to-v6 Your branch is up to date with 'origin/update/sentry-to-v6'. Changes not staged for commit: (use "git add ..." to update what will be committed) (use "git restore ..." to discard changes in working directory) modified: Podfile.lock no changes added to commit (use "git add" and/or "git commit -a") $ git diff diff --git a/TracksDemo/Podfile.lock b/TracksDemo/Podfile.lock index 712c51e..f701edb 100644 --- a/TracksDemo/Podfile.lock +++ b/TracksDemo/Podfile.lock @@ -1,5 +1,5 @@ PODS: - - Automattic-Tracks-iOS (0.7.0): + - Automattic-Tracks-iOS (0.7.1): - CocoaLumberjack (~> 3) - Reachability (~> 3) - Sentry (~> 6) @@ -34,7 +34,7 @@ EXTERNAL SOURCES: :path: "../" SPEC CHECKSUMS: - Automattic-Tracks-iOS: 01fd960098c3e6e2822531c1eeb6d03dbddd865c + Automattic-Tracks-iOS: 7ac2cc974ec22d1273bd8df9eae0cfb3193d959d CocoaLumberjack: db7cc9e464771f12054c22ff6947c5a58d43a0fd Reachability: 33e18b67625424e47b6cde6d202dce689ad7af96 Sentry: 3a6c3aeb258b297a4be08d8960955ecc722331f3 ```
jkmassel commented 3 years ago

I got this log in Xcode console each time I clicked the button when testing with the iPhone Simulator app. I'm not entirely sure it's expected to especially get the "Cancelling log file attachment"? 🤔

That's expected – the encrypted logging system isn't injected into CrashLogging, so we log the fact that we're not uploading an encrypted log for this crash. This is especially important when debugging in-app so devs aren't confused why no logs are showing up and there's no logId associated with the Sentry event.

jkmassel commented 3 years ago

Thanks for the detailed instructions – I was able to replicate that issue this time!

This time I think it was caused by the merge with develop?

But should be resolved now 😅