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

feat: migrate from mixpanel to segment #1005

Merged
merged 3 commits into from
Aug 4, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,14 @@
"typescript": "^4.5.4"
},
"dependencies": {
"analytics-node": "^6.1.0",
"ascii-table": "0.0.9",
"bn.js": "^5.1.1",
"bs58": "^4.0.1",
"chalk": "^4.0.0",
"flagged-respawn": "^1.0.1",
"is-ci": "^2.0.0",
"jest-environment-node": "^27.0.6",
"mixpanel": "^0.13.0",
"ncp": "^2.0.0",
"near-api-js": "^0.44.2",
"near-seed-phrase": "^0.2.0",
Expand Down
46 changes: 28 additions & 18 deletions utils/eventtracking.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
const MIXPANEL_TOKEN = 'e98aa9d6d259d9d78f20cb05cb54f5cb';
const SEGMENT_WRITE_KEY = 'vErt52x0024wPxUfLDZIKcYRPQgh6NyT';
itegulov marked this conversation as resolved.
Show resolved Hide resolved

const Analytics = require('analytics-node');
const chalk = require('chalk'); // colorize output
const crypto = require('crypto');
const mixpanel = require('mixpanel').init(MIXPANEL_TOKEN);
const near_cli_version = require('../package.json').version;
const settings = require('./settings');
const { askYesNoQuestion } = require('./readline');
const uuid = require('uuid');

const analytics = new Analytics(SEGMENT_WRITE_KEY);

const isGitPod = () => {
return !!process.env.GITPOD_WORKSPACE_URL;
};
Expand Down Expand Up @@ -39,7 +41,7 @@ const shouldTrackID = (shellSettings) => {
return !!shellSettings.trackingAccountID;
};

const getMixpanelID = (shellSettings) => isGitPod() ? getGitPodUserHash() : shellSettings.trackingSessionId;
const getSegmentID = (shellSettings) => isGitPod() ? getGitPodUserHash() : shellSettings.trackingSessionId;

const track = async (eventType, eventProperties, options) => {
try {
Expand All @@ -48,19 +50,20 @@ const track = async (eventType, eventProperties, options) => {
return;
}

if (shouldTrackID(shellSettings)) {
if (options.accountId && shouldTrackID(shellSettings)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@itegulov is this change intended?
I think users can avoid tracking even if they do not pass any accountId in command.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, previousId is a non-optional field in Segment, so it was throwing runtime errors without this check

const accountID = options.accountId;
const id = getMixpanelID(shellSettings);
await Promise.all([
mixpanel.alias(accountID, id),
mixpanel.people.set(id, { account_id: accountID })
]);
const id = getSegmentID(shellSettings);
analytics.alias({ previousId: accountID, userId: id });
analytics.identify({
userId: id,
traits: { account_id: accountID }
});
}

const user_country = await getUserCountry();

const mixPanelProperties = {
distinct_id: getMixpanelID(shellSettings),
const segmentProperties = {
distinct_id: getSegmentID(shellSettings),
near_cli_version,
user_country,
os: process.platform,
Expand All @@ -70,13 +73,20 @@ const track = async (eventType, eventProperties, options) => {
is_gitpod: isGitPod(),
timestamp: new Date()
};
Object.assign(mixPanelProperties, eventProperties);
await Promise.all([mixpanel.track(eventType, mixPanelProperties),
mixpanel.people.set(mixPanelProperties.distinct_id, {
Object.assign(segmentProperties, eventProperties);
analytics.track({
userId: segmentProperties.distinct_id,
event: eventType,
properties: segmentProperties
});
analytics.identify({
userId: segmentProperties.distinct_id,
traits: {
deployed_contracts: 0,
network_id: options.networkId,
node_url: options.nodeUrl,
})]);
node_url: options.nodeUrl
}
});
} catch (e) {
console.warn(
'Warning: problem while sending developer event tracking data. This is not critical. Error: ',
Expand Down Expand Up @@ -117,8 +127,8 @@ const askForConsentIfNeeded = async (options) => {

const trackDeployedContract = async () => {
const shellSettings = settings.getShellSettings();
const id = getMixpanelID(shellSettings);
await mixpanel.people.increment(id, 'deployed_contracts');
const id = getSegmentID(shellSettings);
// TODO: find a way to increment a Segment analytic property
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems like there is no persistent analytic properties in segment (or at least a direct replacement). Not sure how much we care about the total amount of deployed contracts as a stored number vs aggregating them by deployed events.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

@itegulov are you planning to work on it now?
Deployed contracts are the most important part of analytics IMO.

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 spent some time looking for alternatives to no avail, so I think it would make sense to hear @TiffanyGYJ's opinion first before spending even more time on this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Deployed contracts are the most important part of analytics IMO.

Curious to hear your reasoning. Wouldn't aggregating deployed events achieve the same goal? Do we need to store/cache this as a definite number for something?

Choose a reason for hiding this comment

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

Deployed events might happen multiple times with different contracts, so that might not be an accurate estimate on how many unique contracts each dev deployed.
Is there a way we can track that somewhere else or finding a way from Segment/Mixpanel is the best option?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@TiffanyGYJ

Deployed events might happen multiple times with different contracts, so that might not be an accurate estimate on how many unique contracts each dev deployed.

Looking through the code, the current logic does not track unique deployed contracts either, just the number of times when devDeploy/deploy were executed successfully.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that the total number of deploys per week is the most important metric since we are tracking developer activity.
The number of active contracts (contracts that have been deployed or redeployed in the past week) is also important.
Tracking a particular user and his deploys is not that important IMO, and a bit shady.

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 think that the total number of deploys per week is the most important metric since we are tracking developer activity. The number of active contracts (contracts that have been deployed or redeployed in the past week) is also important.

I agree with this, but a per-user counter does not really help with that, no? Increments do not have a timestamp, so you would have to track how much a counter has changed each week which kind of beats its point. I think we can deduce all that information just from tracking events (by writing a small custom event consumer or building something in Segment itself, not sure if it provides any processing logic).

Tracking a particular user and his deploys is not that important IMO, and a bit shady.

👍

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agree!

};

module.exports = {
Expand Down
2 changes: 1 addition & 1 deletion utils/exit-on-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ function getNonPrivateDataFromCmdlineOpts(options) {
};
}

// This is a workaround to get Mixpanel to log a crash
// This is a workaround to get Segment to log a crash
process.on('exit', () => {
const crashEventProperties = {
near_cli_command: process.env.NEAR_CLI_ERROR_LAST_COMMAND,
Expand Down
Loading