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.

Sign up for GitHub

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

Jump to bottom

Migrate Sentry settings to environment variables #11085

Merged
merged 1 commit into from
May 18, 2021
Merged

Migrate Sentry settings to environment variables #11085

merged 1 commit into from
May 18, 2021

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented May 13, 2021 •
edited

Sentry is now configured with environment variables, rather than with hard-coded values. This makes it easier to test Sentry functionality using a different Sentry account, as we did recently during QA of v9.5.1.

The only change for the normal build process is the introduction of the SENTRY_DSN_DEV variable, which can be set via .metamaskrc or via an environment variable. This determines where error reports are sent. It still defaults to our team Sentry account's metamask-testing project.

The sentry:publish script now requires SENTRY_ORG and SENTRY_PROJECT to be set in order to publish release artifacts. The CircleCI configuration has been updated with these values, so it should act the same as it did before. Previously we had used a CLI flag to specify the organization and project, but Sentry already natively supports these environment variables.

Manual testing steps for SENTRY_DSN_DEV

  • Create a free Sentry account and log in, and create a project. Take note of the DSN.
  • Build the extension using yarn build:test or yarn dist (only test/prod builds send Sentry errors, they are disabled for dev)
  • Install the extension
  • Do something that causes an error (it may be easiest to introduce a false error for testing)
  • Observe the network tab of whichever process the error is thrown in, and see that it's sending to the correct DSN. You should also see the error on your Sentry dashboard.

Manual testing steps for release artifact publishing:

  • Create a free Sentry account and log in, and create a project.
  • Navigate to "Settings => Developer Settings => "New Internal Integration"
  • Assign a name to this integration, and grant it a "Release" permission of "Admin".
  • After saving this new integration, take note of the token that should be listed on the integration page.
  • Run the command yarn sentry:publish with the environment variable SENTRY_AUTH_TOKEN set to the token from that integration, and with SENTRY_ORG set to your account name and SENTRY_PROJECT set to the project name you created.
  • It should complete successfully. You should see the release under "Releases" with the correct project selected. It should have 22 artifacts.

@Gudahtt Gudahtt requested review from kumavis and a team as code owners May 13, 2021 17:43
@Gudahtt Gudahtt requested a review from brad-decker May 13, 2021 17:43
@Gudahtt
Migrate Sentry settings to environment variables
b99eefb 
Sentry is now configured with environment variables, rather than with
hard-coded values. This makes it easier to test Sentry functionality
using a different Sentry account, as we did recently during QA of
v9.5.1.

The only change for the normal build process is the introduction of the
`SENTRY_DSN_DEV` variable, which can be set via `.metamaskrc` or via an
environment variable. This determines where error reports are sent. It
still defaults to our team Sentry account's `metamask-testing` project.

The `sentry:publish` script now requires SENTRY_ORG and SENTRY_PROJECT
to be set in order to publish release artifacts. The CircleCI
configuration has been updated with these values, so it should act the
same as it did before. Previously we had used a CLI flag to specify the
organization and project, but Sentry already natively supports these
environment variables [1].

[1]: https://docs.sentry.io/product/cli/configuration/#configuration-values
@Gudahtt Gudahtt force-pushed the extract-sentry-account-details-to-environment-variables branch from 5989e67 to b99eefb Compare May 13, 2021 17:44
@Gudahtt
Copy link
Member Author

Gudahtt commented May 13, 2021

I have followed both manual testing instructions, and confirmed that this works with all three of these new environment variables set to point at a different Sentry account.

@metamaskbot
Copy link
Collaborator

Builds ready [b99eefb]
Page Load Metrics (622 ± 32 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint44736094
domContentLoaded4087186196833
load4107196226832
domInteractive4087186196833

@Gudahtt Gudahtt mentioned this pull request May 14, 2021
Merged
brad-decker
brad-decker approved these changes May 17, 2021
View reviewed changes
Copy link
Contributor

@brad-decker brad-decker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

followed testing instructions for both reporting and artifact uploading and everything works well.

@Gudahtt Gudahtt merged commit 37dc19a into develop May 18, 2021
@Gudahtt Gudahtt deleted the extract-sentry-account-details-to-environment-variables branch May 18, 2021 16:26
Gudahtt added a commit that referenced this pull request May 21, 2021
@Gudahtt
Merge remote-tracking branch 'origin/develop' into sync-master
203456d 
* origin/develop: (227 commits)
  Improve UI + content for price difference notifications (#11145)
  Swaps: Create a new swap (#11124)
  Bump @metamask/controllers from 9.0.0 to 9.1.0 (#11150)
  Capture exception instead of throw error in useTransactionDisplayData (#11153)
  Fixing jest component test output errors (#11139)
  Avoid showing  "Gas price extremely low" warning in advanced tab for testnets (#11111)
  @metamask/auto-changelog@2.0.1 (#11140)
  Migrate to new CurrencyRateController (#11005)
  bump allow scripts (#11134)
  Show Sentry CLI output when uploading artifacts (#11100)
  use etherscan-link customBlockExplorer methods with customNetwork usage tracking (#11017)
  Adding notification for updated seed phrase wording (#11131)
  Bumping package.json
  Fix a condition for checking if a token should be added (#11127)
  Removing support survey notification from What's New (#11118)
  Handling custom token decimal fetch failure due to network error (#10956)
  Hide basic tab in advanced gas modal for speedup and cancel when on testnets (#11115)
  Migrate Sentry settings to environment variables (#11085)
  Update eth-ledger-bridge-keyring to v0.5.0 (#11064)
  fix metaRPCClientFactory id handling (#11116)
  ...
danjm pushed a commit that referenced this pull request Jun 6, 2021
@Gudahtt @danjm
Migrate Sentry settings to environment variables (#11085)
f33c724 
Sentry is now configured with environment variables, rather than with
hard-coded values. This makes it easier to test Sentry functionality
using a different Sentry account, as we did recently during QA of
v9.5.1.

The only change for the normal build process is the introduction of the
`SENTRY_DSN_DEV` variable, which can be set via `.metamaskrc` or via an
environment variable. This determines where error reports are sent. It
still defaults to our team Sentry account's `metamask-testing` project.

The `sentry:publish` script now requires SENTRY_ORG and SENTRY_PROJECT
to be set in order to publish release artifacts. The CircleCI
configuration has been updated with these values, so it should act the
same as it did before. Previously we had used a CLI flag to specify the
organization and project, but Sentry already natively supports these
environment variables [1].

[1]: https://docs.sentry.io/product/cli/configuration/#configuration-values
danjm pushed a commit that referenced this pull request Jun 7, 2021
@Gudahtt @danjm
Migrate Sentry settings to environment variables (#11085)
5f538f7 
Sentry is now configured with environment variables, rather than with
hard-coded values. This makes it easier to test Sentry functionality
using a different Sentry account, as we did recently during QA of
v9.5.1.

The only change for the normal build process is the introduction of the
`SENTRY_DSN_DEV` variable, which can be set via `.metamaskrc` or via an
environment variable. This determines where error reports are sent. It
still defaults to our team Sentry account's `metamask-testing` project.

The `sentry:publish` script now requires SENTRY_ORG and SENTRY_PROJECT
to be set in order to publish release artifacts. The CircleCI
configuration has been updated with these values, so it should act the
same as it did before. Previously we had used a CLI flag to specify the
organization and project, but Sentry already natively supports these
environment variables [1].

[1]: https://docs.sentry.io/product/cli/configuration/#configuration-values
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brad-decker brad-decker brad-decker approved these changes

@kumavis kumavis Awaiting requested review from kumavis kumavis is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Gudahtt @metamaskbot @brad-decker