Skip to content

chore: Prep work for adding Mixpanel session recording #36895

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

Merged
hetunandu merged 3 commits into from
Oct 16, 2024
Merged

chore: Prep work for adding Mixpanel session recording #36895

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a4fb73b
chore: Enable Mixpanel Session recordings on cloud
hetunandu Oct 16, 2024
22fdb12
Merge branch 'release' into chore/add-mixpanel-sessions
hetunandu Oct 16, 2024
bede558
remove change from CE
hetunandu Oct 16, 2024
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
11 changes: 9 additions & 2 deletions app/client/jest.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
function parseConfig() {
return "";
}

const LOG_LEVELS = ["debug", "error"];
const CONFIG_LOG_LEVEL_INDEX = 1;

module.exports = {
setupFiles: ["jest-canvas-mock", "<rootDir>/test/__mocks__/reactMarkdown.tsx"],
setupFiles: [
"jest-canvas-mock",
"<rootDir>/test/__mocks__/reactMarkdown.tsx",
],
roots: ["<rootDir>/src"],
transform: {
"^.+\\.(png|js|ts|tsx)$": "ts-jest",
Expand Down Expand Up @@ -98,7 +102,10 @@ module.exports = {
fusioncharts: {
licenseKey: parseConfig("__APPSMITH_FUSIONCHARTS_LICENSE_KEY__"),
},
enableMixpanel: parseConfig("__APPSMITH_SEGMENT_KEY__"),
mixpanel: {
enabled: parseConfig("__APPSMITH_SEGMENT_KEY__"),
apiKey: parseConfig("__APPSMITH_MIXPANEL_KEY__"),
},
algolia: {
apiId: parseConfig("__APPSMITH_ALGOLIA_API_ID__"),
apiKey: parseConfig("__APPSMITH_ALGOLIA_API_KEY__"),
Expand Down
2 changes: 2 additions & 0 deletions app/client/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@
"memoize-one": "^6.0.0",
"micro-memoize": "^4.0.10",
"microdiff": "^1.4.0",
"mixpanel-browser": "^2.55.1",
"moment": "2.29.4",
"moment-timezone": "^0.5.35",
"nanoid": "^2.0.4",
Expand Down Expand Up @@ -261,6 +262,7 @@
"@types/jest": "^27.4.1",
"@types/jshint": "^2.12.0",
"@types/lodash": "^4.14.120",
"@types/mixpanel-browser": "^2.50.1",
"@types/moment-timezone": "^0.5.10",
"@types/nanoid": "^2.0.0",
"@types/node": "^10.12.18",
Expand Down
5 changes: 4 additions & 1 deletion app/client/public/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,10 @@
fusioncharts: {
licenseKey: parseConfig('{{env "APPSMITH_FUSIONCHARTS_LICENSE_KEY"}}'),
},
enableMixpanel: parseConfig('{{env "APPSMITH_SEGMENT_KEY"}}'),
mixpanel: {
enabled: parseConfig('{{env "APPSMITH_SEGMENT_KEY"}}'),
apiKey: parseConfig('{{env "APPSMITH_MIXPANEL_KEY"}}'),
},
algolia: {
apiId: parseConfig('{{env "APPSMITH_ALGOLIA_API_ID"}}'),
apiKey: parseConfig('{{env "APPSMITH_ALGOLIA_API_KEY"}}'),
Expand Down
26 changes: 18 additions & 8 deletions app/client/src/ce/configs/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@ export interface INJECTED_CONFIGS {
fusioncharts: {
licenseKey: string;
};
enableMixpanel: boolean;
mixpanel: {
enabled: boolean;
apiKey: string;
};
cloudHosting: boolean;
algolia: {
apiId: string;
Expand Down Expand Up @@ -77,9 +80,12 @@ export const getConfigsFromEnvVars = (): INJECTED_CONFIGS => {
fusioncharts: {
licenseKey: process.env.REACT_APP_FUSIONCHARTS_LICENSE_KEY || "",
},
enableMixpanel: process.env.REACT_APP_SEGMENT_KEY
? process.env.REACT_APP_SEGMENT_KEY.length > 0
: false,
mixpanel: {
enabled: process.env.REACT_APP_SEGMENT_KEY
? process.env.REACT_APP_SEGMENT_KEY.length > 0
: false,
apiKey: process.env.REACT_APP_MIXPANEL_KEY || "",
},
Comment on lines +83 to +88
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 16, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Attention, class! We have an interesting configuration setup here.

While I appreciate the effort to include Mixpanel in our configuration, I have a concern about our implementation. Can anyone spot the potential issue?

Let's break it down:

  1. We're setting enabled based on REACT_APP_SEGMENT_KEY.
  2. We're getting the apiKey from REACT_APP_MIXPANEL_KEY.

Do you see the mismatch? We're using Segment's key to enable Mixpanel. This could lead to confusion or unexpected behavior.

Here's a homework assignment for you: How could we improve this to make it more logical and less prone to errors? Think about using a dedicated environment variable for enabling Mixpanel.

Would you like to discuss a more appropriate implementation for enabling Mixpanel?

algolia: {
apiId: process.env.REACT_APP_ALGOLIA_API_ID || "",
apiKey: process.env.REACT_APP_ALGOLIA_API_KEY || "",
Expand Down Expand Up @@ -161,6 +167,10 @@ export const getAppsmithConfigs = (): AppsmithUIConfigs => {
ENV_CONFIG.segment.apiKey,
APPSMITH_FEATURE_CONFIGS?.segment.apiKey,
);
const mixpanel = getConfig(
ENV_CONFIG.mixpanel.apiKey,
APPSMITH_FEATURE_CONFIGS?.mixpanel.apiKey,
);
Comment on lines +170 to +173
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 16, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Class, let's examine how we're configuring Mixpanel in our getAppsmithConfigs function.

I see some room for improvement here. Let's analyze it together:

  1. We're correctly retrieving the Mixpanel API key (lines 170-173). Good job on that!
  2. However, in our final configuration (lines 291-294), we're setting enabled based on segment.enabled. Can anyone tell me why this might not be ideal?

Remember, Mixpanel and Segment are different analytics tools. By tying Mixpanel's enablement to Segment, we might be creating a dependency that doesn't necessarily exist.

Also, notice that we're not using the enabled property we set earlier in getConfigsFromEnvVars. We're missing an opportunity to make our configuration more flexible and accurate.

For your next assignment, I want you to think about how we can make this configuration more independent and aligned with our earlier setup. How can we ensure that Mixpanel's configuration stands on its own?

Shall we discuss a more appropriate implementation for configuring Mixpanel in this function?

Also applies to: 291-294

const newRelicAccountId = getConfig(
ENV_CONFIG.newRelic.accountId,
APPSMITH_FEATURE_CONFIGS?.newRelic.accountId,
Expand Down Expand Up @@ -278,10 +288,10 @@ export const getAppsmithConfigs = (): AppsmithUIConfigs => {
enabled: googleRecaptchaSiteKey.enabled,
apiKey: googleRecaptchaSiteKey.value,
},
enableMixpanel:
ENV_CONFIG.enableMixpanel ||
APPSMITH_FEATURE_CONFIGS?.enableMixpanel ||
false,
mixpanel: {
enabled: segment.enabled,
Copy link
Member Author

@hetunandu hetunandu Oct 16, 2024

Choose a reason for hiding this comment

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

This is intentionally using segment enabled because that is how it was working before

apiKey: mixpanel.value,
},
cloudHosting:
ENV_CONFIG.cloudHosting ||
APPSMITH_FEATURE_CONFIGS?.cloudHosting ||
Expand Down
5 changes: 4 additions & 1 deletion app/client/src/ce/configs/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,10 @@ export interface AppsmithUIConfigs {
snippetIndex: string;
};

enableMixpanel: boolean;
mixpanel: {
enabled: boolean;
apiKey: string;
};

cloudHosting: boolean;

Expand Down
3 changes: 3 additions & 0 deletions app/client/src/ce/utils/AnalyticsUtil.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export function getUserSource() {

return source;
}

declare global {
interface Window {
// Zipy is added via script tags in index.html
Expand Down Expand Up @@ -70,6 +71,7 @@ class AnalyticsUtil {
static blockTrackEvent: boolean | undefined;
static instanceId?: string = "";
static blockErrorLogs = false;

static initializeSmartLook(id: string) {
smartlookClient.init(id);
}
Expand Down Expand Up @@ -364,6 +366,7 @@ class AnalyticsUtil {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(window as any).analytics = undefined;
}

static setBlockErrorLogs(value: boolean) {
AnalyticsUtil.blockErrorLogs = value;
}
Expand Down
64 changes: 50 additions & 14 deletions app/client/yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8224,12 +8224,12 @@ __metadata:
languageName: node
linkType: hard

"@rrweb/types@npm:^2.0.0-alpha.11":
version: 2.0.0-alpha.12
resolution: "@rrweb/types@npm:2.0.0-alpha.12"
"@rrweb/types@npm:^2.0.0-alpha.11, @rrweb/types@npm:^2.0.0-alpha.13":
version: 2.0.0-alpha.17
resolution: "@rrweb/types@npm:2.0.0-alpha.17"
dependencies:
rrweb-snapshot: ^2.0.0-alpha.12
checksum: 9764d04dde006e65b0dfef32265a6365cdf1095e5a05fc129ba23f24143e3b967afc450ea401220f23bbd87cd24a6c45866d525096849fdb81831122adee1e91
rrweb-snapshot: ^2.0.0-alpha.17
checksum: 03237d9cc3b5f2a6a05000ae356e6fe6d0866fa06efe66341d9226c098fda778d46ba702efe755ee1664ce2f07c494b0e689e11693707ec2672d193314800e08
languageName: node
linkType: hard

Expand Down Expand Up @@ -10737,6 +10737,13 @@ __metadata:
languageName: node
linkType: hard

"@types/mixpanel-browser@npm:^2.50.1":
version: 2.50.1
resolution: "@types/mixpanel-browser@npm:2.50.1"
checksum: bdc42048c3c44ee36a106ed0ebe4a4027d13a52cb4ef624339ca6dc13f3e9a08c7d747f32f97cbfc3fb23d1c73185be1210d0dd6793a868e9cc067a8cff15225
languageName: node
linkType: hard

"@types/moment-timezone@npm:^0.5.10":
version: 0.5.30
resolution: "@types/moment-timezone@npm:0.5.30"
Expand Down Expand Up @@ -12770,6 +12777,7 @@ __metadata:
"@types/jest": ^27.4.1
"@types/jshint": ^2.12.0
"@types/lodash": ^4.14.120
"@types/mixpanel-browser": ^2.50.1
"@types/moment-timezone": ^0.5.10
"@types/nanoid": ^2.0.0
"@types/node": ^10.12.18
Expand Down Expand Up @@ -12903,6 +12911,7 @@ __metadata:
memoize-one: ^6.0.0
micro-memoize: ^4.0.10
microdiff: ^1.4.0
mixpanel-browser: ^2.55.1
moment: 2.29.4
moment-timezone: ^0.5.35
msw: ^0.28.0
Expand Down Expand Up @@ -24882,6 +24891,15 @@ __metadata:
languageName: node
linkType: hard

"mixpanel-browser@npm:^2.55.1":
version: 2.55.1
resolution: "mixpanel-browser@npm:2.55.1"
dependencies:
rrweb: 2.0.0-alpha.13
checksum: 10bc4d9267113c6357ef25f63e1708b1dc38f3ce6c89d97b53d85052bbeed53889b2ac4599aed0ea752814cc34342dd7d5dbe2aaca05c181f469742e8661cfb6
languageName: node
linkType: hard

"mkdirp-classic@npm:^0.5.2":
version: 0.5.3
resolution: "mkdirp-classic@npm:0.5.3"
Expand Down Expand Up @@ -30088,19 +30106,21 @@ __metadata:
languageName: node
linkType: hard

"rrdom@npm:^2.0.0-alpha.11":
version: 2.0.0-alpha.12
resolution: "rrdom@npm:2.0.0-alpha.12"
"rrdom@npm:^2.0.0-alpha.11, rrdom@npm:^2.0.0-alpha.13":
version: 2.0.0-alpha.17
resolution: "rrdom@npm:2.0.0-alpha.17"
dependencies:
rrweb-snapshot: ^2.0.0-alpha.12
checksum: 64647547f099eaf0f77d207a83498b321c3bf76e6aa12c17dd1543e1019145bd7e564dd1c16439d3f9029e472e4d1d25c1d6717e4b37af1d8c0d42c8452c96f2
rrweb-snapshot: ^2.0.0-alpha.17
checksum: a73d6e1386bb8fae02cc563ee38949e71fe031db6c55fe56fe2031f16235a107dbeb6ea30dc15e6f4fffd829908a6fe1b09e153903eda254e7c27bc11d52f278
languageName: node
linkType: hard

"rrweb-snapshot@npm:^2.0.0-alpha.11, rrweb-snapshot@npm:^2.0.0-alpha.12":
version: 2.0.0-alpha.12
resolution: "rrweb-snapshot@npm:2.0.0-alpha.12"
checksum: de8a7f8208ac9834d6cef1cea41d91782f6199ea101dde1eb9016e7253d79531444fe63466dfd42249b278ba0022c610e3806c2846df3a1cbe4328c5e88ce3a4
"rrweb-snapshot@npm:^2.0.0-alpha.11, rrweb-snapshot@npm:^2.0.0-alpha.13, rrweb-snapshot@npm:^2.0.0-alpha.17":
version: 2.0.0-alpha.17
resolution: "rrweb-snapshot@npm:2.0.0-alpha.17"
dependencies:
postcss: ^8.4.38
checksum: 076d25fa91b56f7f863bf36cb0b772efb0ad74b92e2f5318ac7beb22ab41e750c2f85120bd31769d2eca638f96f62f8bf7b4b262741cd56e74403335f77f815f
languageName: node
linkType: hard

Expand All @@ -30120,6 +30140,22 @@ __metadata:
languageName: node
linkType: hard

"rrweb@npm:2.0.0-alpha.13":
version: 2.0.0-alpha.13
resolution: "rrweb@npm:2.0.0-alpha.13"
dependencies:
"@rrweb/types": ^2.0.0-alpha.13
"@types/css-font-loading-module": 0.0.7
"@xstate/fsm": ^1.4.0
base64-arraybuffer: ^1.0.1
fflate: ^0.4.4
mitt: ^3.0.0
rrdom: ^2.0.0-alpha.13
rrweb-snapshot: ^2.0.0-alpha.13
checksum: e20e88c5fe897a54648a0f96ad3bcb285ac82a7b716944bf38ffecd6b88601d23846d4a8ad7471a4cfcb3ce37d77ed7d093e48d15936d33e8282c5e84125f797
languageName: node
linkType: hard

"run-applescript@npm:^5.0.0":
version: 5.0.0
resolution: "run-applescript@npm:5.0.0"
Expand Down