-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Add web3 usage metrics, prepare for web3 removal #9144
Conversation
71ccb60
to
44298af
Compare
Builds ready [44298af]
Page Load Metrics (605 ± 18 ms)
|
44298af
to
9c64bb4
Compare
* move web3 metrics method to new middleware
7a68f35
to
004adc8
Compare
@@ -4,7 +4,7 @@ import { ethErrors } from 'eth-json-rpc-errors' | |||
/** | |||
* Create middleware for handling certain methods and preprocessing permissions requests. | |||
*/ | |||
export default function createMethodMiddleware ({ | |||
export default function createPermissionsMethodMiddleware ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file and all references to its export were renamed to disambiguate it from the new top-level methodMiddleware
.
14f6452
to
70b50a0
Compare
import 'web3/dist/web3.min.js' | ||
|
||
const shouldLogUsage = !([ | ||
'docs.metamask.io', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting - are these sites using web3
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah right, the test dapp definitely is. For the others, it might be useful to know if they're using the injected web3
instance though. That would be something we should fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I swear to verify this manually 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Though I did have one outstanding suggestion, of relatively low importance.
Builds ready [70b50a0]
Page Load Metrics (677 ± 48 ms)
|
* add web3 usage metrics * move web3 metrics method to new middleware * rename some methods, files, and exports
* origin/master: (44 commits) Add category in eventOpts (#9164) Update changelog for v8.0.7 (#9161) Version v8.0.7 Remove web3 e2e tests (#9159) Add web3 usage metrics, prepare for web3 removal (#9144) Use `pathname` instead of URL for `currentPath` metrics parameter (#9158) Remove `url` parameter from `metricsEvent` (#9157) Change MetaMetrics category for background events (#9155) remove .network-name height Use luxon@1.24.1 (#9154) Update 'react-devtools' to ^4.8.0 (#9140) Fix connection removal bug (#9137) Add source map validator to CI (#9135) Update source map validator target files (#9133) Improve sourcemap validator console report (#9131) Add `validate-source-maps` npm script (#9134) Non-zero exit code upon failure to validate source maps (#9132) Update `brfs` from v1.6.1 to v2.0.2 (#9115) Factor out `getEnvironment` function in build script (#9114) Update `browserify` from v16.2.3 to v16.5.1 (#9113) ...
The web3 usage metrics added in #9144 assumed that all web3 properties were strings. When a `Symbol` property is accessed, our `inpage.js` script crashes because the `Symbol` cannot be serialized correctly. A check has been added for non-string property access. The metric event in these cases is set to the string "typeof ", followed by the type of the key. (e.g. `typeof symbol` for a `Symbol` property). Fixes #9234
The web3 usage metrics added in #9144 assumed that all web3 properties were strings. When a `Symbol` property is accessed, our `inpage.js` script crashes because the `Symbol` cannot be serialized correctly. A check has been added for non-string property access. The metric event in these cases is set to the string "typeof ", followed by the type of the key. (e.g. `typeof symbol` for a `Symbol` property). Fixes #9234
The web3 usage metrics added in #9144 assumed that all web3 properties were strings. When a `Symbol` property is accessed, our `inpage.js` script crashes because the `Symbol` cannot be serialized correctly. A check has been added for non-string property access. The metric event in these cases is set to the string "typeof ", followed by the type of the key. (e.g. `typeof symbol` for a `Symbol` property). Fixes #9234
The web3 usage metrics added in #9144 assumed that all web3 properties were strings. When a `Symbol` property is accessed, our `inpage.js` script crashes because the `Symbol` cannot be serialized correctly. A check has been added for non-string property access. The metric event in these cases is set to the string "typeof ", followed by the type of the key. (e.g. `typeof symbol` for a `Symbol` property). Fixes #9234
This PR adds
window.web3
usage metrics for users that opted in to metrics. It also consolidates someweb3
-related functionality, and adds a method and a middleware to perform the metrics-sending work.window.web3
usage metrics via new RPC method:metamask_logInjectedWeb3Usage
window.web3
usage metrics onwindow.web3
property access and assignment, via proxy handlersweb3
-related code intoauto-reload.js
; rename file tosetupWeb3.js
metamask_logInjectedWeb3Usage
implementationmetamask-controller
method for sending backend metricsmethodMiddleware.js
topermissionsMethodMiddleware.js
to distinguish from new top-levelmethodMiddleware