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.

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

[Flask] bitrise pipelines #6645

Merged
merged 39 commits into from
Jul 14, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
99710ef
basic pipeline scaffold
owencraston Jun 20, 2023
f621274
flask release ios
owencraston Jun 20, 2023
95fda8a
android release scripts
owencraston Jun 20, 2023
018ff06
check for sentry props on flask build
owencraston Jun 20, 2023
3e0e16e
bitrise certificate
owencraston Jun 20, 2023
cd861a8
debug/release provisioning profiles
owencraston Jun 20, 2023
9b02f37
basic build steps
owencraston Jun 20, 2023
47b75d2
flask pipelines
owencraston Jun 20, 2023
50a3475
fix yaml
owencraston Jun 20, 2023
5247447
remap flask variables
owencraston Jun 20, 2023
5924596
remove pubnub keys
owencraston Jun 20, 2023
7b275c4
add deploy pipelines
owencraston Jun 21, 2023
56f03d4
fix file paths and undo edits to qa build
owencraston Jun 21, 2023
69ccfe9
new flask release pipelines
owencraston Jun 21, 2023
061b507
update set-versions script and try to fix sourcemaps for flask
owencraston Jun 21, 2023
df55ec1
try a different sourcemp filepath
owencraston Jun 21, 2023
e82ff36
upgrade ruby version
owencraston Jun 22, 2023
5727bc3
update signing
owencraston Jun 23, 2023
2acb01b
set env to flask
owencraston Jun 23, 2023
5ca20dd
square icons
owencraston Jun 28, 2023
5b6d2d8
manually remove icon alpha
owencraston Jun 28, 2023
48b71e8
add archive path var
owencraston Jun 28, 2023
575e326
introduce build types in build pipeline
owencraston Jun 28, 2023
2191e4c
remove echo
owencraston Jun 28, 2023
17b6758
pass params to derive sentry environment and tests
owencraston Jun 29, 2023
2a71962
new build numbers
owencraston Jul 11, 2023
37aeb8a
fix: update cookie-tough dependency (#6772)
tommasini Jul 11, 2023
80aa292
move versions into product flavours
owencraston Jul 12, 2023
78c4a01
test prod build with flask variables
owencraston Jul 13, 2023
da12ce0
cleanup deriveSentryEnvironment function
owencraston Jul 13, 2023
fe3069e
cleanup comment
owencraston Jul 13, 2023
f79aab3
reset build numbers
owencraston Jul 13, 2023
9490d89
enable deploy to android and pass in env variable
owencraston Jul 13, 2023
38d2dc6
new syntax
owencraston Jul 13, 2023
1da8ac1
is expand true
owencraston Jul 13, 2023
6dfcce6
no android pipeline
owencraston Jul 13, 2023
cbd72aa
new syntax
owencraston Jul 13, 2023
889d066
new new syntax
owencraston Jul 13, 2023
285b28c
new new new syntax
owencraston Jul 13, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion app/util/sentryUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import DefaultPreference from 'react-native-default-preference';
import { AGREED, METRICS_OPT_IN } from '../constants/storage';

const METAMASK_ENVIRONMENT = process.env['METAMASK_ENVIRONMENT'] || 'local'; // eslint-disable-line dot-notation
const METAMASK_BUILD_TYPE = process.env.METAMASK_BUILD_TYPE || 'main';

const ERROR_URL_ALLOWLIST = [
'cryptocompare.com',
Expand Down Expand Up @@ -154,8 +155,15 @@ export function setupSentry() {
const init = async () => {
const dsn = process.env.MM_SENTRY_DSN;

/* Similar to the environment logic we use on extension:
- https://github.com/MetaMask/metamask-extension/blob/34375a57e558853aab95fe35d5f278aa52b66636/app/scripts/lib/setupSentry.js#L91
*/
const environment =
__DEV__ || !METAMASK_ENVIRONMENT ? 'development' : METAMASK_ENVIRONMENT;
__DEV__ || !METAMASK_ENVIRONMENT
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add tests with explicit output and comments? This thing is getting out of hand.

Copy link
Member

Choose a reason for hiding this comment

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

On extension we found it easier to disable Sentry entirely in development builds (see https://github.com/MetaMask/metamask-extension/blob/230c0c6fa1626bc1afd68c22321f5c166425dd12/app/scripts/lib/setupSentry.js#L88). Most of our Sentry testing was done with QA or production-like builds as part of regression testing. On the rare occasion that we wanted to test Sentry with a development build, it's easy to comment out that line.

That would reduce this condition to a single ternary:

const environment = METAMASK_BUILD_TYPE === 'main' ?
  METAMASK_ENVIRONMENT :
  `${METAMASK_ENVIRONMENT}-${METAMASK_BUILD_TYPE}`;

Copy link
Member

Choose a reason for hiding this comment

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

If we wanted to go that route here, and disable Sentry for development builds, that would be simpler to do in a separate PR. Better to keep this one focused if we can.

A more direct solution to cleaning this up for now would be to split this into two steps: determine the environment, then the sentry environment. e.g.

const metamaskEnvironment = __DEV__ || !METAMASK_ENVIRONMENT ? 'development' : METAMASK_ENVIRONMENT;

const sentryEnvironment = METAMASK_BUILD_TYPE === 'main' ?
  METAMASK_ENVIRONMENT :
  `${METAMASK_ENVIRONMENT}-${METAMASK_BUILD_TYPE}`;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have went ahead and modified this logic to live inside a function I am calling deriveSentryEnvironment.

/**
 * Derives the Sentry environment based on input parameters.
 * This function is similar to the environment logic used in MetaMask extension.
 * - https://github.com/MetaMask/metamask-extension/blob/34375a57e558853aab95fe35d5f278aa52b66636/app/scripts/lib/setupSentry.js#L91
 *
 * @param {boolean} isDev - Represents if the current environment is development (__DEV__ global variable).
 * @param {string} [metamaskEnvironment='local'] - The environment MetaMask is running in
 *                                                  (process.env.METAMASK_ENVIRONMENT).
 *                                                  It defaults to 'local' if not provided.
 * @param {string} [metamaskBuildType='main'] - The build type of MetaMask
 *                                              (process.env.METAMASK_BUILD_TYPE).
 *                                              It defaults to 'main' if not provided.
 *
 * @returns {string} - The Sentry environment. Possible values are 'development', 'local',
 *                     'production', or a string in the format `${metamaskEnvironment}-${metamaskBuildType}`.
 *                     'development' is returned if 'isDev' is true or 'metamaskEnvironment' is not provided.
 *                     'metamaskEnvironment' is returned if 'metamaskBuildType' is 'main' or undefined.
 *                     `${metamaskEnvironment}-${metamaskBuildType}` is returned for other cases,
 *                     for example 'production-flask' or 'debug-flask'.
 */
export function deriveSentryEnvironment(
  isDev,
  metamaskEnvironment = 'local',
  metamaskBuildType = 'main',
) {
  const environment =
    isDev || !metamaskEnvironment
      ? 'development'
      : metamaskBuildType === 'main'
      ? metamaskEnvironment
      : `${metamaskEnvironment}-${metamaskBuildType}`;

  return environment;
}

This function is well tested in the new sentryUtils.test.ts file. I opted to not split up the meta mask environment and sentry environment to keep continuity with what we have already published in sentry. what would be the use case for having a separate metamaskEnvironment if we don't log it in sentry?

? 'development'
: METAMASK_BUILD_TYPE === 'main'
? METAMASK_ENVIRONMENT
: `${METAMASK_ENVIRONMENT}-${METAMASK_BUILD_TYPE}`;

const metricsOptIn = await DefaultPreference.get(METRICS_OPT_IN);

Expand Down
4 changes: 2 additions & 2 deletions bitrise.yml
Original file line number Diff line number Diff line change
Expand Up @@ -708,7 +708,7 @@ workflows:
- content: |-
#!/usr/bin/env bash
node -v
METAMASK_ENVIRONMENT='flask' yarn build:ios:pre-flask
METAMASK_BUILD_TYPE=flask METAMASK_ENVIRONMENT='production' yarn build:ios:pre-flask
title: iOS Sourcemaps & Build
is_always_run: false
- [email protected]:
Expand Down Expand Up @@ -756,7 +756,7 @@ workflows:
- content: |-
#!/usr/bin/env bash
node -v
METAMASK_ENVIRONMENT='flask' yarn build:android:pre-release:bundle:flask
METAMASK_BUILD_TYPE=flask METAMASK_ENVIRONMENT='production' yarn build:android:pre-release:bundle:flask
title: Build Android Pre-Release Bundle
is_always_run: false
- save-gradle-cache@1: {}
Expand Down