[Gutenberg] Add Sentry React native SDK by fluiddot · Pull Request #16700 · wordpress-mobile/WordPress-iOS

fluiddot · 2021-06-17T10:45:51Z

gutenberg-mobile PR: wordpress-mobile/gutenberg-mobile#3629
gutenberg PR: WordPress/gutenberg#32768
Automattic-Tracks-iOS PR: Automattic/Automattic-Tracks-iOS#183

To test:
Follow the testing instructions from gutenberg PR: WordPress/gutenberg#32768

Regression Notes

Potential unintended areas of impact
The lanes (Fastfile) related to building and uploading the build as they're using a new lane I've added for uploading the source maps to Sentry.
What I did to test those areas of impact (or what existing automated tests I relied on)
I manually tested the lane by running the command bundle exec fastlane upload_gutenberg_sourcemaps internal:true but I haven't tested the ones related to building and uploading builds.
What automated tests I added (or what prevented me from doing so)
N/A

PR submission checklist:

I have completed the Regression Notes.
I have considered adding unit tests for my changes.
I have considered adding accessibility improvements for my changes.
I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

peril-wordpress-mobile · 2021-06-17T10:45:56Z

You can trigger an installable build for these changes by visiting CircleCI here.

peril-wordpress-mobile · 2021-06-17T10:45:56Z

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

fluiddot · 2021-06-17T15:10:37Z

+        DispatchQueue.main.async {
+            WordPressAppDelegate.crashLogging?.logEnvelope(envelope)
+        }


Calls to WordPressAppDelegate are required to be done in the main thread.

fluiddot · 2021-06-17T15:14:21Z

+    FileUtils.cp(File.join(gutenberg_bundle, 'App.js'), File.join(sourcemaps_folder, 'main.jsbundle'))
+    FileUtils.cp(File.join(gutenberg_bundle, 'App.js.map'), File.join(sourcemaps_folder, 'main.jsbundle.map'))


It's important that the source map files have specific names, otherwise, Sentry doesn't un-minify the stack traces.

fluiddot · 2021-06-17T15:16:26Z

+  # bundle exec fastlane upload_gutenberg_sourcemaps internal:true
+  #####################################################################################
+  lane :upload_gutenberg_sourcemaps do | options |
+    build_version = ios_get_build_version(internal: options[:internal])


The build version is different depending on the internal option, here is an example:

internal: true: 17.6.0.20210614 (reference)

internal: false: 17.6.0.0 (reference)

# Conflicts: # Podfile # Podfile.lock # fastlane/Fastfile

fluiddot · 2021-06-24T10:44:58Z

@mokagio @jkmassel I've added a new lane to the Fastfile to upload the React native source maps to Sentry, I'd appreciate it if you could take a look at that part just in case I'm overlooking anything, thanks 🙇 !

antonis

Tested via WordPress/gutenberg#32768 (review)
The code changes LGTM too 🎉

mokagio

Thank you for jumping into adding logic to the Fastlane automation @fluiddot 👏 🙇‍♂️

I only left nitpick / minor improvement comments

mokagio · 2021-06-30T01:40:44Z

+      sourcemap: sourcemaps_folder
+    )
+
+    FileUtils.rm_rf(sourcemaps_folder)


Ruby suggestion: there's a way to run this code that depends on a temporary directory in a block, so that even if something fails, the runtime will ensure the directory is removed.

It should be something like:

require 'tmpdir' Dir.mktmpdir do |sourcemaps_folder| # all the code should "just work" the way it's written right now end

Thanks for the suggestion 🙇 ! I'll update the code to use the temporary directory block.

I applied your suggestion in this commit and run some tests, it worked like charm 🎊 .

mokagio · 2021-06-30T01:41:20Z

+      version: build_version,
+      dist: build_version,


Just to check, what's the difference between these two parameters and why is it okay for them to have the same value?

They are documented here, the version value should match the one set here. However, I'm not sure about what to use for dist, when I checked Sentry events, I found that they had the same value (see attached examples) so I went ahead and use the same version value.

I've just checked from where the Sentry iOS SDK takes the dist value (code reference) and it's CFBundleVersion, so looks like that in the end it's actually the same value 😄 . One thing I could try is to remove the dist parameter from the action and check if it affects somehow 👍.

I've done a test by removing the dist parameter and so far it works, however, this change also implies initializing the SDK in the RN side without that parameter (code reference).

To be honest, I don't have a strong opinion on whether include it or not, wdyt?

@fluiddot I dug a bit into the documents and found a definition of what the dist value is meant to be here which matches your finding:

You will also need to pass a distribution to dist for events to be attributed correctly to a release. Usually this value is the build number, for example 52. This value needs to be unique to only each release.

Given that, your original approach of passing the build number was spot on.

The snippet in that pages shows the release as something that looks more like a node module name that the build number

Sentry.init({ release: "my-project-name@2.3.12", dist: "52", });

I guess they're making a bit of confusion on their end between release, dist, version, and build in the attempt of supporting different platforms? 🤷‍♂️

From what I see in your explanation and screenshots, this setup seems fine. 👍

Co-authored-by: Gio Lodi <giovanni.lodi42@gmail.com>

Include details about the use of `rewrite` and `strip_common_prefix` params of `sentry_upload_sourcemap` Fastlane action. Co-authored-by: Gio Lodi <giovanni.lodi42@gmail.com>

peril-wordpress-mobile · 2021-07-05T12:03:43Z

	Fails
🚫	Podfile: reference to a commit hash

Generated by 🚫 dangerJS

fluiddot · 2021-07-05T12:07:31Z

As posted in this comment, the Sentry native module has been moved to the WordPress-iOS project.

# Conflicts: # Podfile # Podfile.lock

…r.swift Co-authored-by: Gio Lodi <giovanni.lodi42@gmail.com>

twstokes · 2023-08-25T21:43:21Z

👋 I'm going to close this PR due to lack of activity, but please reopen if this was an error. Thank you.

fluiddot added 4 commits June 10, 2021 17:11

Add bridge methods for React native crash logging

99596ca

Autocorrect lint issues

ed72263

Add getSentryUser to Gutenberg view controller

2ef1920

Add upload Gutenberg source maps lane to Fastfile

edd59e5

fluiddot self-assigned this Jun 17, 2021

fluiddot added the Gutenberg Editing and display of Gutenberg blocks. label Jun 17, 2021

Update Tracks ref

23d2a62

fluiddot commented Jun 17, 2021

View reviewed changes

Comment thread fastlane/Fastfile Outdated

fluiddot commented Jun 17, 2021

View reviewed changes

fluiddot added 5 commits June 17, 2021 17:40

Update Podfile.lock

4d40f39

Update Gutenberg Mobile ref

6440d98

Change internal value in build_and_upload_internal lane

1240e89

Merge branch 'develop' into gutenberg/add/sentry-react-native

86a6ca9

# Conflicts: # Podfile # Podfile.lock # fastlane/Fastfile

Update Gutenberg Mobile ref

70aa63d

fluiddot marked this pull request as ready for review June 24, 2021 10:43

fluiddot mentioned this pull request Jun 24, 2021

Improve integration with sentry wordpress-mobile/gutenberg-mobile#1187

Closed

antonis approved these changes Jun 25, 2021

View reviewed changes

fluiddot requested review from jkmassel and mokagio June 25, 2021 15:31

mokagio reviewed Jun 30, 2021

View reviewed changes

fluiddot and others added 2 commits July 1, 2021 12:31

Remove redundant comment from Fastfile file

32ff050

Co-authored-by: Gio Lodi <giovanni.lodi42@gmail.com>

Remove redundant comment from Fastfile file

d1f3788

Co-authored-by: Gio Lodi <giovanni.lodi42@gmail.com>

fluiddot and others added 7 commits July 1, 2021 12:31

Remove redundant comment from Fastfile file

cff282e

Co-authored-by: Gio Lodi <giovanni.lodi42@gmail.com>

Add details to RN bundle naming in Fastfile file

73aadc9

Co-authored-by: Gio Lodi <giovanni.lodi42@gmail.com>

Add info comments to upload_gutenberg_sourcemaps lane

98f7252

Include details about the use of `rewrite` and `strip_common_prefix` params of `sentry_upload_sourcemap` Fastlane action. Co-authored-by: Gio Lodi <giovanni.lodi42@gmail.com>

Use temporary dir for uploading JS source maps

20f1892

Add Gutenberg Sentry native module

ac313a9

Rename RNSentry to GutenbergSentry

14f6cd7

Update Gutenberg Mobile ref

2adce8c

Apply swift lint autocorrection

04d18c8

fluiddot added 2 commits July 9, 2021 13:24

Rename gutenberg sentry options function

8d591c8

Update Gutenberg Mobile ref

7fb5928

fluiddot requested a review from mchowning July 9, 2021 12:13

Merge branch 'develop' into gutenberg/add/sentry-react-native

3f1a4fc

# Conflicts: # Podfile # Podfile.lock

mokagio reviewed Jul 28, 2021

View reviewed changes

Comment thread WordPress/Classes/ViewRelated/Gutenberg/GutenbergViewController.swift Outdated

Update WordPress/Classes/ViewRelated/Gutenberg/GutenbergViewControlle…

848924d

…r.swift Co-authored-by: Gio Lodi <giovanni.lodi42@gmail.com>

twstokes closed this Aug 25, 2023

jkmassel deleted the gutenberg/add/sentry-react-native branch July 26, 2024 18:18

		FileUtils.cp(File.join(gutenberg_bundle, 'App.js'), File.join(sourcemaps_folder, 'main.jsbundle'))
		FileUtils.cp(File.join(gutenberg_bundle, 'App.js.map'), File.join(sourcemaps_folder, 'main.jsbundle.map'))

Conversation

fluiddot commented Jun 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Regression Notes

Uh oh!

peril-wordpress-mobile Bot commented Jun 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peril-wordpress-mobile Bot commented Jun 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fluiddot commented Jun 24, 2021

Uh oh!

antonis left a comment

Choose a reason for hiding this comment

Uh oh!

mokagio left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peril-wordpress-mobile Bot commented Jul 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fluiddot commented Jul 5, 2021

Uh oh!

Uh oh!

twstokes commented Aug 25, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

fluiddot commented Jun 17, 2021 •

edited

Loading

peril-wordpress-mobile Bot commented Jun 17, 2021 •

edited

Loading

peril-wordpress-mobile Bot commented Jun 17, 2021 •

edited

Loading

peril-wordpress-mobile Bot commented Jul 5, 2021 •

edited

Loading