Metrics events - add utils to support mixpanel integration and persisting user settings in shell. #323

janedegtiareva · 2020-04-21T23:57:01Z

utils/eventtracking.js

mikedotexe · 2020-04-22T17:13:33Z

In standup I heard that we wanted to reach consensus on something? I don't see any details on what is being asked. Looking at the code, this seems great to me. I think adding events to every command is the only way to go. Can't wait to see this live!
Looks like we've got some linting issues, FYI

commands/create-account.js

utils/settings.js

test/testsettings

ilblackdragon · 2020-04-22T19:36:53Z

utils/settings.js

+// Persistent shell settings
+
+const getShellSettings = () => {
+    const nearPath = path.join(homedir, '.near-config');


should this be .near or .near-shell or .neardev?

also let's make this a constant? same for settings.js

should this be .near or .near-shell or .neardev?

@ilblackdragon I recommended Jane put it here as it fits with the structure I laid out in the NEP.
https://github.com/nearprotocol/NEPs/blob/974887126800be47a58e968fac5b997db7d50f7d/text/0000-near-shell.md#settings-config-and-key-management

─ awesome-near-project ⟵ NEAR dApp ├── .near-config ⟵ Stores shell-experience settings and connection configuration │ ├── connections ⟵ Contains files used to connect and deploy a contract (except keys) │ │ ├── default.js ⟵ Default configuration is for development │ │ └── localnet.js ⟵ Custom connection added by user for localnet │ └── settings.js ⟵ Stores near-shell settings, how shell behaves when run in this project └── .near-credentials ⟵ Previously "neardev" this directory contains private key inforamtion └── default └── alice.json ⟵ NEAR account "alice" has private key information here

(the same structure also appears in the home directory)
Based on the comment in the shell NEP, I understood that we were not going to put anything in ~/.near as that's reserved for validator data. We can modify the NEP to be .neardev if we want instead of .near-config. Currently neardev isn't a great name and holds private keys, which the NEP calls to change to a more informative directory .near-credentials that folks aren't likely to accidentally revision. Anyway, that's the thought process. Again, we can modify the NEP.

moved string literals to const. @mikedotexe thanks for the context for dir structure!

Do we need configs inside project directory?
I really think we should not store any credentials inside project directory - because that's the easiest way to lose them. E.g. #198
I also think settings should live in common directory for all projects.

@ilblackdragon we don't need credentials, but per-project config is necessary. At the very least, it's important for making sure that people have updated deps e.g. shell itself. I agree that creds and global config should be in a common dir for everything.

@potatodepaulo read the NEP and left comments there - near/NEPs#31 (review)
High level, I don't think there is need in local configuration folder, especially one that starts with . as those directories should be gitignored. package.json can contain settings that should be "clonnable".

Can we limit the discussion to user defined settings here in order to get the PR in for RL1?

utils/eventtracking.js

This is instead of having a file in the repository

mikedotexe · 2020-04-23T16:53:40Z

Merge conflict 🎗️

behaviary · 2020-04-23T17:33:05Z

@janedegtiareva looks like it's asking CI to opt in and that's why the build is breaking for this PR.

ilblackdragon · 2020-04-23T20:19:31Z

@janedegtiareva we for sure need to figure out how to not show this in CI - we will have this problem in all examples.

janedegtiareva · 2020-04-23T22:01:06Z

@janedegtiareva we for sure need to figure out how to not show this in CI - we will have this problem in all examples.

This was temporary not working due to changing some things due to comments. Back to working again.

janedegtiareva · 2020-04-23T22:01:36Z

Merge conflict 🎗️

resolved

janedegtiareva · 2020-04-23T22:02:09Z

@janedegtiareva looks like it's asking CI to opt in and that's why the build is breaking for this PR.

Yeah this was temporarily broken due to commit /addressing comments churn. Back to working now.

mikedotexe · 2020-04-24T17:46:34Z

utils/eventtracking.js

+    EVENT_ID_TX_STATUS_START: 'shell_tx_status_start',
+    EVENT_ID_TX_STATUS_SUCCESS: 'shell_tx_status_success',
+    EVENT_ID_LOGIN: 'shell_login',
+    EVENT_ID_DEPLOY: 'shell_deploy',


that's already addressed in the part 2 of this effort which actually does events for all the things. can we let this be to avoid unnecessary churn?

mikedotexe · 2020-04-24T17:46:43Z

utils/eventtracking.js

+    EVENT_ID_TX_STATUS_SUCCESS: 'shell_tx_status_success',
+    EVENT_ID_LOGIN: 'shell_login',
+    EVENT_ID_DEPLOY: 'shell_deploy',
+    EVENT_ID_DEV_DEPLOY: 'shell_dev_deploy',


mikedotexe · 2020-04-24T17:47:04Z

utils/eventtracking.js

+    EVENT_ID_ACCOUNT_KEYS_SUCCESS: 'shell_account_keys_success',
+    EVENT_ID_TX_STATUS_START: 'shell_tx_status_start',
+    EVENT_ID_TX_STATUS_SUCCESS: 'shell_tx_status_success',
+    EVENT_ID_LOGIN: 'shell_login',


janedegtiareva added 14 commits April 15, 2020 17:19

Initial stab at dev event tracking (E2E with mixpanel works)

e2bc3c9

Clean up eventtracking util class

d986761

Add error handling to the mixpanel calls

b229756

Pull out settings keys into constants

c799108

Fix lint errors

fd64c97

Remove extra test code from main login command.

96ddf5a

Add example of tracking usage in viewState

1025c64

Undo accidental change in login

fc93a27

Fix lint errors

83b5c51

Pull out saving settings into a separate util

62b8288

Add test settings file to init with on ci

e306f5c

Remove accountId from view state tracking (PII)

eec7065

Fix typo in test settings setup

1e0a592

Adding more events in tracking

e6684af

mikedotexe reviewed Apr 22, 2020

View reviewed changes

utils/eventtracking.js Outdated Show resolved Hide resolved

mikedotexe reviewed Apr 22, 2020

View reviewed changes

commands/create-account.js Outdated Show resolved Hide resolved

mikedotexe reviewed Apr 22, 2020

View reviewed changes

utils/settings.js Outdated Show resolved Hide resolved

mikedotexe mentioned this pull request Apr 22, 2020

Initial stab at dev event tracking (E2E with mixpanel works) #313

Closed

ilblackdragon reviewed Apr 22, 2020

View reviewed changes

janedegtiareva changed the title ~~Metrics events~~ Metrics events - add utils to support mixpanel integration and persisting user settings in shell. Apr 22, 2020

janedegtiareva added 3 commits April 22, 2020 16:00

Address code review comments

1f79bbd

Generate test settings file on the fly from test script

f99491d

This is instead of having a file in the repository

Remove copying testsettings step from the test script

2a80c73

Update text for dev event tracking opt-in question

9ae4d96

janedegtiareva requested review from ilblackdragon, behaviary and mikedotexe April 23, 2020 19:24

janedegtiareva added 3 commits April 23, 2020 12:28

Merge branch 'master' into metrics-events

4df2336

Merge conflict cleanup

80ff9d2

Fix typo in settings key in generated test settings file

ad77f3c

janedegtiareva added 3 commits April 23, 2020 14:29

Remove tracking from create account temporarily

ab9a4b7

Fix merge error and readd event tracking to create account

cc93a4e

Remove unused import (lint)

80caf92

mikedotexe reviewed Apr 24, 2020

View reviewed changes

mikedotexe approved these changes Apr 24, 2020

View reviewed changes

janedegtiareva merged commit 92fee29 into master Apr 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Metrics events - add utils to support mixpanel integration and persisting user settings in shell. #323

Metrics events - add utils to support mixpanel integration and persisting user settings in shell. #323

janedegtiareva commented Apr 21, 2020 •

edited by gitpod-io bot

Loading

mikedotexe commented Apr 22, 2020

ilblackdragon Apr 22, 2020

ilblackdragon Apr 22, 2020

mikedotexe Apr 22, 2020

janedegtiareva Apr 22, 2020 •

edited

Loading

ilblackdragon Apr 23, 2020

behaviary Apr 23, 2020

ilblackdragon Apr 23, 2020

janedegtiareva Apr 23, 2020

mikedotexe commented Apr 23, 2020

behaviary commented Apr 23, 2020

ilblackdragon commented Apr 23, 2020

janedegtiareva commented Apr 23, 2020

janedegtiareva commented Apr 23, 2020

janedegtiareva commented Apr 23, 2020

mikedotexe Apr 24, 2020

janedegtiareva Apr 24, 2020

mikedotexe Apr 24, 2020

mikedotexe Apr 24, 2020

Metrics events - add utils to support mixpanel integration and persisting user settings in shell. #323

Metrics events - add utils to support mixpanel integration and persisting user settings in shell. #323

Conversation

janedegtiareva commented Apr 21, 2020 • edited by gitpod-io bot Loading

mikedotexe commented Apr 22, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

janedegtiareva Apr 22, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikedotexe commented Apr 23, 2020

behaviary commented Apr 23, 2020

ilblackdragon commented Apr 23, 2020

janedegtiareva commented Apr 23, 2020

janedegtiareva commented Apr 23, 2020

janedegtiareva commented Apr 23, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

janedegtiareva commented Apr 21, 2020 •

edited by gitpod-io bot

Loading

janedegtiareva Apr 22, 2020 •

edited

Loading