[Analytics] Unify client analytics #2224

0xFirekeeper · 2024-01-26T01:50:54Z

Problem solved

Short description of the bug fixed or feature added

Changes made

Public API changes: list the public API changes made if any
Internal API changes: explain the internal logic changes

How to test

Automated tests: link to unit test file
Manual tests: step by step instructions on how to test

changeset-bot · 2024-01-26T01:50:58Z

🦋 Changeset detected

Latest commit: 3c414a9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 10 packages

Name	Type
@thirdweb-dev/unity-js-bridge	Patch
@thirdweb-dev/react-native	Patch
@thirdweb-dev/storage	Patch
@thirdweb-dev/wallets	Patch
@thirdweb-dev/react	Patch
@thirdweb-dev/sdk	Patch
thirdweb	Patch
@thirdweb-dev/react-core	Patch
@thirdweb-dev/auth	Patch
@thirdweb-dev/react-native-compat	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

packages/unity-js-bridge/src/thirdweb-bridge.ts

joaquim-verges · 2024-01-29T21:13:59Z

packages/unity-js-bridge/src/thirdweb-bridge.ts

+      (globalThis as any).X_SDK_NAME = "UnitySDK";
+      (globalThis as any).X_SDK_PLATFORM = "unity";
+      (globalThis as any).X_SDK_VERSION = "4.5.1";
+      (globalThis as any).X_SDK_OS = "webgl";


here we should call detectOS() - 'win' / 'mac' / etc

need to add detect-browser as a dependency and import it here

Fixed in 110dd8f

…dev/js into firekeeper/client-analytics-v2

packages/react-native/src/core/storage/uploader.ts

packages/sdk/src/evm/common/verification.ts

packages/sdk/src/evm/core/classes/internal/contract-wrapper.ts

joaquim-verges · 2024-01-30T00:42:22Z

packages/sdk/src/evm/core/sdk.ts

+            ? "browser"
+            : "node";
+      (globalThis as any).X_SDK_VERSION = pkg.version;
+      (globalThis as any).X_SDK_OS = getOperatingSystem();


hmmm i wonder if we should just do these as fallbacks at the callsite where we pass the headers instead. Bit worried about the order of operations here

joaquim-verges · 2024-01-30T00:43:00Z

packages/storage/src/core/downloaders/storage-downloader.ts

+      headers["x-sdk-version"] = (globalThis as any).X_SDK_VERSION;
+      headers["x-sdk-name"] = (globalThis as any).X_SDK_NAME;
+      headers["x-sdk-platform"] = (globalThis as any).X_SDK_PLATFORM;
+      headers["x-sdk-os"] = (globalThis as any).X_SDK_OS;


same for storage, would just add the fallbacks inline here

joaquim-verges · 2024-01-30T00:44:09Z

packages/wallets/src/evm/connectors/smart-wallet/lib/paymaster.ts

+      headers["x-sdk-version"] = (globalThis as any).X_SDK_VERSION;
+      headers["x-sdk-name"] = (globalThis as any).X_SDK_NAME;
+      headers["x-sdk-platform"] = (globalThis as any).X_SDK_PLATFORM;
+      headers["x-sdk-os"] = (globalThis as any).X_SDK_OS;


same here, fallback inline, otherwise its too dependent on who sets up their globals first

We desperately need a utils package to share between all packages

joaquim-verges · 2024-01-30T00:44:17Z

packages/wallets/src/evm/utils/analytics.ts

+      "x-sdk-version": (globalThis as any).X_SDK_VERSION,
+      "x-sdk-name": (globalThis as any).X_SDK_NAME,
+      "x-sdk-platform": (globalThis as any).X_SDK_PLATFORM,
+      "x-sdk-os": (globalThis as any).X_SDK_OS,


codecov · 2024-01-30T22:07:15Z

Codecov Report

Attention: 101 lines in your changes are missing coverage. Please review.

Comparison is base (771bd1e) 67.56% compared to head (3c414a9) 67.09%.

Files	Patch %	Lines
packages/sdk/src/core/utils/detect-browser.ts	11.90%	74 Missing ⚠️
packages/sdk/src/core/utils/headers.ts	50.00%	13 Missing and 3 partials ⚠️
packages/sdk/src/core/utils/os.ts	37.50%	3 Missing and 2 partials ⚠️
packages/sdk/src/evm/core/sdk.ts	20.00%	4 Missing ⚠️
packages/sdk/src/evm/common/verification.ts	0.00%	1 Missing ⚠️
packages/sdk/src/evm/constants/urls.ts	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2224      +/-   ##
==========================================
- Coverage   67.56%   67.09%   -0.48%     
==========================================
  Files         291      294       +3     
  Lines       11042    11162     +120     
  Branches     1519     1542      +23     
==========================================
+ Hits         7461     7489      +28     
- Misses       2954     3043      +89     
- Partials      627      630       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

joaquim-verges · 2024-01-31T00:31:44Z

packages/wallets/src/evm/connectors/smart-wallet/lib/http-rpc-client.ts

+      headers["x-sdk-version"] = (globalThis as any).X_SDK_VERSION;
+      headers["x-sdk-name"] = (globalThis as any).X_SDK_NAME;
+      headers["x-sdk-platform"] = (globalThis as any).X_SDK_PLATFORM;
+      headers["x-sdk-os"] = (globalThis as any).X_SDK_OS;


this one also needs to fallback :/ could be using wallet SDK standalone

joaquim-verges · 2024-01-31T00:33:48Z

packages/react-native/src/evm/wallets/connectors/embedded-wallet/embedded/auth.ts

+  [X_SDK_NAME_HEADER]: (globalThis as any).X_SDK_NAME,
+  [X_SDK_OS_HEADER]: (globalThis as any).X_SDK_OS,
+  [X_SDK_PLATFORM_HEADER]: (globalThis as any).X_SDK_PLATFORM,
+  [X_SDK_VERSION_HEADER]: (globalThis as any).X_SDK_VERSION,


are you sure this is ok? not sure if the values are set (in the provider) at the time of importing this file

joaquim-verges · 2024-01-31T00:33:59Z

...ges/react-native/src/evm/wallets/connectors/embedded-wallet/embedded/helpers/api/fetchers.ts

+  [X_SDK_NAME_HEADER]: (globalThis as any).X_SDK_NAME,
+  [X_SDK_OS_HEADER]: (globalThis as any).X_SDK_OS,
+  [X_SDK_PLATFORM_HEADER]: (globalThis as any).X_SDK_PLATFORM,
+  [X_SDK_VERSION_HEADER]: (globalThis as any).X_SDK_VERSION,


packages/react/src/evm/utils/headers.ts

packages/react-native/src/core/storage/utils.ts

joaquim-verges · 2024-01-31T05:10:48Z

/release-pr

iketw · 2024-01-30T21:54:01Z

packages/unity-js-bridge/package.json

@@ -18,6 +18,7 @@
    "@thirdweb-dev/sdk": "workspace:*",
    "@thirdweb-dev/storage": "workspace:*",
    "@thirdweb-dev/wallets": "workspace:*",
+    "detect-browser": "^5.3.0",


I chose to add the code since it's been unmaintained for 2 years now. Although copy pasting it in all sdks feels wrong. We need a utils package.

iketw · 2024-01-31T19:17:15Z

packages/unity-js-bridge/src/thirdweb-bridge.ts

@@ -143,7 +143,7 @@ class ThirdwebBridge implements TWBridge {
      }
      (globalThis as any).X_SDK_NAME = "UnitySDK_WebGL";
      (globalThis as any).X_SDK_PLATFORM = "unity";
-      (globalThis as any).X_SDK_VERSION = "4.5.1";
+      (globalThis as any).X_SDK_VERSION = "4.6.0";


There's no way to automate this?

no, I make builds before every version either way to grab latest ts updates

…dev/js into firekeeper/client-analytics-v2

iketw · 2024-01-31T20:06:29Z

/release-pr

iketw · 2024-01-31T20:56:27Z

/release-pr

[Analytics] Unify client analytics

44f3e44

ikethirdweb added 2 commits January 29, 2024 14:18

Merge branch 'main' into firekeeper/client-analytics-v2

8545e1f

react and rn

c57cf15

joaquim-verges reviewed Jan 29, 2024

View reviewed changes

ikethirdweb and others added 9 commits January 29, 2024 16:26

platform

8fe1505

UnitySDK -> UnitySDK_WebGL

a35b725

detect-browser for webgl os

110dd8f

detect os

99ebaba

detect os

122d9f9

Merge branch 'firekeeper/client-analytics-v2' of github.com:thirdweb-…

a44c3d8

…dev/js into firekeeper/client-analytics-v2

report

6896428

add headers

360d032

lint

5a93c34

joaquim-verges reviewed Jan 30, 2024

View reviewed changes

ikethirdweb added 3 commits January 30, 2024 16:04

Merge branch 'main' into firekeeper/client-analytics-v2

3cce7e6

set

5b36c96

fix

31f44a6

joaquim-verges reviewed Jan 31, 2024

View reviewed changes

ikethirdweb added 2 commits January 30, 2024 22:21

revamp

2dd74de

improve

2946b03

joaquim-verges reviewed Jan 31, 2024

View reviewed changes

packages/react/src/evm/utils/headers.ts Show resolved Hide resolved

packages/react-native/src/core/storage/utils.ts Show resolved Hide resolved

ikethirdweb added 2 commits January 31, 2024 00:04

lint

a86280d

Merge branch 'main' into firekeeper/client-analytics-v2

2f2eba5

ver

9bc5d85

iketw reviewed Jan 31, 2024

View reviewed changes

ikethirdweb added 2 commits January 31, 2024 14:59

Merge branch 'main' into firekeeper/client-analytics-v2

8bf331a

changeset

b0f3bfd

Merge branch 'firekeeper/client-analytics-v2' of github.com:thirdweb-…

0b768a6

…dev/js into firekeeper/client-analytics-v2

add contract headers

3c414a9

iketw marked this pull request as ready for review January 31, 2024 23:46

iketw requested a review from a team as a code owner January 31, 2024 23:46

iketw requested a review from a team January 31, 2024 23:46

joaquim-verges approved these changes Jan 31, 2024

View reviewed changes

iketw added this pull request to the merge queue Jan 31, 2024

Merged via the queue into main with commit 3ba1536 Jan 31, 2024
10 checks passed

iketw deleted the firekeeper/client-analytics-v2 branch January 31, 2024 23:50

jnsdls mentioned this pull request Jan 31, 2024

Version Packages #2246

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Analytics] Unify client analytics #2224

[Analytics] Unify client analytics #2224

0xFirekeeper commented Jan 26, 2024

changeset-bot bot commented Jan 26, 2024 •

edited

Loading

joaquim-verges Jan 29, 2024

0xFirekeeper Jan 29, 2024

joaquim-verges Jan 30, 2024

joaquim-verges Jan 30, 2024

joaquim-verges Jan 30, 2024

iketw Jan 30, 2024

joaquim-verges Jan 30, 2024

codecov bot commented Jan 30, 2024 •

edited

Loading

joaquim-verges Jan 31, 2024

joaquim-verges Jan 31, 2024

joaquim-verges Jan 31, 2024

joaquim-verges commented Jan 31, 2024

iketw Jan 30, 2024

iketw Jan 31, 2024

0xFirekeeper Jan 31, 2024

iketw commented Jan 31, 2024

iketw commented Jan 31, 2024

[Analytics] Unify client analytics #2224

[Analytics] Unify client analytics #2224

Conversation

0xFirekeeper commented Jan 26, 2024

Problem solved

Changes made

How to test

changeset-bot bot commented Jan 26, 2024 • edited Loading

🦋 Changeset detected

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jan 30, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joaquim-verges commented Jan 31, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iketw commented Jan 31, 2024

iketw commented Jan 31, 2024

changeset-bot bot commented Jan 26, 2024 •

edited

Loading

codecov bot commented Jan 30, 2024 •

edited

Loading