From 34a361d97d23afe9e0bc41cb09d4a4eb050c2b71 Mon Sep 17 00:00:00 2001 From: Christopher Dedominici <18092467+ChristopherDedominici@users.noreply.github.com> Date: Mon, 22 Jul 2024 11:57:14 +0200 Subject: [PATCH 01/27] Modify telemetry functions --- .../cli/telemetry/telemetry-consent.ts | 28 +++++++++++++++---- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/v-next/hardhat/src/internal/cli/telemetry/telemetry-consent.ts b/v-next/hardhat/src/internal/cli/telemetry/telemetry-consent.ts index 8f9bd33ead..69a580960c 100644 --- a/v-next/hardhat/src/internal/cli/telemetry/telemetry-consent.ts +++ b/v-next/hardhat/src/internal/cli/telemetry/telemetry-consent.ts @@ -15,13 +15,32 @@ interface TelemetryConsent { } /** - * Get the user's telemetry consent. If already provided, returns the answer. - * If not, prompts the user. + * Ensure that the user's telemetry consent is set. If the consent is already provided, returns the answer. + * If not, prompts the user to provide it. * Consent is only asked in interactive environments. * @returns True if the user consents to telemetry, false otherwise. */ +export async function ensureTelemetryConsent(): Promise { + const consent = await getTelemetryConsentIfAlreadySet(); + if (consent !== undefined) { + return consent; + } + + // Telemetry consent not provided yet, ask for it + return requestTelemetryConsent(); +} + +/** + * Retrieves the user's telemetry consent status. + * @returns True if the user consents to telemetry, false otherwise. + */ export async function getTelemetryConsent(): Promise { - if (canTelemetryBeEnabled() === false) { + const consent = await getTelemetryConsentIfAlreadySet(); + return consent !== undefined ? consent : false; +} + +async function getTelemetryConsentIfAlreadySet(): Promise { + if (!canTelemetryBeEnabled()) { return false; } @@ -33,8 +52,7 @@ export async function getTelemetryConsent(): Promise { .consent; } - // Telemetry consent not provided yet, ask for it - return requestTelemetryConsent(); + return undefined; } function canTelemetryBeEnabled(): boolean { From 24b35bb75d57d2fb48adbcbd0735852f1645800d Mon Sep 17 00:00:00 2001 From: Christopher Dedominici <18092467+ChristopherDedominici@users.noreply.github.com> Date: Tue, 23 Jul 2024 17:35:06 +0200 Subject: [PATCH 02/27] add uuid npm package --- pnpm-lock.yaml | 6 ++++++ v-next/hardhat/package.json | 2 ++ 2 files changed, 8 insertions(+) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index c957e9d686..213cbadddb 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1534,6 +1534,9 @@ importers: '@ignored/hardhat-vnext-zod-utils': specifier: workspace:^3.0.0-next.2 version: link:../hardhat-zod-utils + '@types/uuid': + specifier: ^8.3.1 + version: 8.3.4 chalk: specifier: ^5.3.0 version: 5.3.0 @@ -1543,6 +1546,9 @@ importers: tsx: specifier: ^4.11.0 version: 4.11.0 + uuid: + specifier: ^8.3.2 + version: 8.3.2 zod: specifier: ^3.23.8 version: 3.23.8 diff --git a/v-next/hardhat/package.json b/v-next/hardhat/package.json index 229d2e1667..8a0f8e2c8b 100644 --- a/v-next/hardhat/package.json +++ b/v-next/hardhat/package.json @@ -77,9 +77,11 @@ "@ignored/hardhat-vnext-errors": "workspace:^3.0.0-next.2", "@ignored/hardhat-vnext-utils": "workspace:^3.0.0-next.2", "@ignored/hardhat-vnext-zod-utils": "workspace:^3.0.0-next.2", + "@types/uuid": "^8.3.1", "chalk": "^5.3.0", "enquirer": "^2.3.0", "tsx": "^4.11.0", + "uuid": "^8.3.2", "zod": "^3.23.8" } } From 44071676ecf5a09dc806ef4fe4fc7bbd1a4aea9d Mon Sep 17 00:00:00 2001 From: Christopher Dedominici <18092467+ChristopherDedominici@users.noreply.github.com> Date: Tue, 23 Jul 2024 17:36:29 +0200 Subject: [PATCH 03/27] add a method in hh-core to get the telemetry directory --- v-next/core/src/global-dir.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/v-next/core/src/global-dir.ts b/v-next/core/src/global-dir.ts index d5e189810f..695b30534c 100644 --- a/v-next/core/src/global-dir.ts +++ b/v-next/core/src/global-dir.ts @@ -11,6 +11,21 @@ export async function getConfigDir(): Promise { return config; } +/** + * Returns the path to the telemetry directory for the specified package. + * If no package name is provided, the default package name "hardhat" is used. + * Ensures that the directory exists before returning the path. + * + * @param packageName - The name of the package to get the telemetry directory for. Defaults to "hardhat". + * + * @returns A promise that resolves to the path of the telemetry directory. + */ +export async function getTelemetryDir(packageName?: string): Promise { + const { data } = await generatePaths(packageName); + await ensureDir(data); + return data; +} + async function generatePaths(packageName = "hardhat") { const { default: envPaths } = await import("env-paths"); return envPaths(packageName); From 40b00f5f2eb4ca4f7d84b47d273530e952d77ef0 Mon Sep 17 00:00:00 2001 From: Christopher Dedominici <18092467+ChristopherDedominici@users.noreply.github.com> Date: Tue, 23 Jul 2024 17:44:21 +0200 Subject: [PATCH 04/27] add main logic (still a few minors TODOs) --- .../analytics/analytics-subprocess.ts | 17 +++ .../cli/telemetry/analytics/analytics.ts | 115 ++++++++++++++++++ .../internal/cli/telemetry/analytics/types.ts | 63 ++++++++++ .../internal/cli/telemetry/analytics/utils.ts | 90 ++++++++++++++ 4 files changed, 285 insertions(+) create mode 100644 v-next/hardhat/src/internal/cli/telemetry/analytics/analytics-subprocess.ts create mode 100644 v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts create mode 100644 v-next/hardhat/src/internal/cli/telemetry/analytics/types.ts create mode 100644 v-next/hardhat/src/internal/cli/telemetry/analytics/utils.ts diff --git a/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics-subprocess.ts b/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics-subprocess.ts new file mode 100644 index 0000000000..48afbbf35b --- /dev/null +++ b/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics-subprocess.ts @@ -0,0 +1,17 @@ +import { postJsonRequest } from "@ignored/hardhat-vnext-utils/request"; + +// These keys are expected to be public +const ANALYTICS_URL = "https://www.google-analytics.com/mp/collect"; +const API_SECRET = "fQ5joCsDRTOp55wX8a2cVw"; +const MEASUREMENT_ID = "G-8LQ007N2QJ"; + +(async () => { + const payload = JSON.parse(process.argv[2]); + + await postJsonRequest(ANALYTICS_URL, payload, { + queryParams: { + api_secret: API_SECRET, + measurement_id: MEASUREMENT_ID, + }, + }); +})().catch((_err: unknown) => {}); diff --git a/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts b/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts new file mode 100644 index 0000000000..3ab895858b --- /dev/null +++ b/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts @@ -0,0 +1,115 @@ +import type { + EventNames, + Payload, + TaskParams, + TelemetryConsentPayload, +} from "./types.js"; + +import os from "node:os"; +import { dirname } from "node:path"; +import { fileURLToPath } from "node:url"; + +import { spawnDetachedSubProcess } from "@ignored/hardhat-vnext-utils/subprocess"; + +import { getHardhatVersion } from "../../../utils/package.js"; +import { getTelemetryConsent } from "../telemetry-consent.js"; + +import { getClientId, getUserType } from "./utils.js"; + +// TODO:log const log = debug("hardhat:core:global-dir"); + +const SESSION_ID = Math.random().toString(); // The same for the whole Hardhat execution + +export async function sendTelemetryConsentAnalytics( + userConsent: boolean, +): Promise { + // This is a special scenario where only the consent is sent, all the other telemetry info + // (like node version, hardhat version, etc.) are stripped. + const payload: TelemetryConsentPayload = { + client_id: "hardhat_telemetry_consent", + user_id: "hardhat_telemetry_consent", + user_properties: {}, + events: [ + { + name: "TelemetryConsentResponse", + params: { + userConsent: userConsent ? "yes" : "no", + }, + }, + ], + }; + + await createSubprocessToSendAnalytics(payload); +} + +export async function sendTaskAnalytics( + taskName: string, + scopeName: string | undefined, +): Promise { + const eventParams: TaskParams = { + task: taskName, + scope: scopeName, + }; + + return sendAnalytics("task", eventParams); +} + +// Return a boolean for test purposes, so we can check if the analytics was sent based on the consent value +async function sendAnalytics( + eventName: EventNames, + eventParams: TaskParams, +): Promise { + if ((await getTelemetryConsent()) === false) { + return false; + } + + const payload = await buildPayload(eventName, eventParams); + + await createSubprocessToSendAnalytics(payload); + + return true; +} + +async function createSubprocessToSendAnalytics( + payload: TelemetryConsentPayload | Payload, +): Promise { + // The file extension is 'js' because the 'ts' file will be compiled + const analyticsSubprocessFilePath = `${dirname(fileURLToPath(import.meta.url))}/analytics-subprocess.js`; + + await spawnDetachedSubProcess(analyticsSubprocessFilePath, [ + JSON.stringify(payload), + ]); +} + +async function buildPayload( + eventName: EventNames, + eventParams: TaskParams, +): Promise { + const clientId = await getClientId(); + + return { + client_id: clientId, + user_id: clientId, + user_properties: { + projectId: { value: "hardhat-project" }, + userType: { value: getUserType() }, + hardhatVersion: { value: await getHardhatVersion() }, + operatingSystem: { value: os.platform() }, + nodeVersion: { value: process.version }, + }, + events: [ + { + name: eventName, + params: { + // From the GA docs: amount of time someone spends with your web + // page in focus or app screen in the foreground + // The parameter has no use for our app, but it's required in order + // for user activity to display in standard reports like Realtime + engagement_time_msec: "10000", + session_id: SESSION_ID, + ...eventParams, + }, + }, + ], + }; +} diff --git a/v-next/hardhat/src/internal/cli/telemetry/analytics/types.ts b/v-next/hardhat/src/internal/cli/telemetry/analytics/types.ts new file mode 100644 index 0000000000..1e0d6c2b34 --- /dev/null +++ b/v-next/hardhat/src/internal/cli/telemetry/analytics/types.ts @@ -0,0 +1,63 @@ +export interface AnalyticsFile { + analytics: { + clientId: string; + }; +} + +/* eslint-disable @typescript-eslint/naming-convention -- these payload is formatted based on what google analytics expects*/ +export interface BasePayload { + client_id: string; + user_id: string; + user_properties: {}; + events: Array<{ + name: string; + params: { + engagement_time_msec?: string; // TODO: can be removed? + session_id?: string; + }; + }>; +} + +export interface TelemetryConsentPayload extends BasePayload { + events: Array<{ + name: "TelemetryConsentResponse"; + params: { + session_id?: string; + userConsent: "yes" | "no"; + }; + }>; +} + +export type EventNames = "task"; + +export interface TaskParams { + task: string; + scope?: string; +} + +export interface Payload extends BasePayload { + user_properties: { + projectId: { + value: string; + }; + userType: { + value: string; + }; + hardhatVersion: { + value: string; + }; + operatingSystem: { + value: string; + }; + nodeVersion: { + value: string; + }; + }; + events: Array<{ + name: EventNames; + params: { + engagement_time_msec: string; + session_id: string; + } & TaskParams; + }>; +} diff --git a/v-next/hardhat/src/internal/cli/telemetry/analytics/utils.ts b/v-next/hardhat/src/internal/cli/telemetry/analytics/utils.ts new file mode 100644 index 0000000000..243a9e328b --- /dev/null +++ b/v-next/hardhat/src/internal/cli/telemetry/analytics/utils.ts @@ -0,0 +1,90 @@ +import type { AnalyticsFile } from "./types.js"; + +import os from "node:os"; +import path from "node:path"; + +import { getTelemetryDir } from "@ignored/hardhat-vnext-core/global-dir"; +import { isCi } from "@ignored/hardhat-vnext-utils/ci"; +import { + exists, + readJsonFile, + writeJsonFile, +} from "@ignored/hardhat-vnext-utils/fs"; + +const ANALYTICS_FILE_NAME = "analytics.json"; + +export async function getClientId(): Promise { + let clientId = await readAnalyticsId(); + + if (clientId === undefined) { + // If the clientId is not undefined and it is of type "firstLegacy" or "secondLegacy," store it in the new file format + clientId = + (await readSecondLegacyAnalyticsId()) ?? + (await readFirstLegacyAnalyticsId()); + + if (clientId === undefined) { + const { v4: uuid } = await import("uuid"); + // TODO:log log("Client Id not found, generating a new one"); + clientId = uuid(); + } + + await writeAnalyticsId(clientId); + } + + return clientId; +} + +export function getUserType(): string { + return isCi() ? "CI" : "Developer"; +} + +async function readAnalyticsId(): Promise { + const globalTelemetryDir = await getTelemetryDir(); + const filePath = path.join(globalTelemetryDir, ANALYTICS_FILE_NAME); + return readId(filePath); +} + +/** + * This is the first way that the analytics id was saved. + */ +function readFirstLegacyAnalyticsId(): Promise { + const oldIdFile = path.join(os.homedir(), ".buidler", "config.json"); + return readId(oldIdFile); +} + +/** + * This is the same way the analytics id is saved now, but using buidler as the + * name of the project for env-paths + */ +async function readSecondLegacyAnalyticsId(): Promise { + const globalTelemetryDir = await getTelemetryDir("buidler"); + const filePath = path.join(globalTelemetryDir, ANALYTICS_FILE_NAME); + return readId(filePath); +} + +async function writeAnalyticsId(clientId: string): Promise { + const globalTelemetryDir = await getTelemetryDir(); + const filePath = path.join(globalTelemetryDir, ANALYTICS_FILE_NAME); + await writeJsonFile(filePath, { + analytics: { + clientId, + }, + }); + + // TODO:log log(`Stored clientId ${clientId}`); +} + +async function readId(filePath: string): Promise { + // TODO:log log(`Looking up Client Id at ${filePath}`); + + if ((await exists(filePath)) === false) { + return undefined; + } + + const data: AnalyticsFile = await readJsonFile(filePath); + + const clientId = data.analytics.clientId; + + // TODO:log log(`Client Id found: ${clientId}`); + return clientId; +} From a664c3b301d24018d2a1dabaaa707231f38394d5 Mon Sep 17 00:00:00 2001 From: Christopher Dedominici <18092467+ChristopherDedominici@users.noreply.github.com> Date: Tue, 23 Jul 2024 17:50:25 +0200 Subject: [PATCH 05/27] add tests --- .../analytics/analytics-subprocess.js | 22 +++ .../analytics/telemetry-consent-payload.json | 13 ++ .../cli/telemetry/analytics/analytics.ts | 172 ++++++++++++++++++ .../internal/cli/telemetry/analytics/utils.ts | 123 +++++++++++++ 4 files changed, 330 insertions(+) create mode 100644 v-next/hardhat/test/fixture-projects/cli/telemetry/analytics/analytics-subprocess.js create mode 100644 v-next/hardhat/test/fixture-projects/cli/telemetry/analytics/telemetry-consent-payload.json create mode 100644 v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts create mode 100644 v-next/hardhat/test/internal/cli/telemetry/analytics/utils.ts diff --git a/v-next/hardhat/test/fixture-projects/cli/telemetry/analytics/analytics-subprocess.js b/v-next/hardhat/test/fixture-projects/cli/telemetry/analytics/analytics-subprocess.js new file mode 100644 index 0000000000..ca0560a9ae --- /dev/null +++ b/v-next/hardhat/test/fixture-projects/cli/telemetry/analytics/analytics-subprocess.js @@ -0,0 +1,22 @@ +// This file will be copied to the "analytics" folder so it will be executed as a subprocess. +// This will allow checking that the subprocess received the correct payload. + +// We need to directly use the node method "fs" instead of "writeJsonFile" because a TypeScript ESM file cannot be imported without compiling it first +import { writeFileSync } from "node:fs"; +import path from "node:path"; + +const PATH_TO_RESULT_FILE = path.join( + process.cwd(), + "test", + "fixture-projects", + "cli", + "telemetry", + "analytics", + "result.json", +); + +(() => { + const stringifiedPayload = process.argv[2]; + + writeFileSync(PATH_TO_RESULT_FILE, JSON.stringify(stringifiedPayload)); +})(); diff --git a/v-next/hardhat/test/fixture-projects/cli/telemetry/analytics/telemetry-consent-payload.json b/v-next/hardhat/test/fixture-projects/cli/telemetry/analytics/telemetry-consent-payload.json new file mode 100644 index 0000000000..3f7afa0a72 --- /dev/null +++ b/v-next/hardhat/test/fixture-projects/cli/telemetry/analytics/telemetry-consent-payload.json @@ -0,0 +1,13 @@ +{ + "client_id": "hardhat_telemetry_consent", + "user_id": "hardhat_telemetry_consent", + "user_properties": {}, + "events": [ + { + "name": "TelemetryConsentResponse", + "params": { + "userConsent": "yes" + } + } + ] +} diff --git a/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts b/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts new file mode 100644 index 0000000000..25378a1d75 --- /dev/null +++ b/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts @@ -0,0 +1,172 @@ +import type { Payload } from "../../../../../src/internal/cli/telemetry/analytics/types.js"; + +import assert from "node:assert/strict"; +import path from "node:path"; +import { after, afterEach, before, beforeEach, describe, it } from "node:test"; + +import { getConfigDir } from "@ignored/hardhat-vnext-core/global-dir"; +import { + copy, + exists, + readJsonFile, + remove, + writeJsonFile, +} from "@ignored/hardhat-vnext-utils/fs"; + +import { + sendTaskAnalytics, + sendTelemetryConsentAnalytics, +} from "../../../../../src/internal/cli/telemetry/analytics/analytics.js"; +import { getHardhatVersion } from "../../../../../src/internal/utils/package.js"; + +// The analytics logic uses a detached subprocess to send the payload via HTTP call. +// We cannot test the HTTP call directly, but we can use a test subprocess to verify if the payload is correctly created. +// This is possible because the analytics code attempts to execute a subprocess file of type 'JS'. JS files are only available after compilation. +// During the tests, no JS file is available, so the expected subprocess does not exist. Therefore, we can copy a test subprocess file +// to the expected location instead of the original one and check if it receives the correct payload. + +const PATH_TO_FIXTURE = path.join( + process.cwd(), + "test", + "fixture-projects", + "cli", + "telemetry", + "analytics", +); + +const SOURCE_PATH_TEST_SUBPROCESS_FILE = path.join( + PATH_TO_FIXTURE, + "analytics-subprocess.js", +); + +const DEST_PATH_TEST_SUBPROCESS_FILE = path.join( + process.cwd(), + "src", + "internal", + "cli", + "telemetry", + "analytics", + "analytics-subprocess.js", +); + +const RESULT_FILE_PATH = path.join(PATH_TO_FIXTURE, "result.json"); + +async function copyTestSubprocessFile() { + await copy(SOURCE_PATH_TEST_SUBPROCESS_FILE, DEST_PATH_TEST_SUBPROCESS_FILE); +} + +async function removeTestSubprocessFile() { + remove(DEST_PATH_TEST_SUBPROCESS_FILE); +} + +async function setTelemetryConsentFile(consent: boolean) { + const configDir = await getConfigDir(); + const filePath = path.join(configDir, "telemetry-consent.json"); + await writeJsonFile(filePath, { consent }); +} + +async function checkIfSubprocessWasExecuted() { + // Checks if the subprocess was executed by waiting for a file to be created. + // Uses an interval to periodically check for the file. If the file isn't found + // within a specified number of attempts, an error is thrown, indicating a failure in subprocess execution. + const MAX_COUNTER = 20; + + return new Promise((resolve, reject) => { + let counter = 0; + + const intervalId = setInterval(async () => { + counter++; + + if (await exists(RESULT_FILE_PATH)) { + clearInterval(intervalId); + resolve(true); + } else if (counter > MAX_COUNTER) { + clearInterval(intervalId); + reject("Subprocess was not executed in the expected time"); + } + }, 100); + }); +} + +describe("analytics", () => { + before(async () => { + copyTestSubprocessFile(); + }); + + after(async () => { + await removeTestSubprocessFile(); + }); + + beforeEach(async () => { + await remove(RESULT_FILE_PATH); + }); + + afterEach(async () => { + await remove(RESULT_FILE_PATH); + }); + + it("should create the correct payload for the telemetry consent", async () => { + await sendTelemetryConsentAnalytics(true); + + await checkIfSubprocessWasExecuted(); + + const result = JSON.parse(await readJsonFile(RESULT_FILE_PATH)); + + const expected = await readJsonFile( + path.join(PATH_TO_FIXTURE, "telemetry-consent-payload.json"), + ); + + assert.deepEqual(result, expected); + }); + + describe("analytics payload", async () => { + const ORIGINAL_PROCESS_ENV = { ...process }; + + before(() => { + // Force Ci to not be detected as Ci so the test can run (Ci is blocked for analytics) + delete process.env.GITHUB_ACTIONS; + delete process.env.NOW; + delete process.env.DEPLOYMENT_ID; + delete process.env.CODEBUILD_BUILD_NUMBER; + delete process.env.CI; + delete process.env.CONTINUOUS_INTEGRATION; + delete process.env.BUILD_NUMBER; + delete process.env.RUN_ID; + + process.stdout.isTTY = true; + }); + + after(() => { + process = ORIGINAL_PROCESS_ENV; + }); + + it("should create the correct payload for the task analytics", async () => { + await setTelemetryConsentFile(true); + + const wasSent = await sendTaskAnalytics("hardhat", "compile"); + + await checkIfSubprocessWasExecuted(); + + const result: Payload = JSON.parse(await readJsonFile(RESULT_FILE_PATH)); + + assert.equal(wasSent, true); + + // Check payload properties + assert.notEqual(result.client_id, undefined); + assert.notEqual(result.user_id, undefined); + assert.equal(result.user_properties.projectId.value, "hardhat-project"); + assert.equal(result.user_properties.userType.value, "Developer"); + assert.equal( + result.user_properties.hardhatVersion.value, + await getHardhatVersion(), + ); + assert.notEqual(result.user_properties.operatingSystem.value, undefined); + assert.notEqual(result.user_properties.nodeVersion.value, undefined); + assert.equal(result.events[0].name, "task"); + assert.equal(result.events[0].params.engagement_time_msec, "10000"); + assert.notEqual(result.events[0].params.session_id, undefined); + assert.equal(result.events[0].params.task, "hardhat"); + assert.equal(result.events[0].params.scope, "compile"); + }); + }); +}); diff --git a/v-next/hardhat/test/internal/cli/telemetry/analytics/utils.ts b/v-next/hardhat/test/internal/cli/telemetry/analytics/utils.ts new file mode 100644 index 0000000000..039fd2e3a3 --- /dev/null +++ b/v-next/hardhat/test/internal/cli/telemetry/analytics/utils.ts @@ -0,0 +1,123 @@ +import type { AnalyticsFile } from "../../../../../src/internal/cli/telemetry/analytics/types.js"; + +import assert from "node:assert/strict"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, beforeEach, describe, it } from "node:test"; + +import { getTelemetryDir } from "@ignored/hardhat-vnext-core/global-dir"; +import { isCi } from "@ignored/hardhat-vnext-utils/ci"; +import { + readJsonFile, + remove, + writeJsonFile, +} from "@ignored/hardhat-vnext-utils/fs"; + +import { + getClientId, + getUserType, +} from "../../../../../src/internal/cli/telemetry/analytics/utils.js"; + +const ANALYTICS_FILE_NAME = "analytics.json"; + +type FileType = "current" | "firstLegacy" | "secondLegacy"; + +const CLIENT_ID_CURRENT = "from-current-file"; +const CLIENT_ID_FIRST_LEGACY = "from-first-legacy-file"; +const CLIENT_ID_SECOND_LEGACY = "from-second-legacy-file"; + +async function createClientIdFile(fileType: FileType) { + const [filePath, clientId] = await getFileInfo(fileType); + await writeJsonFile(filePath, { + analytics: { + clientId, + }, + }); +} + +async function getClientIdFromFile(fileType: FileType) { + const data: AnalyticsFile = await readJsonFile( + (await getFileInfo(fileType))[0], + ); + + return data.analytics.clientId; +} + +async function deleteClientIdFile(fileType: FileType) { + const [filePath] = await getFileInfo(fileType); + await remove(filePath); +} + +async function getFileInfo(fileType: FileType) { + let filePath: string; + let clientId: string; + + if (fileType === "current") { + filePath = path.join(await getTelemetryDir(), ANALYTICS_FILE_NAME); + clientId = CLIENT_ID_CURRENT; + } else if (fileType === "firstLegacy") { + filePath = path.join(os.homedir(), ".buidler", "config.json"); + clientId = CLIENT_ID_FIRST_LEGACY; + } else { + filePath = path.join(await getTelemetryDir("buidler"), ANALYTICS_FILE_NAME); + clientId = CLIENT_ID_SECOND_LEGACY; + } + + return [filePath, clientId]; +} + +describe("telemetry/analytics/utils", () => { + describe("clientId", () => { + beforeEach(async () => { + await deleteClientIdFile("current"); + await deleteClientIdFile("firstLegacy"); + await deleteClientIdFile("secondLegacy"); + }); + + afterEach(async () => { + await deleteClientIdFile("current"); + await deleteClientIdFile("firstLegacy"); + await deleteClientIdFile("secondLegacy"); + }); + + it("should set a new clientId because the value is not yet defined", async () => { + const clientId = await getClientId(); + + // The clientId should be generate as uuid + assert.notEqual(clientId, undefined); + assert.notEqual(clientId, CLIENT_ID_CURRENT); + assert.notEqual(clientId, CLIENT_ID_FIRST_LEGACY); + assert.notEqual(clientId, CLIENT_ID_SECOND_LEGACY); + + // The clientId should also be saved in the file + assert.equal(clientId, await getClientIdFromFile("current")); + }); + + it("should get the 'current' clientId because it already exists", async () => { + await createClientIdFile("current"); + const clientId = await getClientId(); + assert.equal(clientId, CLIENT_ID_CURRENT); + }); + + it("should get the 'firstLegacy' clientId because it already exists and store it the new analytics file (current)", async () => { + await createClientIdFile("firstLegacy"); + + const clientId = await getClientId(); + assert.equal(clientId, CLIENT_ID_FIRST_LEGACY); + assert.equal(clientId, await getClientIdFromFile("current")); + }); + + it("should get the 'secondLegacy' clientId because it already exists and store it the new analytics file (current)", async () => { + await createClientIdFile("secondLegacy"); + + const clientId = await getClientId(); + assert.equal(clientId, CLIENT_ID_SECOND_LEGACY); + assert.equal(clientId, await getClientIdFromFile("current")); + }); + }); + + it("should return the correct user type", () => { + const userType = isCi() ? "CI" : "Developer"; + assert.equal(getUserType(), userType); + }); +}); From 648592ce0508a028db06b99470ea43c157e70808 Mon Sep 17 00:00:00 2001 From: Christopher Dedominici <18092467+ChristopherDedominici@users.noreply.github.com> Date: Wed, 24 Jul 2024 11:57:03 +0200 Subject: [PATCH 06/27] small code reorganization --- .../src/internal/cli/telemetry/analytics/analytics.ts | 11 ++++------- .../src/internal/cli/telemetry/analytics/types.ts | 8 ++++++-- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts b/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts index 3ab895858b..6d91f487fe 100644 --- a/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts +++ b/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts @@ -18,12 +18,13 @@ import { getClientId, getUserType } from "./utils.js"; // TODO:log const log = debug("hardhat:core:global-dir"); -const SESSION_ID = Math.random().toString(); // The same for the whole Hardhat execution +const SESSION_ID = Math.random().toString(); +const ENGAGEMENT_TIME_MSEC = "10000"; export async function sendTelemetryConsentAnalytics( userConsent: boolean, ): Promise { - // This is a special scenario where only the consent is sent, all the other telemetry info + // This is a special scenario where only the consent is sent, all the other analytics info // (like node version, hardhat version, etc.) are stripped. const payload: TelemetryConsentPayload = { client_id: "hardhat_telemetry_consent", @@ -101,11 +102,7 @@ async function buildPayload( { name: eventName, params: { - // From the GA docs: amount of time someone spends with your web - // page in focus or app screen in the foreground - // The parameter has no use for our app, but it's required in order - // for user activity to display in standard reports like Realtime - engagement_time_msec: "10000", + engagement_time_msec: ENGAGEMENT_TIME_MSEC, session_id: SESSION_ID, ...eventParams, }, diff --git a/v-next/hardhat/src/internal/cli/telemetry/analytics/types.ts b/v-next/hardhat/src/internal/cli/telemetry/analytics/types.ts index 1e0d6c2b34..168d1d8e92 100644 --- a/v-next/hardhat/src/internal/cli/telemetry/analytics/types.ts +++ b/v-next/hardhat/src/internal/cli/telemetry/analytics/types.ts @@ -12,7 +12,11 @@ export interface BasePayload { events: Array<{ name: string; params: { - engagement_time_msec?: string; // TODO: can be removed? + // From the GA docs: amount of time someone spends with your web + // page in focus or app screen in the foreground. + // The parameter has no use for our app, but it's required in order + // for user activity to display in standard reports like Realtime. + engagement_time_msec?: string; session_id?: string; }; }>; @@ -22,8 +26,8 @@ export interface TelemetryConsentPayload extends BasePayload { events: Array<{ name: "TelemetryConsentResponse"; params: { - session_id?: string; userConsent: "yes" | "no"; + session_id?: string; }; }>; } From 94a3abe4969f0c0eaf8173a5ccac5858dbcd41e5 Mon Sep 17 00:00:00 2001 From: Christopher Dedominici <18092467+ChristopherDedominici@users.noreply.github.com> Date: Wed, 24 Jul 2024 18:39:55 +0200 Subject: [PATCH 07/27] Small PR comments' fixes --- .../src/internal/cli/telemetry/analytics/analytics.ts | 6 ++---- .../cli/telemetry/analytics/analytics-subprocess.js | 2 +- .../test/internal/cli/telemetry/analytics/analytics.ts | 4 ++-- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts b/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts index 6d91f487fe..86cdc9f1ce 100644 --- a/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts +++ b/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts @@ -6,8 +6,6 @@ import type { } from "./types.js"; import os from "node:os"; -import { dirname } from "node:path"; -import { fileURLToPath } from "node:url"; import { spawnDetachedSubProcess } from "@ignored/hardhat-vnext-utils/subprocess"; @@ -60,7 +58,7 @@ async function sendAnalytics( eventName: EventNames, eventParams: TaskParams, ): Promise { - if ((await getTelemetryConsent()) === false) { + if (!(await getTelemetryConsent())) { return false; } @@ -75,7 +73,7 @@ async function createSubprocessToSendAnalytics( payload: TelemetryConsentPayload | Payload, ): Promise { // The file extension is 'js' because the 'ts' file will be compiled - const analyticsSubprocessFilePath = `${dirname(fileURLToPath(import.meta.url))}/analytics-subprocess.js`; + const analyticsSubprocessFilePath = `${import.meta.dirname}/analytics-subprocess.js`; await spawnDetachedSubProcess(analyticsSubprocessFilePath, [ JSON.stringify(payload), diff --git a/v-next/hardhat/test/fixture-projects/cli/telemetry/analytics/analytics-subprocess.js b/v-next/hardhat/test/fixture-projects/cli/telemetry/analytics/analytics-subprocess.js index ca0560a9ae..5b9fc7931d 100644 --- a/v-next/hardhat/test/fixture-projects/cli/telemetry/analytics/analytics-subprocess.js +++ b/v-next/hardhat/test/fixture-projects/cli/telemetry/analytics/analytics-subprocess.js @@ -18,5 +18,5 @@ const PATH_TO_RESULT_FILE = path.join( (() => { const stringifiedPayload = process.argv[2]; - writeFileSync(PATH_TO_RESULT_FILE, JSON.stringify(stringifiedPayload)); + writeFileSync(PATH_TO_RESULT_FILE, stringifiedPayload); })(); diff --git a/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts b/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts index 25378a1d75..b91bb704c7 100644 --- a/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts +++ b/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts @@ -110,7 +110,7 @@ describe("analytics", () => { await checkIfSubprocessWasExecuted(); - const result = JSON.parse(await readJsonFile(RESULT_FILE_PATH)); + const result = await readJsonFile(RESULT_FILE_PATH); const expected = await readJsonFile( path.join(PATH_TO_FIXTURE, "telemetry-consent-payload.json"), @@ -147,7 +147,7 @@ describe("analytics", () => { await checkIfSubprocessWasExecuted(); - const result: Payload = JSON.parse(await readJsonFile(RESULT_FILE_PATH)); + const result: Payload = await readJsonFile(RESULT_FILE_PATH); assert.equal(wasSent, true); From c01024a1eb3e43e07cfd6fcfc4cb8fc8d9b19dc2 Mon Sep 17 00:00:00 2001 From: Christopher Dedominici <18092467+ChristopherDedominici@users.noreply.github.com> Date: Mon, 29 Jul 2024 11:31:01 +0200 Subject: [PATCH 08/27] remove userType. CI is no longer sent --- .../src/internal/cli/telemetry/analytics/analytics.ts | 3 +-- .../src/internal/cli/telemetry/analytics/types.ts | 3 --- .../src/internal/cli/telemetry/analytics/utils.ts | 5 ----- .../internal/cli/telemetry/analytics/analytics.ts | 1 - .../test/internal/cli/telemetry/analytics/utils.ts | 11 +---------- 5 files changed, 2 insertions(+), 21 deletions(-) diff --git a/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts b/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts index 86cdc9f1ce..e1d8bcaa3a 100644 --- a/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts +++ b/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts @@ -12,7 +12,7 @@ import { spawnDetachedSubProcess } from "@ignored/hardhat-vnext-utils/subprocess import { getHardhatVersion } from "../../../utils/package.js"; import { getTelemetryConsent } from "../telemetry-consent.js"; -import { getClientId, getUserType } from "./utils.js"; +import { getClientId } from "./utils.js"; // TODO:log const log = debug("hardhat:core:global-dir"); @@ -91,7 +91,6 @@ async function buildPayload( user_id: clientId, user_properties: { projectId: { value: "hardhat-project" }, - userType: { value: getUserType() }, hardhatVersion: { value: await getHardhatVersion() }, operatingSystem: { value: os.platform() }, nodeVersion: { value: process.version }, diff --git a/v-next/hardhat/src/internal/cli/telemetry/analytics/types.ts b/v-next/hardhat/src/internal/cli/telemetry/analytics/types.ts index 168d1d8e92..c590345f7a 100644 --- a/v-next/hardhat/src/internal/cli/telemetry/analytics/types.ts +++ b/v-next/hardhat/src/internal/cli/telemetry/analytics/types.ts @@ -44,9 +44,6 @@ export interface Payload extends BasePayload { projectId: { value: string; }; - userType: { - value: string; - }; hardhatVersion: { value: string; }; diff --git a/v-next/hardhat/src/internal/cli/telemetry/analytics/utils.ts b/v-next/hardhat/src/internal/cli/telemetry/analytics/utils.ts index 243a9e328b..a5ca77f155 100644 --- a/v-next/hardhat/src/internal/cli/telemetry/analytics/utils.ts +++ b/v-next/hardhat/src/internal/cli/telemetry/analytics/utils.ts @@ -4,7 +4,6 @@ import os from "node:os"; import path from "node:path"; import { getTelemetryDir } from "@ignored/hardhat-vnext-core/global-dir"; -import { isCi } from "@ignored/hardhat-vnext-utils/ci"; import { exists, readJsonFile, @@ -34,10 +33,6 @@ export async function getClientId(): Promise { return clientId; } -export function getUserType(): string { - return isCi() ? "CI" : "Developer"; -} - async function readAnalyticsId(): Promise { const globalTelemetryDir = await getTelemetryDir(); const filePath = path.join(globalTelemetryDir, ANALYTICS_FILE_NAME); diff --git a/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts b/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts index b91bb704c7..200b58d80d 100644 --- a/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts +++ b/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts @@ -155,7 +155,6 @@ describe("analytics", () => { assert.notEqual(result.client_id, undefined); assert.notEqual(result.user_id, undefined); assert.equal(result.user_properties.projectId.value, "hardhat-project"); - assert.equal(result.user_properties.userType.value, "Developer"); assert.equal( result.user_properties.hardhatVersion.value, await getHardhatVersion(), diff --git a/v-next/hardhat/test/internal/cli/telemetry/analytics/utils.ts b/v-next/hardhat/test/internal/cli/telemetry/analytics/utils.ts index 039fd2e3a3..08271ef627 100644 --- a/v-next/hardhat/test/internal/cli/telemetry/analytics/utils.ts +++ b/v-next/hardhat/test/internal/cli/telemetry/analytics/utils.ts @@ -6,17 +6,13 @@ import path from "node:path"; import { afterEach, beforeEach, describe, it } from "node:test"; import { getTelemetryDir } from "@ignored/hardhat-vnext-core/global-dir"; -import { isCi } from "@ignored/hardhat-vnext-utils/ci"; import { readJsonFile, remove, writeJsonFile, } from "@ignored/hardhat-vnext-utils/fs"; -import { - getClientId, - getUserType, -} from "../../../../../src/internal/cli/telemetry/analytics/utils.js"; +import { getClientId } from "../../../../../src/internal/cli/telemetry/analytics/utils.js"; const ANALYTICS_FILE_NAME = "analytics.json"; @@ -115,9 +111,4 @@ describe("telemetry/analytics/utils", () => { assert.equal(clientId, await getClientIdFromFile("current")); }); }); - - it("should return the correct user type", () => { - const userType = isCi() ? "CI" : "Developer"; - assert.equal(getUserType(), userType); - }); }); From 170f773f9942a792d466627d689965fad715c2b1 Mon Sep 17 00:00:00 2001 From: Christopher Dedominici <18092467+ChristopherDedominici@users.noreply.github.com> Date: Mon, 29 Jul 2024 11:35:59 +0200 Subject: [PATCH 09/27] remove old analytics client ids --- .../internal/cli/telemetry/analytics/utils.ts | 56 +++-------- .../internal/cli/telemetry/analytics/utils.ts | 95 +++++-------------- 2 files changed, 38 insertions(+), 113 deletions(-) diff --git a/v-next/hardhat/src/internal/cli/telemetry/analytics/utils.ts b/v-next/hardhat/src/internal/cli/telemetry/analytics/utils.ts index a5ca77f155..a8e71548b3 100644 --- a/v-next/hardhat/src/internal/cli/telemetry/analytics/utils.ts +++ b/v-next/hardhat/src/internal/cli/telemetry/analytics/utils.ts @@ -1,6 +1,5 @@ import type { AnalyticsFile } from "./types.js"; -import os from "node:os"; import path from "node:path"; import { getTelemetryDir } from "@ignored/hardhat-vnext-core/global-dir"; @@ -16,16 +15,9 @@ export async function getClientId(): Promise { let clientId = await readAnalyticsId(); if (clientId === undefined) { - // If the clientId is not undefined and it is of type "firstLegacy" or "secondLegacy," store it in the new file format - clientId = - (await readSecondLegacyAnalyticsId()) ?? - (await readFirstLegacyAnalyticsId()); - - if (clientId === undefined) { - const { v4: uuid } = await import("uuid"); - // TODO:log log("Client Id not found, generating a new one"); - clientId = uuid(); - } + const { v4: uuid } = await import("uuid"); + // TODO:log log("Client Id not found, generating a new one"); + clientId = uuid(); await writeAnalyticsId(clientId); } @@ -36,25 +28,18 @@ export async function getClientId(): Promise { async function readAnalyticsId(): Promise { const globalTelemetryDir = await getTelemetryDir(); const filePath = path.join(globalTelemetryDir, ANALYTICS_FILE_NAME); - return readId(filePath); -} -/** - * This is the first way that the analytics id was saved. - */ -function readFirstLegacyAnalyticsId(): Promise { - const oldIdFile = path.join(os.homedir(), ".buidler", "config.json"); - return readId(oldIdFile); -} + // TODO:log log(`Looking up Client Id at ${filePath}`); -/** - * This is the same way the analytics id is saved now, but using buidler as the - * name of the project for env-paths - */ -async function readSecondLegacyAnalyticsId(): Promise { - const globalTelemetryDir = await getTelemetryDir("buidler"); - const filePath = path.join(globalTelemetryDir, ANALYTICS_FILE_NAME); - return readId(filePath); + if ((await exists(filePath)) === false) { + return undefined; + } + + const data: AnalyticsFile = await readJsonFile(filePath); + const clientId = data.analytics.clientId; + // TODO:log log(`Client Id found: ${clientId}`); + + return clientId; } async function writeAnalyticsId(clientId: string): Promise { @@ -68,18 +53,3 @@ async function writeAnalyticsId(clientId: string): Promise { // TODO:log log(`Stored clientId ${clientId}`); } - -async function readId(filePath: string): Promise { - // TODO:log log(`Looking up Client Id at ${filePath}`); - - if ((await exists(filePath)) === false) { - return undefined; - } - - const data: AnalyticsFile = await readJsonFile(filePath); - - const clientId = data.analytics.clientId; - - // TODO:log log(`Client Id found: ${clientId}`); - return clientId; -} diff --git a/v-next/hardhat/test/internal/cli/telemetry/analytics/utils.ts b/v-next/hardhat/test/internal/cli/telemetry/analytics/utils.ts index 08271ef627..357f48fe1f 100644 --- a/v-next/hardhat/test/internal/cli/telemetry/analytics/utils.ts +++ b/v-next/hardhat/test/internal/cli/telemetry/analytics/utils.ts @@ -1,7 +1,6 @@ import type { AnalyticsFile } from "../../../../../src/internal/cli/telemetry/analytics/types.js"; import assert from "node:assert/strict"; -import os from "node:os"; import path from "node:path"; import { afterEach, beforeEach, describe, it } from "node:test"; @@ -14,101 +13,57 @@ import { import { getClientId } from "../../../../../src/internal/cli/telemetry/analytics/utils.js"; -const ANALYTICS_FILE_NAME = "analytics.json"; +const CLIENT_ID = "test-client-id"; -type FileType = "current" | "firstLegacy" | "secondLegacy"; - -const CLIENT_ID_CURRENT = "from-current-file"; -const CLIENT_ID_FIRST_LEGACY = "from-first-legacy-file"; -const CLIENT_ID_SECOND_LEGACY = "from-second-legacy-file"; - -async function createClientIdFile(fileType: FileType) { - const [filePath, clientId] = await getFileInfo(fileType); +async function createClientIdFile() { + const filePath = await getFilePath(); await writeJsonFile(filePath, { analytics: { - clientId, + clientId: CLIENT_ID, }, }); } -async function getClientIdFromFile(fileType: FileType) { - const data: AnalyticsFile = await readJsonFile( - (await getFileInfo(fileType))[0], - ); - +async function getClientIdFromFile() { + const filePath = await getFilePath(); + const data: AnalyticsFile = await readJsonFile(filePath); return data.analytics.clientId; } -async function deleteClientIdFile(fileType: FileType) { - const [filePath] = await getFileInfo(fileType); +async function deleteClientIdFile() { + const filePath = await getFilePath(); await remove(filePath); } -async function getFileInfo(fileType: FileType) { - let filePath: string; - let clientId: string; - - if (fileType === "current") { - filePath = path.join(await getTelemetryDir(), ANALYTICS_FILE_NAME); - clientId = CLIENT_ID_CURRENT; - } else if (fileType === "firstLegacy") { - filePath = path.join(os.homedir(), ".buidler", "config.json"); - clientId = CLIENT_ID_FIRST_LEGACY; - } else { - filePath = path.join(await getTelemetryDir("buidler"), ANALYTICS_FILE_NAME); - clientId = CLIENT_ID_SECOND_LEGACY; - } - - return [filePath, clientId]; +async function getFilePath() { + return path.join(await getTelemetryDir(), "analytics.json"); } describe("telemetry/analytics/utils", () => { - describe("clientId", () => { + describe("analyticsClientId", () => { beforeEach(async () => { - await deleteClientIdFile("current"); - await deleteClientIdFile("firstLegacy"); - await deleteClientIdFile("secondLegacy"); + await deleteClientIdFile(); }); afterEach(async () => { - await deleteClientIdFile("current"); - await deleteClientIdFile("firstLegacy"); - await deleteClientIdFile("secondLegacy"); - }); - - it("should set a new clientId because the value is not yet defined", async () => { - const clientId = await getClientId(); - - // The clientId should be generate as uuid - assert.notEqual(clientId, undefined); - assert.notEqual(clientId, CLIENT_ID_CURRENT); - assert.notEqual(clientId, CLIENT_ID_FIRST_LEGACY); - assert.notEqual(clientId, CLIENT_ID_SECOND_LEGACY); - - // The clientId should also be saved in the file - assert.equal(clientId, await getClientIdFromFile("current")); - }); - - it("should get the 'current' clientId because it already exists", async () => { - await createClientIdFile("current"); - const clientId = await getClientId(); - assert.equal(clientId, CLIENT_ID_CURRENT); + await deleteClientIdFile(); }); - it("should get the 'firstLegacy' clientId because it already exists and store it the new analytics file (current)", async () => { - await createClientIdFile("firstLegacy"); + it("should generate a new analytics clientId because the clientId is not yet defined", async () => { + const analyticsClientId = await getClientId(); - const clientId = await getClientId(); - assert.equal(clientId, CLIENT_ID_FIRST_LEGACY); - assert.equal(clientId, await getClientIdFromFile("current")); + // The analyticsClientId should be generate as uuid + assert.notEqual(analyticsClientId, undefined); + assert.notEqual(analyticsClientId, CLIENT_ID); + // The analyticsClientId should also be saved in the file + assert.equal(analyticsClientId, await getClientIdFromFile()); }); - it("should get the 'secondLegacy' clientId because it already exists and store it the new analytics file (current)", async () => { - await createClientIdFile("secondLegacy"); + it("should get the analyticsClientId from the file because it already exists", async () => { + await createClientIdFile(); + const analyticsClientId = await getClientIdFromFile(); - const clientId = await getClientId(); - assert.equal(clientId, CLIENT_ID_SECOND_LEGACY); - assert.equal(clientId, await getClientIdFromFile("current")); + assert.equal(analyticsClientId, CLIENT_ID); }); }); }); From 12a5cd604cecccb3f7eea7a6c2420a5606b8b0bd Mon Sep 17 00:00:00 2001 From: Christopher Dedominici <18092467+ChristopherDedominici@users.noreply.github.com> Date: Mon, 29 Jul 2024 11:37:43 +0200 Subject: [PATCH 10/27] Improve naming for functions --- .../src/internal/cli/telemetry/analytics/analytics.ts | 4 ++-- .../src/internal/cli/telemetry/analytics/utils.ts | 10 +++++----- .../test/internal/cli/telemetry/analytics/utils.ts | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts b/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts index e1d8bcaa3a..77d42f461d 100644 --- a/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts +++ b/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts @@ -12,7 +12,7 @@ import { spawnDetachedSubProcess } from "@ignored/hardhat-vnext-utils/subprocess import { getHardhatVersion } from "../../../utils/package.js"; import { getTelemetryConsent } from "../telemetry-consent.js"; -import { getClientId } from "./utils.js"; +import { getAnalyticsClientId } from "./utils.js"; // TODO:log const log = debug("hardhat:core:global-dir"); @@ -84,7 +84,7 @@ async function buildPayload( eventName: EventNames, eventParams: TaskParams, ): Promise { - const clientId = await getClientId(); + const clientId = await getAnalyticsClientId(); return { client_id: clientId, diff --git a/v-next/hardhat/src/internal/cli/telemetry/analytics/utils.ts b/v-next/hardhat/src/internal/cli/telemetry/analytics/utils.ts index a8e71548b3..4dbbaf82ce 100644 --- a/v-next/hardhat/src/internal/cli/telemetry/analytics/utils.ts +++ b/v-next/hardhat/src/internal/cli/telemetry/analytics/utils.ts @@ -11,21 +11,21 @@ import { const ANALYTICS_FILE_NAME = "analytics.json"; -export async function getClientId(): Promise { - let clientId = await readAnalyticsId(); +export async function getAnalyticsClientId(): Promise { + let clientId = await readAnalyticsClientId(); if (clientId === undefined) { const { v4: uuid } = await import("uuid"); // TODO:log log("Client Id not found, generating a new one"); clientId = uuid(); - await writeAnalyticsId(clientId); + await writeAnalyticsClientId(clientId); } return clientId; } -async function readAnalyticsId(): Promise { +async function readAnalyticsClientId(): Promise { const globalTelemetryDir = await getTelemetryDir(); const filePath = path.join(globalTelemetryDir, ANALYTICS_FILE_NAME); @@ -42,7 +42,7 @@ async function readAnalyticsId(): Promise { return clientId; } -async function writeAnalyticsId(clientId: string): Promise { +async function writeAnalyticsClientId(clientId: string): Promise { const globalTelemetryDir = await getTelemetryDir(); const filePath = path.join(globalTelemetryDir, ANALYTICS_FILE_NAME); await writeJsonFile(filePath, { diff --git a/v-next/hardhat/test/internal/cli/telemetry/analytics/utils.ts b/v-next/hardhat/test/internal/cli/telemetry/analytics/utils.ts index 357f48fe1f..53c7075acf 100644 --- a/v-next/hardhat/test/internal/cli/telemetry/analytics/utils.ts +++ b/v-next/hardhat/test/internal/cli/telemetry/analytics/utils.ts @@ -11,7 +11,7 @@ import { writeJsonFile, } from "@ignored/hardhat-vnext-utils/fs"; -import { getClientId } from "../../../../../src/internal/cli/telemetry/analytics/utils.js"; +import { getAnalyticsClientId } from "../../../../../src/internal/cli/telemetry/analytics/utils.js"; const CLIENT_ID = "test-client-id"; @@ -50,7 +50,7 @@ describe("telemetry/analytics/utils", () => { }); it("should generate a new analytics clientId because the clientId is not yet defined", async () => { - const analyticsClientId = await getClientId(); + const analyticsClientId = await getAnalyticsClientId(); // The analyticsClientId should be generate as uuid assert.notEqual(analyticsClientId, undefined); From 89bbc8af87528bafefb1c2158be10e62b9056a7b Mon Sep 17 00:00:00 2001 From: Christopher Dedominici <18092467+ChristopherDedominici@users.noreply.github.com> Date: Mon, 29 Jul 2024 12:03:24 +0200 Subject: [PATCH 11/27] use builtin method to generate uuid --- pnpm-lock.yaml | 6 ------ v-next/hardhat/package.json | 2 -- .../hardhat/src/internal/cli/telemetry/analytics/utils.ts | 3 +-- 3 files changed, 1 insertion(+), 10 deletions(-) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 213cbadddb..c957e9d686 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1534,9 +1534,6 @@ importers: '@ignored/hardhat-vnext-zod-utils': specifier: workspace:^3.0.0-next.2 version: link:../hardhat-zod-utils - '@types/uuid': - specifier: ^8.3.1 - version: 8.3.4 chalk: specifier: ^5.3.0 version: 5.3.0 @@ -1546,9 +1543,6 @@ importers: tsx: specifier: ^4.11.0 version: 4.11.0 - uuid: - specifier: ^8.3.2 - version: 8.3.2 zod: specifier: ^3.23.8 version: 3.23.8 diff --git a/v-next/hardhat/package.json b/v-next/hardhat/package.json index 8a0f8e2c8b..229d2e1667 100644 --- a/v-next/hardhat/package.json +++ b/v-next/hardhat/package.json @@ -77,11 +77,9 @@ "@ignored/hardhat-vnext-errors": "workspace:^3.0.0-next.2", "@ignored/hardhat-vnext-utils": "workspace:^3.0.0-next.2", "@ignored/hardhat-vnext-zod-utils": "workspace:^3.0.0-next.2", - "@types/uuid": "^8.3.1", "chalk": "^5.3.0", "enquirer": "^2.3.0", "tsx": "^4.11.0", - "uuid": "^8.3.2", "zod": "^3.23.8" } } diff --git a/v-next/hardhat/src/internal/cli/telemetry/analytics/utils.ts b/v-next/hardhat/src/internal/cli/telemetry/analytics/utils.ts index 4dbbaf82ce..dd564d169d 100644 --- a/v-next/hardhat/src/internal/cli/telemetry/analytics/utils.ts +++ b/v-next/hardhat/src/internal/cli/telemetry/analytics/utils.ts @@ -15,9 +15,8 @@ export async function getAnalyticsClientId(): Promise { let clientId = await readAnalyticsClientId(); if (clientId === undefined) { - const { v4: uuid } = await import("uuid"); // TODO:log log("Client Id not found, generating a new one"); - clientId = uuid(); + clientId = crypto.randomUUID(); await writeAnalyticsClientId(clientId); } From f1ab5a47cabf4d13d1ff3419a96b66e3fec4e2d8 Mon Sep 17 00:00:00 2001 From: Christopher Dedominici <18092467+ChristopherDedominici@users.noreply.github.com> Date: Mon, 29 Jul 2024 12:17:36 +0200 Subject: [PATCH 12/27] remove unnecessary lambda functions from subprocess'file --- .../telemetry/analytics/analytics-subprocess.ts | 16 +++++++--------- .../telemetry/analytics/analytics-subprocess.js | 6 ++---- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics-subprocess.ts b/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics-subprocess.ts index 48afbbf35b..b16ee4625d 100644 --- a/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics-subprocess.ts +++ b/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics-subprocess.ts @@ -5,13 +5,11 @@ const ANALYTICS_URL = "https://www.google-analytics.com/mp/collect"; const API_SECRET = "fQ5joCsDRTOp55wX8a2cVw"; const MEASUREMENT_ID = "G-8LQ007N2QJ"; -(async () => { - const payload = JSON.parse(process.argv[2]); +const payload = JSON.parse(process.argv[2]); - await postJsonRequest(ANALYTICS_URL, payload, { - queryParams: { - api_secret: API_SECRET, - measurement_id: MEASUREMENT_ID, - }, - }); -})().catch((_err: unknown) => {}); +await postJsonRequest(ANALYTICS_URL, payload, { + queryParams: { + api_secret: API_SECRET, + measurement_id: MEASUREMENT_ID, + }, +}); diff --git a/v-next/hardhat/test/fixture-projects/cli/telemetry/analytics/analytics-subprocess.js b/v-next/hardhat/test/fixture-projects/cli/telemetry/analytics/analytics-subprocess.js index 5b9fc7931d..6ce089b6fc 100644 --- a/v-next/hardhat/test/fixture-projects/cli/telemetry/analytics/analytics-subprocess.js +++ b/v-next/hardhat/test/fixture-projects/cli/telemetry/analytics/analytics-subprocess.js @@ -15,8 +15,6 @@ const PATH_TO_RESULT_FILE = path.join( "result.json", ); -(() => { - const stringifiedPayload = process.argv[2]; +const stringifiedPayload = process.argv[2]; - writeFileSync(PATH_TO_RESULT_FILE, stringifiedPayload); -})(); +writeFileSync(PATH_TO_RESULT_FILE, stringifiedPayload); From 87f6ab4eedef9e1214d649973cc739650c958461 Mon Sep 17 00:00:00 2001 From: Christopher Dedominici <18092467+ChristopherDedominici@users.noreply.github.com> Date: Mon, 29 Jul 2024 17:39:40 +0200 Subject: [PATCH 13/27] use taskId and subtaskId instead of taskName and scopeName --- .../src/internal/cli/telemetry/analytics/analytics.ts | 8 ++++---- .../hardhat/src/internal/cli/telemetry/analytics/types.ts | 2 +- .../test/internal/cli/telemetry/analytics/analytics.ts | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts b/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts index 77d42f461d..a1c0878886 100644 --- a/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts +++ b/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts @@ -42,12 +42,12 @@ export async function sendTelemetryConsentAnalytics( } export async function sendTaskAnalytics( - taskName: string, - scopeName: string | undefined, + taskId: string, + subtaskId: string | undefined, ): Promise { const eventParams: TaskParams = { - task: taskName, - scope: scopeName, + task: taskId, + subtask: subtaskId, }; return sendAnalytics("task", eventParams); diff --git a/v-next/hardhat/src/internal/cli/telemetry/analytics/types.ts b/v-next/hardhat/src/internal/cli/telemetry/analytics/types.ts index c590345f7a..0209db3898 100644 --- a/v-next/hardhat/src/internal/cli/telemetry/analytics/types.ts +++ b/v-next/hardhat/src/internal/cli/telemetry/analytics/types.ts @@ -36,7 +36,7 @@ export type EventNames = "task"; export interface TaskParams { task: string; - scope?: string; + subtask?: string; } export interface Payload extends BasePayload { diff --git a/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts b/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts index 200b58d80d..5b29cc5e40 100644 --- a/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts +++ b/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts @@ -143,7 +143,7 @@ describe("analytics", () => { it("should create the correct payload for the task analytics", async () => { await setTelemetryConsentFile(true); - const wasSent = await sendTaskAnalytics("hardhat", "compile"); + const wasSent = await sendTaskAnalytics("taskId", "subtaskId"); await checkIfSubprocessWasExecuted(); @@ -164,8 +164,8 @@ describe("analytics", () => { assert.equal(result.events[0].name, "task"); assert.equal(result.events[0].params.engagement_time_msec, "10000"); assert.notEqual(result.events[0].params.session_id, undefined); - assert.equal(result.events[0].params.task, "hardhat"); - assert.equal(result.events[0].params.scope, "compile"); + assert.equal(result.events[0].params.task, "taskId"); + assert.equal(result.events[0].params.subtask, "subtaskId"); }); }); }); From b4b96ed9a5709d0db4ba36c7ef7462ca5668959a Mon Sep 17 00:00:00 2001 From: Christopher Dedominici <18092467+ChristopherDedominici@users.noreply.github.com> Date: Mon, 29 Jul 2024 18:15:07 +0200 Subject: [PATCH 14/27] use test keys fro google analytics --- .../internal/cli/telemetry/analytics/analytics-subprocess.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics-subprocess.ts b/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics-subprocess.ts index b16ee4625d..2d54bdb11e 100644 --- a/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics-subprocess.ts +++ b/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics-subprocess.ts @@ -2,8 +2,8 @@ import { postJsonRequest } from "@ignored/hardhat-vnext-utils/request"; // These keys are expected to be public const ANALYTICS_URL = "https://www.google-analytics.com/mp/collect"; -const API_SECRET = "fQ5joCsDRTOp55wX8a2cVw"; -const MEASUREMENT_ID = "G-8LQ007N2QJ"; +const API_SECRET = "iXzTRik5RhahYpgiatSv1w"; +const MEASUREMENT_ID = "G-ZFZWHGZ64H"; const payload = JSON.parse(process.argv[2]); From 6c7dcf988928f47e3d7480a19f62a8deba2d4c14 Mon Sep 17 00:00:00 2001 From: Christopher Dedominici <18092467+ChristopherDedominici@users.noreply.github.com> Date: Mon, 29 Jul 2024 18:21:46 +0200 Subject: [PATCH 15/27] remove subtaskId, task is an array of string --- .../src/internal/cli/telemetry/analytics/analytics.ts | 8 ++------ .../hardhat/src/internal/cli/telemetry/analytics/types.ts | 1 - .../test/internal/cli/telemetry/analytics/analytics.ts | 5 ++--- 3 files changed, 4 insertions(+), 10 deletions(-) diff --git a/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts b/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts index a1c0878886..e9de54e519 100644 --- a/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts +++ b/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts @@ -41,13 +41,9 @@ export async function sendTelemetryConsentAnalytics( await createSubprocessToSendAnalytics(payload); } -export async function sendTaskAnalytics( - taskId: string, - subtaskId: string | undefined, -): Promise { +export async function sendTaskAnalytics(taskId: string[]): Promise { const eventParams: TaskParams = { - task: taskId, - subtask: subtaskId, + task: taskId.join(", "), }; return sendAnalytics("task", eventParams); diff --git a/v-next/hardhat/src/internal/cli/telemetry/analytics/types.ts b/v-next/hardhat/src/internal/cli/telemetry/analytics/types.ts index 0209db3898..63f205c33f 100644 --- a/v-next/hardhat/src/internal/cli/telemetry/analytics/types.ts +++ b/v-next/hardhat/src/internal/cli/telemetry/analytics/types.ts @@ -36,7 +36,6 @@ export type EventNames = "task"; export interface TaskParams { task: string; - subtask?: string; } export interface Payload extends BasePayload { diff --git a/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts b/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts index 5b29cc5e40..ff097333ef 100644 --- a/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts +++ b/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts @@ -143,7 +143,7 @@ describe("analytics", () => { it("should create the correct payload for the task analytics", async () => { await setTelemetryConsentFile(true); - const wasSent = await sendTaskAnalytics("taskId", "subtaskId"); + const wasSent = await sendTaskAnalytics(["task", "subtask"]); await checkIfSubprocessWasExecuted(); @@ -164,8 +164,7 @@ describe("analytics", () => { assert.equal(result.events[0].name, "task"); assert.equal(result.events[0].params.engagement_time_msec, "10000"); assert.notEqual(result.events[0].params.session_id, undefined); - assert.equal(result.events[0].params.task, "taskId"); - assert.equal(result.events[0].params.subtask, "subtaskId"); + assert.equal(result.events[0].params.task, "task, subtask"); }); }); }); From 2cb67d4fae185fc50d5d672c3a1fdec6fe92a34b Mon Sep 17 00:00:00 2001 From: Christopher Dedominici <18092467+ChristopherDedominici@users.noreply.github.com> Date: Tue, 30 Jul 2024 12:57:03 +0200 Subject: [PATCH 16/27] mark getTelemetryConsent as private and add send user telemetry consent --- .../cli/telemetry/telemetry-permissions.ts | 45 +++++++++---------- .../cli/telemetry/telemetry-permissions.ts | 14 +----- 2 files changed, 22 insertions(+), 37 deletions(-) diff --git a/v-next/hardhat/src/internal/cli/telemetry/telemetry-permissions.ts b/v-next/hardhat/src/internal/cli/telemetry/telemetry-permissions.ts index 3ec868a34f..f862386543 100644 --- a/v-next/hardhat/src/internal/cli/telemetry/telemetry-permissions.ts +++ b/v-next/hardhat/src/internal/cli/telemetry/telemetry-permissions.ts @@ -10,6 +10,8 @@ import { import { confirmationPromptWithTimeout } from "../prompt/prompt.js"; +import { sendTelemetryConsentAnalytics } from "./analytics/analytics.js"; + interface TelemetryConsent { consent: boolean; } @@ -49,24 +51,6 @@ export async function isTelemetryAllowed(): Promise { return consent !== undefined ? consent : false; } -/** - * Retrieves the user's telemetry consent status from the consent file. - * - * @returns True if the user consents to telemetry, false if they do not consent, - * and undefined if no consent has been provided. - */ -export async function getTelemetryConsent(): Promise { - const telemetryConsentFilePath = await getTelemetryConsentFilePath(); - - if (await exists(telemetryConsentFilePath)) { - // Telemetry consent was already provided, hence return the answer - return (await readJsonFile(telemetryConsentFilePath)) - .consent; - } - - return undefined; -} - /** * Determines if telemetry is allowed in the current environment. * This function checks various environmental factors to decide if telemetry data can be collected. @@ -84,6 +68,24 @@ export function isTelemetryAllowedInEnvironment(): boolean { ); } +/** + * Retrieves the user's telemetry consent status from the consent file. + * + * @returns True if the user consents to telemetry, false if they do not consent, + * and undefined if no consent has been provided. + */ +async function getTelemetryConsent(): Promise { + const telemetryConsentFilePath = await getTelemetryConsentFilePath(); + + if (await exists(telemetryConsentFilePath)) { + // Telemetry consent was already provided, hence return the answer + return (await readJsonFile(telemetryConsentFilePath)) + .consent; + } + + return undefined; +} + async function getTelemetryConsentFilePath() { const configDir = await getConfigDir(); return path.join(configDir, "telemetry-consent.json"); @@ -99,12 +101,7 @@ async function requestTelemetryConsent(): Promise { // Store user's consent choice await writeJsonFile(await getTelemetryConsentFilePath(), { consent }); - // TODO: this will be enabled in a following PR as soon as the function to send telemetry is implemented - // const subprocessFilePath = path.join( - // path.dirname(fileURLToPath(import.meta.url)), - // "report-telemetry-consent.js", - // ); - // await spawnDetachedSubProcess(subprocessFilePath, [consent ? "yes" : "no"]); + await sendTelemetryConsentAnalytics(consent); return consent; } diff --git a/v-next/hardhat/test/internal/cli/telemetry/telemetry-permissions.ts b/v-next/hardhat/test/internal/cli/telemetry/telemetry-permissions.ts index e27eeb9ea0..5610bdfe86 100644 --- a/v-next/hardhat/test/internal/cli/telemetry/telemetry-permissions.ts +++ b/v-next/hardhat/test/internal/cli/telemetry/telemetry-permissions.ts @@ -5,10 +5,7 @@ import { afterEach, beforeEach, describe, it } from "node:test"; import { getConfigDir } from "@ignored/hardhat-vnext-core/global-dir"; import { remove, writeJsonFile } from "@ignored/hardhat-vnext-utils/fs"; -import { - getTelemetryConsent, - isTelemetryAllowed, -} from "../../../../src/internal/cli/telemetry/telemetry-permissions.js"; +import { isTelemetryAllowed } from "../../../../src/internal/cli/telemetry/telemetry-permissions.js"; async function setTelemetryConsentFile(consent: boolean) { const configDir = await getConfigDir(); @@ -66,13 +63,4 @@ describe("telemetry-permissions", () => { assert.equal(res, true); }); }); - - it("should return undefined because the telemetry consent is not set", async () => { - // All other possible results are tested in the previous tests, they are included in the the function 'isTelemetryAllowed' - process.env.HARDHAT_ENABLE_TELEMETRY_IN_TEST = "true"; - - const res = await getTelemetryConsent(); - - assert.equal(res, undefined); - }); }); From 12c36c46e6c3ac296cea815eefc9e0005b7ddaf6 Mon Sep 17 00:00:00 2001 From: Christopher Dedominici <18092467+ChristopherDedominici@users.noreply.github.com> Date: Tue, 30 Jul 2024 13:31:16 +0200 Subject: [PATCH 17/27] small improve in logic and added more tests --- .../cli/telemetry/analytics/analytics.ts | 23 ++- .../analytics/telemetry-consent-payload.json | 13 -- .../cli/telemetry/analytics/analytics.ts | 156 ++++++++++++------ 3 files changed, 118 insertions(+), 74 deletions(-) delete mode 100644 v-next/hardhat/test/fixture-projects/cli/telemetry/analytics/telemetry-consent-payload.json diff --git a/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts b/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts index e9de54e519..b5202568cc 100644 --- a/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts +++ b/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts @@ -10,7 +10,10 @@ import os from "node:os"; import { spawnDetachedSubProcess } from "@ignored/hardhat-vnext-utils/subprocess"; import { getHardhatVersion } from "../../../utils/package.js"; -import { getTelemetryConsent } from "../telemetry-consent.js"; +import { + isTelemetryAllowedInEnvironment, + isTelemetryAllowed, +} from "../telemetry-permissions.js"; import { getAnalyticsClientId } from "./utils.js"; @@ -19,11 +22,17 @@ import { getAnalyticsClientId } from "./utils.js"; const SESSION_ID = Math.random().toString(); const ENGAGEMENT_TIME_MSEC = "10000"; +// Return a boolean for testing purposes to verify that analytics are not sent in CI environments export async function sendTelemetryConsentAnalytics( - userConsent: boolean, -): Promise { + consent: boolean, +): Promise { // This is a special scenario where only the consent is sent, all the other analytics info // (like node version, hardhat version, etc.) are stripped. + + if (!isTelemetryAllowedInEnvironment()) { + return false; + } + const payload: TelemetryConsentPayload = { client_id: "hardhat_telemetry_consent", user_id: "hardhat_telemetry_consent", @@ -32,13 +41,15 @@ export async function sendTelemetryConsentAnalytics( { name: "TelemetryConsentResponse", params: { - userConsent: userConsent ? "yes" : "no", + userConsent: consent ? "yes" : "no", }, }, ], }; await createSubprocessToSendAnalytics(payload); + + return true; } export async function sendTaskAnalytics(taskId: string[]): Promise { @@ -49,12 +60,12 @@ export async function sendTaskAnalytics(taskId: string[]): Promise { return sendAnalytics("task", eventParams); } -// Return a boolean for test purposes, so we can check if the analytics was sent based on the consent value +// Return a boolean for testing purposes to confirm whether analytics were sent based on the consent value and not in CI environments async function sendAnalytics( eventName: EventNames, eventParams: TaskParams, ): Promise { - if (!(await getTelemetryConsent())) { + if (!(await isTelemetryAllowed())) { return false; } diff --git a/v-next/hardhat/test/fixture-projects/cli/telemetry/analytics/telemetry-consent-payload.json b/v-next/hardhat/test/fixture-projects/cli/telemetry/analytics/telemetry-consent-payload.json deleted file mode 100644 index 3f7afa0a72..0000000000 --- a/v-next/hardhat/test/fixture-projects/cli/telemetry/analytics/telemetry-consent-payload.json +++ /dev/null @@ -1,13 +0,0 @@ -{ - "client_id": "hardhat_telemetry_consent", - "user_id": "hardhat_telemetry_consent", - "user_properties": {}, - "events": [ - { - "name": "TelemetryConsentResponse", - "params": { - "userConsent": "yes" - } - } - ] -} diff --git a/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts b/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts index ff097333ef..fe191d0e5f 100644 --- a/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts +++ b/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts @@ -105,66 +105,112 @@ describe("analytics", () => { await remove(RESULT_FILE_PATH); }); - it("should create the correct payload for the telemetry consent", async () => { - await sendTelemetryConsentAnalytics(true); - - await checkIfSubprocessWasExecuted(); - - const result = await readJsonFile(RESULT_FILE_PATH); - - const expected = await readJsonFile( - path.join(PATH_TO_FIXTURE, "telemetry-consent-payload.json"), - ); - - assert.deepEqual(result, expected); - }); - describe("analytics payload", async () => { const ORIGINAL_PROCESS_ENV = { ...process }; - before(() => { - // Force Ci to not be detected as Ci so the test can run (Ci is blocked for analytics) - delete process.env.GITHUB_ACTIONS; - delete process.env.NOW; - delete process.env.DEPLOYMENT_ID; - delete process.env.CODEBUILD_BUILD_NUMBER; - delete process.env.CI; - delete process.env.CONTINUOUS_INTEGRATION; - delete process.env.BUILD_NUMBER; - delete process.env.RUN_ID; - - process.stdout.isTTY = true; + describe("not running in CI", () => { + before(() => { + // Force Ci to not be detected as Ci so the test can run (Ci is blocked for analytics) + process.env.HARDHAT_ENABLE_TELEMETRY_IN_TEST = "true"; + }); + + after(() => { + delete process.env.HARDHAT_ENABLE_TELEMETRY_IN_TEST; + process = ORIGINAL_PROCESS_ENV; + }); + + it("should create the correct payload for the telemetry consent (positive consent)", async () => { + await sendTelemetryConsentAnalytics(true); + + await checkIfSubprocessWasExecuted(); + + const result = await readJsonFile(RESULT_FILE_PATH); + + assert.deepEqual(result, { + client_id: "hardhat_telemetry_consent", + user_id: "hardhat_telemetry_consent", + user_properties: {}, + events: [ + { + name: "TelemetryConsentResponse", + params: { + userConsent: "yes", + }, + }, + ], + }); + }); + + it("should create the correct payload for the telemetry consent (negative consent)", async () => { + await sendTelemetryConsentAnalytics(false); + + await checkIfSubprocessWasExecuted(); + + const result = await readJsonFile(RESULT_FILE_PATH); + + assert.deepEqual(result, { + client_id: "hardhat_telemetry_consent", + user_id: "hardhat_telemetry_consent", + user_properties: {}, + events: [ + { + name: "TelemetryConsentResponse", + params: { + userConsent: "no", + }, + }, + ], + }); + }); + + it("should create the correct payload for the task analytics", async () => { + await setTelemetryConsentFile(true); + + const wasSent = await sendTaskAnalytics(["task", "subtask"]); + + await checkIfSubprocessWasExecuted(); + + const result: Payload = await readJsonFile(RESULT_FILE_PATH); + + assert.equal(wasSent, true); + + // Check payload properties + assert.notEqual(result.client_id, undefined); + assert.notEqual(result.user_id, undefined); + assert.equal(result.user_properties.projectId.value, "hardhat-project"); + assert.equal( + result.user_properties.hardhatVersion.value, + await getHardhatVersion(), + ); + assert.notEqual( + result.user_properties.operatingSystem.value, + undefined, + ); + assert.notEqual(result.user_properties.nodeVersion.value, undefined); + assert.equal(result.events[0].name, "task"); + assert.equal(result.events[0].params.engagement_time_msec, "10000"); + assert.notEqual(result.events[0].params.session_id, undefined); + assert.equal(result.events[0].params.task, "task, subtask"); + }); + + it("should not send analytics because the consent is not given", async () => { + await setTelemetryConsentFile(false); + const wasSent = await sendTaskAnalytics(["task", "subtask"]); + assert.equal(wasSent, false); + }); }); - after(() => { - process = ORIGINAL_PROCESS_ENV; - }); - - it("should create the correct payload for the task analytics", async () => { - await setTelemetryConsentFile(true); - - const wasSent = await sendTaskAnalytics(["task", "subtask"]); - - await checkIfSubprocessWasExecuted(); - - const result: Payload = await readJsonFile(RESULT_FILE_PATH); - - assert.equal(wasSent, true); - - // Check payload properties - assert.notEqual(result.client_id, undefined); - assert.notEqual(result.user_id, undefined); - assert.equal(result.user_properties.projectId.value, "hardhat-project"); - assert.equal( - result.user_properties.hardhatVersion.value, - await getHardhatVersion(), - ); - assert.notEqual(result.user_properties.operatingSystem.value, undefined); - assert.notEqual(result.user_properties.nodeVersion.value, undefined); - assert.equal(result.events[0].name, "task"); - assert.equal(result.events[0].params.engagement_time_msec, "10000"); - assert.notEqual(result.events[0].params.session_id, undefined); - assert.equal(result.events[0].params.task, "task, subtask"); + describe("running in non interactive environment", () => { + it("should not send consent because the environment is non interactive", async () => { + const wasSent = await sendTelemetryConsentAnalytics(true); + assert.equal(wasSent, false); + }); + + it("should not send analytics because the environment is not interactive", async () => { + await setTelemetryConsentFile(true); + const wasSent = await sendTaskAnalytics(["task", "subtask"]); + assert.equal(wasSent, false); + }); }); }); }); From a3202f102aefe434018e54ff229b548ad5f4a288 Mon Sep 17 00:00:00 2001 From: Christopher Dedominici <18092467+ChristopherDedominici@users.noreply.github.com> Date: Fri, 2 Aug 2024 19:56:30 +0200 Subject: [PATCH 18/27] add a helper file for telemetry tests --- .../test/internal/cli/telemetry/helpers.ts | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 v-next/hardhat/test/internal/cli/telemetry/helpers.ts diff --git a/v-next/hardhat/test/internal/cli/telemetry/helpers.ts b/v-next/hardhat/test/internal/cli/telemetry/helpers.ts new file mode 100644 index 0000000000..e76fb1db4c --- /dev/null +++ b/v-next/hardhat/test/internal/cli/telemetry/helpers.ts @@ -0,0 +1,44 @@ +import path from "node:path"; + +import { exists } from "@ignored/hardhat-vnext-utils/fs"; + +export const ROOT_PATH_TO_FIXTURE: string = path.join( + process.cwd(), + "test", + "fixture-projects", + "cli", + "telemetry", +); + +export const TELEMETRY_FOLDER_PATH: string = path.join( + process.cwd(), + "src", + "internal", + "cli", + "telemetry", +); + +export async function checkIfSubprocessWasExecuted( + resultFilePath: string, +): Promise { + // Checks if the subprocess was executed by waiting for a file to be created. + // Uses an interval to periodically check for the file. If the file isn't found + // within a specified number of attempts, an error is thrown, indicating a failure in subprocess execution. + const MAX_COUNTER = 20; + + return new Promise((resolve, reject) => { + let counter = 0; + + const intervalId = setInterval(async () => { + counter++; + + if (await exists(resultFilePath)) { + clearInterval(intervalId); + resolve(true); + } else if (counter > MAX_COUNTER) { + clearInterval(intervalId); + reject("Subprocess was not executed in the expected time"); + } + }, 100); + }); +} From 0fd655573b4867b9e43f1ac29a554048fd0b14c2 Mon Sep 17 00:00:00 2001 From: Christopher Dedominici <18092467+ChristopherDedominici@users.noreply.github.com> Date: Fri, 2 Aug 2024 19:57:41 +0200 Subject: [PATCH 19/27] add ENV variables to simplify testing (e.g.: force to run in non interactive env) --- .../analytics/analytics-subprocess.ts | 15 - .../cli/telemetry/analytics/analytics.ts | 16 +- .../cli/telemetry/analytics/subprocess.ts | 27 ++ .../cli/telemetry/telemetry-permissions.ts | 10 +- .../cli/telemetry/analytics/analytics.ts | 265 ++++++------------ .../cli/telemetry/telemetry-permissions.ts | 12 +- 6 files changed, 146 insertions(+), 199 deletions(-) delete mode 100644 v-next/hardhat/src/internal/cli/telemetry/analytics/analytics-subprocess.ts create mode 100644 v-next/hardhat/src/internal/cli/telemetry/analytics/subprocess.ts diff --git a/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics-subprocess.ts b/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics-subprocess.ts deleted file mode 100644 index 2d54bdb11e..0000000000 --- a/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics-subprocess.ts +++ /dev/null @@ -1,15 +0,0 @@ -import { postJsonRequest } from "@ignored/hardhat-vnext-utils/request"; - -// These keys are expected to be public -const ANALYTICS_URL = "https://www.google-analytics.com/mp/collect"; -const API_SECRET = "iXzTRik5RhahYpgiatSv1w"; -const MEASUREMENT_ID = "G-ZFZWHGZ64H"; - -const payload = JSON.parse(process.argv[2]); - -await postJsonRequest(ANALYTICS_URL, payload, { - queryParams: { - api_secret: API_SECRET, - measurement_id: MEASUREMENT_ID, - }, -}); diff --git a/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts b/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts index b5202568cc..415a75190c 100644 --- a/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts +++ b/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts @@ -79,12 +79,18 @@ async function sendAnalytics( async function createSubprocessToSendAnalytics( payload: TelemetryConsentPayload | Payload, ): Promise { - // The file extension is 'js' because the 'ts' file will be compiled - const analyticsSubprocessFilePath = `${import.meta.dirname}/analytics-subprocess.js`; + const fileExt = + process.env.HARDHAT_TEST_SUBPROCESS_RESULT_PATH !== undefined ? "ts" : "js"; + const subprocessFile = `${import.meta.dirname}/subprocess.${fileExt}`; + + const env: Record = {}; + if (process.env.HARDHAT_TEST_SUBPROCESS_RESULT_PATH !== undefined) { + // ATTENTION: only for testing + env.HARDHAT_TEST_SUBPROCESS_RESULT_PATH = + process.env.HARDHAT_TEST_SUBPROCESS_RESULT_PATH; + } - await spawnDetachedSubProcess(analyticsSubprocessFilePath, [ - JSON.stringify(payload), - ]); + await spawnDetachedSubProcess(subprocessFile, [JSON.stringify(payload)], env); } async function buildPayload( diff --git a/v-next/hardhat/src/internal/cli/telemetry/analytics/subprocess.ts b/v-next/hardhat/src/internal/cli/telemetry/analytics/subprocess.ts new file mode 100644 index 0000000000..5d14b155f0 --- /dev/null +++ b/v-next/hardhat/src/internal/cli/telemetry/analytics/subprocess.ts @@ -0,0 +1,27 @@ +import { writeJsonFile } from "@ignored/hardhat-vnext-utils/fs"; +import { postJsonRequest } from "@ignored/hardhat-vnext-utils/request"; + +// These keys are expected to be public +const ANALYTICS_URL = "https://www.google-analytics.com/mp/collect"; +const API_SECRET = "iXzTRik5RhahYpgiatSv1w"; +const MEASUREMENT_ID = "G-ZFZWHGZ64H"; + +async function main(): Promise { + const payload = JSON.parse(process.argv[2]); + + if (process.env.HARDHAT_TEST_SUBPROCESS_RESULT_PATH === undefined) { + await postJsonRequest(ANALYTICS_URL, payload, { + queryParams: { + api_secret: API_SECRET, + measurement_id: MEASUREMENT_ID, + }, + }); + + return; + } + + // ATTENTION: only for testing + await writeJsonFile(process.env.HARDHAT_TEST_SUBPROCESS_RESULT_PATH, payload); +} + +await main(); diff --git a/v-next/hardhat/src/internal/cli/telemetry/telemetry-permissions.ts b/v-next/hardhat/src/internal/cli/telemetry/telemetry-permissions.ts index f862386543..eec381095a 100644 --- a/v-next/hardhat/src/internal/cli/telemetry/telemetry-permissions.ts +++ b/v-next/hardhat/src/internal/cli/telemetry/telemetry-permissions.ts @@ -48,6 +48,14 @@ export async function isTelemetryAllowed(): Promise { } const consent = await getTelemetryConsent(); + + // ATTENTION: only for testing + if (process.env.HARDHAT_TEST_TELEMETRY_CONSENT_VALUE !== undefined) { + return process.env.HARDHAT_TEST_TELEMETRY_CONSENT_VALUE === "true" + ? true + : false; + } + return consent !== undefined ? consent : false; } @@ -64,7 +72,7 @@ export function isTelemetryAllowedInEnvironment(): boolean { (!isCi() && process.stdout.isTTY === true && process.env.HARDHAT_DISABLE_TELEMETRY_PROMPT !== "true") || - process.env.HARDHAT_ENABLE_TELEMETRY_IN_TEST === "true" // Used in tests to force telemetry execution + process.env.HARDHAT_TEST_INTERACTIVE_ENV === "true" // Used in tests to force telemetry execution ); } diff --git a/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts b/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts index fe191d0e5f..a0ab62ea96 100644 --- a/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts +++ b/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts @@ -4,213 +4,134 @@ import assert from "node:assert/strict"; import path from "node:path"; import { after, afterEach, before, beforeEach, describe, it } from "node:test"; -import { getConfigDir } from "@ignored/hardhat-vnext-core/global-dir"; -import { - copy, - exists, - readJsonFile, - remove, - writeJsonFile, -} from "@ignored/hardhat-vnext-utils/fs"; +import { readJsonFile, remove } from "@ignored/hardhat-vnext-utils/fs"; import { sendTaskAnalytics, sendTelemetryConsentAnalytics, } from "../../../../../src/internal/cli/telemetry/analytics/analytics.js"; import { getHardhatVersion } from "../../../../../src/internal/utils/package.js"; +import { + checkIfSubprocessWasExecuted, + ROOT_PATH_TO_FIXTURE, +} from "../helpers.js"; -// The analytics logic uses a detached subprocess to send the payload via HTTP call. -// We cannot test the HTTP call directly, but we can use a test subprocess to verify if the payload is correctly created. -// This is possible because the analytics code attempts to execute a subprocess file of type 'JS'. JS files are only available after compilation. -// During the tests, no JS file is available, so the expected subprocess does not exist. Therefore, we can copy a test subprocess file -// to the expected location instead of the original one and check if it receives the correct payload. - -const PATH_TO_FIXTURE = path.join( - process.cwd(), - "test", - "fixture-projects", - "cli", - "telemetry", - "analytics", -); - -const SOURCE_PATH_TEST_SUBPROCESS_FILE = path.join( - PATH_TO_FIXTURE, - "analytics-subprocess.js", -); - -const DEST_PATH_TEST_SUBPROCESS_FILE = path.join( - process.cwd(), - "src", - "internal", - "cli", - "telemetry", - "analytics", - "analytics-subprocess.js", -); - +const PATH_TO_FIXTURE = path.join(ROOT_PATH_TO_FIXTURE, "analytics"); const RESULT_FILE_PATH = path.join(PATH_TO_FIXTURE, "result.json"); -async function copyTestSubprocessFile() { - await copy(SOURCE_PATH_TEST_SUBPROCESS_FILE, DEST_PATH_TEST_SUBPROCESS_FILE); -} - -async function removeTestSubprocessFile() { - remove(DEST_PATH_TEST_SUBPROCESS_FILE); -} - -async function setTelemetryConsentFile(consent: boolean) { - const configDir = await getConfigDir(); - const filePath = path.join(configDir, "telemetry-consent.json"); - await writeJsonFile(filePath, { consent }); -} - -async function checkIfSubprocessWasExecuted() { - // Checks if the subprocess was executed by waiting for a file to be created. - // Uses an interval to periodically check for the file. If the file isn't found - // within a specified number of attempts, an error is thrown, indicating a failure in subprocess execution. - const MAX_COUNTER = 20; - - return new Promise((resolve, reject) => { - let counter = 0; - - const intervalId = setInterval(async () => { - counter++; - - if (await exists(RESULT_FILE_PATH)) { - clearInterval(intervalId); - resolve(true); - } else if (counter > MAX_COUNTER) { - clearInterval(intervalId); - reject("Subprocess was not executed in the expected time"); - } - }, 100); - }); -} - describe("analytics", () => { - before(async () => { - copyTestSubprocessFile(); + beforeEach(async () => { + process.env.HARDHAT_TEST_TELEMETRY_CONSENT_VALUE = "true"; }); - after(async () => { - await removeTestSubprocessFile(); + afterEach(async () => { + delete process.env.HARDHAT_TEST_TELEMETRY_CONSENT_VALUE; }); - beforeEach(async () => { - await remove(RESULT_FILE_PATH); - }); + describe("running in non interactive environment", () => { + it("should not send consent because the environment is non interactive", async () => { + const wasSent = await sendTelemetryConsentAnalytics(true); + assert.equal(wasSent, false); + }); - afterEach(async () => { - await remove(RESULT_FILE_PATH); + it("should not send analytics because the environment is not interactive", async () => { + const wasSent = await sendTaskAnalytics(["task", "subtask"]); + assert.equal(wasSent, false); + }); }); - describe("analytics payload", async () => { - const ORIGINAL_PROCESS_ENV = { ...process }; + describe("running in an interactive environment (simulated with ENV variables)", () => { + before(() => { + process.env.HARDHAT_TEST_INTERACTIVE_ENV = "true"; + process.env.HARDHAT_TEST_SUBPROCESS_RESULT_PATH = RESULT_FILE_PATH; + }); + + after(() => { + delete process.env.HARDHAT_TEST_INTERACTIVE_ENV; + }); - describe("not running in CI", () => { - before(() => { - // Force Ci to not be detected as Ci so the test can run (Ci is blocked for analytics) - process.env.HARDHAT_ENABLE_TELEMETRY_IN_TEST = "true"; - }); + beforeEach(async () => { + await remove(RESULT_FILE_PATH); + }); - after(() => { - delete process.env.HARDHAT_ENABLE_TELEMETRY_IN_TEST; - process = ORIGINAL_PROCESS_ENV; - }); + afterEach(async () => { + await remove(RESULT_FILE_PATH); + }); - it("should create the correct payload for the telemetry consent (positive consent)", async () => { - await sendTelemetryConsentAnalytics(true); + it("should create the correct payload for the telemetry consent (positive consent)", async () => { + await sendTelemetryConsentAnalytics(true); - await checkIfSubprocessWasExecuted(); + await checkIfSubprocessWasExecuted(RESULT_FILE_PATH); - const result = await readJsonFile(RESULT_FILE_PATH); + const result = await readJsonFile(RESULT_FILE_PATH); - assert.deepEqual(result, { - client_id: "hardhat_telemetry_consent", - user_id: "hardhat_telemetry_consent", - user_properties: {}, - events: [ - { - name: "TelemetryConsentResponse", - params: { - userConsent: "yes", - }, + assert.deepEqual(result, { + client_id: "hardhat_telemetry_consent", + user_id: "hardhat_telemetry_consent", + user_properties: {}, + events: [ + { + name: "TelemetryConsentResponse", + params: { + userConsent: "yes", }, - ], - }); + }, + ], }); + }); - it("should create the correct payload for the telemetry consent (negative consent)", async () => { - await sendTelemetryConsentAnalytics(false); + it("should create the correct payload for the telemetry consent (negative consent)", async () => { + await sendTelemetryConsentAnalytics(false); - await checkIfSubprocessWasExecuted(); + await checkIfSubprocessWasExecuted(RESULT_FILE_PATH); - const result = await readJsonFile(RESULT_FILE_PATH); + const result = await readJsonFile(RESULT_FILE_PATH); - assert.deepEqual(result, { - client_id: "hardhat_telemetry_consent", - user_id: "hardhat_telemetry_consent", - user_properties: {}, - events: [ - { - name: "TelemetryConsentResponse", - params: { - userConsent: "no", - }, + assert.deepEqual(result, { + client_id: "hardhat_telemetry_consent", + user_id: "hardhat_telemetry_consent", + user_properties: {}, + events: [ + { + name: "TelemetryConsentResponse", + params: { + userConsent: "no", }, - ], - }); - }); - - it("should create the correct payload for the task analytics", async () => { - await setTelemetryConsentFile(true); - - const wasSent = await sendTaskAnalytics(["task", "subtask"]); - - await checkIfSubprocessWasExecuted(); - - const result: Payload = await readJsonFile(RESULT_FILE_PATH); - - assert.equal(wasSent, true); - - // Check payload properties - assert.notEqual(result.client_id, undefined); - assert.notEqual(result.user_id, undefined); - assert.equal(result.user_properties.projectId.value, "hardhat-project"); - assert.equal( - result.user_properties.hardhatVersion.value, - await getHardhatVersion(), - ); - assert.notEqual( - result.user_properties.operatingSystem.value, - undefined, - ); - assert.notEqual(result.user_properties.nodeVersion.value, undefined); - assert.equal(result.events[0].name, "task"); - assert.equal(result.events[0].params.engagement_time_msec, "10000"); - assert.notEqual(result.events[0].params.session_id, undefined); - assert.equal(result.events[0].params.task, "task, subtask"); + }, + ], }); + }); - it("should not send analytics because the consent is not given", async () => { - await setTelemetryConsentFile(false); - const wasSent = await sendTaskAnalytics(["task", "subtask"]); - assert.equal(wasSent, false); - }); + it("should create the correct payload for the task analytics", async () => { + const wasSent = await sendTaskAnalytics(["task", "subtask"]); + + await checkIfSubprocessWasExecuted(RESULT_FILE_PATH); + + const result: Payload = await readJsonFile(RESULT_FILE_PATH); + + assert.equal(wasSent, true); + + // Check payload properties + assert.notEqual(result.client_id, undefined); + assert.notEqual(result.user_id, undefined); + assert.equal(result.user_properties.projectId.value, "hardhat-project"); + assert.equal( + result.user_properties.hardhatVersion.value, + await getHardhatVersion(), + ); + assert.notEqual(result.user_properties.operatingSystem.value, undefined); + assert.notEqual(result.user_properties.nodeVersion.value, undefined); + assert.equal(result.events[0].name, "task"); + assert.equal(result.events[0].params.engagement_time_msec, "10000"); + assert.notEqual(result.events[0].params.session_id, undefined); + assert.equal(result.events[0].params.task, "task, subtask"); }); - describe("running in non interactive environment", () => { - it("should not send consent because the environment is non interactive", async () => { - const wasSent = await sendTelemetryConsentAnalytics(true); - assert.equal(wasSent, false); - }); + it("should not send analytics because the consent is not given", async () => { + process.env.HARDHAT_TEST_TELEMETRY_CONSENT_VALUE = "false"; - it("should not send analytics because the environment is not interactive", async () => { - await setTelemetryConsentFile(true); - const wasSent = await sendTaskAnalytics(["task", "subtask"]); - assert.equal(wasSent, false); - }); + const wasSent = await sendTaskAnalytics(["task", "subtask"]); + assert.equal(wasSent, false); }); }); }); diff --git a/v-next/hardhat/test/internal/cli/telemetry/telemetry-permissions.ts b/v-next/hardhat/test/internal/cli/telemetry/telemetry-permissions.ts index 5610bdfe86..130a4f169a 100644 --- a/v-next/hardhat/test/internal/cli/telemetry/telemetry-permissions.ts +++ b/v-next/hardhat/test/internal/cli/telemetry/telemetry-permissions.ts @@ -21,27 +21,27 @@ async function deleteTelemetryConsentFile() { describe("telemetry-permissions", () => { beforeEach(async () => { - delete process.env.HARDHAT_ENABLE_TELEMETRY_IN_TEST; + delete process.env.HARDHAT_TEST_INTERACTIVE_ENV; await deleteTelemetryConsentFile(); }); afterEach(async () => { - delete process.env.HARDHAT_ENABLE_TELEMETRY_IN_TEST; + delete process.env.HARDHAT_TEST_INTERACTIVE_ENV; await deleteTelemetryConsentFile(); }); describe("isTelemetryAllowed", () => { it("should return false because not an interactive environment", async () => { - await setTelemetryConsentFile(true); // Needed to be sure that the file is not read and the process exits before + await setTelemetryConsentFile(true); const res = await isTelemetryAllowed(); assert.equal(res, false); }); it("should return false because the user did not give telemetry consent", async () => { - process.env.HARDHAT_ENABLE_TELEMETRY_IN_TEST = "true"; + process.env.HARDHAT_TEST_INTERACTIVE_ENV = "true"; await setTelemetryConsentFile(false); const res = await isTelemetryAllowed(); @@ -49,14 +49,14 @@ describe("telemetry-permissions", () => { }); it("should return false because the telemetry consent is not set", async () => { - process.env.HARDHAT_ENABLE_TELEMETRY_IN_TEST = "true"; + process.env.HARDHAT_TEST_INTERACTIVE_ENV = "true"; const res = await isTelemetryAllowed(); assert.equal(res, false); }); it("should return true because the user gave telemetry consent", async () => { - process.env.HARDHAT_ENABLE_TELEMETRY_IN_TEST = "true"; + process.env.HARDHAT_TEST_INTERACTIVE_ENV = "true"; await setTelemetryConsentFile(true); const res = await isTelemetryAllowed(); From 141174df44891a4a4702e9573bd869fd7c4b31c2 Mon Sep 17 00:00:00 2001 From: Christopher Dedominici <18092467+ChristopherDedominici@users.noreply.github.com> Date: Fri, 2 Aug 2024 20:05:03 +0200 Subject: [PATCH 20/27] Add test explanation --- .../test/internal/cli/telemetry/analytics/analytics.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts b/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts index a0ab62ea96..ddbe769249 100644 --- a/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts +++ b/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts @@ -16,6 +16,12 @@ import { ROOT_PATH_TO_FIXTURE, } from "../helpers.js"; +// +// TEST EXPLANATION: When setting the environment variable HARDHAT_TEST_SUBPROCESS_RESULT_PATH, +// the subprocess writes the payload to the specified file instead of sending it to a remote server. +// This allows us to verify that the payload is formatted correctly. +// + const PATH_TO_FIXTURE = path.join(ROOT_PATH_TO_FIXTURE, "analytics"); const RESULT_FILE_PATH = path.join(PATH_TO_FIXTURE, "result.json"); From f1c7c7d891bd7096eafb46d49162a6f184f65c64 Mon Sep 17 00:00:00 2001 From: Christopher Dedominici <18092467+ChristopherDedominici@users.noreply.github.com> Date: Mon, 5 Aug 2024 10:52:32 +0200 Subject: [PATCH 21/27] move the check on the ENV variable to force telemetry consent in tests --- .../src/internal/cli/telemetry/telemetry-permissions.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/v-next/hardhat/src/internal/cli/telemetry/telemetry-permissions.ts b/v-next/hardhat/src/internal/cli/telemetry/telemetry-permissions.ts index eec381095a..16bba81851 100644 --- a/v-next/hardhat/src/internal/cli/telemetry/telemetry-permissions.ts +++ b/v-next/hardhat/src/internal/cli/telemetry/telemetry-permissions.ts @@ -47,8 +47,6 @@ export async function isTelemetryAllowed(): Promise { return false; } - const consent = await getTelemetryConsent(); - // ATTENTION: only for testing if (process.env.HARDHAT_TEST_TELEMETRY_CONSENT_VALUE !== undefined) { return process.env.HARDHAT_TEST_TELEMETRY_CONSENT_VALUE === "true" @@ -56,6 +54,8 @@ export async function isTelemetryAllowed(): Promise { : false; } + const consent = await getTelemetryConsent(); + return consent !== undefined ? consent : false; } @@ -72,7 +72,8 @@ export function isTelemetryAllowedInEnvironment(): boolean { (!isCi() && process.stdout.isTTY === true && process.env.HARDHAT_DISABLE_TELEMETRY_PROMPT !== "true") || - process.env.HARDHAT_TEST_INTERACTIVE_ENV === "true" // Used in tests to force telemetry execution + // ATTENTION: used in tests to force telemetry execution + process.env.HARDHAT_TEST_INTERACTIVE_ENV === "true" ); } From 3e59fa5af5ec32e01d1b158f487cbcc8dad42769 Mon Sep 17 00:00:00 2001 From: Christopher Dedominici <18092467+ChristopherDedominici@users.noreply.github.com> Date: Mon, 5 Aug 2024 15:05:50 +0200 Subject: [PATCH 22/27] modify helper: wait for subprocess file to be readable --- v-next/hardhat/test/internal/cli/telemetry/helpers.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/v-next/hardhat/test/internal/cli/telemetry/helpers.ts b/v-next/hardhat/test/internal/cli/telemetry/helpers.ts index e76fb1db4c..7d43fc6ffb 100644 --- a/v-next/hardhat/test/internal/cli/telemetry/helpers.ts +++ b/v-next/hardhat/test/internal/cli/telemetry/helpers.ts @@ -1,6 +1,6 @@ import path from "node:path"; -import { exists } from "@ignored/hardhat-vnext-utils/fs"; +import { exists, readUtf8File } from "@ignored/hardhat-vnext-utils/fs"; export const ROOT_PATH_TO_FIXTURE: string = path.join( process.cwd(), @@ -33,12 +33,15 @@ export async function checkIfSubprocessWasExecuted( counter++; if (await exists(resultFilePath)) { - clearInterval(intervalId); - resolve(true); + try { + await readUtf8File(resultFilePath); // Wait for the file to be readable + clearInterval(intervalId); + resolve(true); + } catch (_err) {} } else if (counter > MAX_COUNTER) { clearInterval(intervalId); reject("Subprocess was not executed in the expected time"); } - }, 100); + }, 50); }); } From 39a151194c6d9a63c15a5f2845c54de53c63f9c3 Mon Sep 17 00:00:00 2001 From: Christopher Dedominici <18092467+ChristopherDedominici@users.noreply.github.com> Date: Mon, 5 Aug 2024 15:45:52 +0200 Subject: [PATCH 23/27] add logic to wait for the file to be readable --- .../internal/cli/telemetry/analytics/analytics.ts | 6 +++--- .../test/internal/cli/telemetry/helpers.ts | 15 +++++++++++++-- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts b/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts index ddbe769249..854db6f394 100644 --- a/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts +++ b/v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts @@ -67,7 +67,7 @@ describe("analytics", () => { it("should create the correct payload for the telemetry consent (positive consent)", async () => { await sendTelemetryConsentAnalytics(true); - await checkIfSubprocessWasExecuted(RESULT_FILE_PATH); + await checkIfSubprocessWasExecuted(RESULT_FILE_PATH, true); const result = await readJsonFile(RESULT_FILE_PATH); @@ -89,7 +89,7 @@ describe("analytics", () => { it("should create the correct payload for the telemetry consent (negative consent)", async () => { await sendTelemetryConsentAnalytics(false); - await checkIfSubprocessWasExecuted(RESULT_FILE_PATH); + await checkIfSubprocessWasExecuted(RESULT_FILE_PATH, true); const result = await readJsonFile(RESULT_FILE_PATH); @@ -111,7 +111,7 @@ describe("analytics", () => { it("should create the correct payload for the task analytics", async () => { const wasSent = await sendTaskAnalytics(["task", "subtask"]); - await checkIfSubprocessWasExecuted(RESULT_FILE_PATH); + await checkIfSubprocessWasExecuted(RESULT_FILE_PATH, true); const result: Payload = await readJsonFile(RESULT_FILE_PATH); diff --git a/v-next/hardhat/test/internal/cli/telemetry/helpers.ts b/v-next/hardhat/test/internal/cli/telemetry/helpers.ts index 7d43fc6ffb..14fdd287e4 100644 --- a/v-next/hardhat/test/internal/cli/telemetry/helpers.ts +++ b/v-next/hardhat/test/internal/cli/telemetry/helpers.ts @@ -1,6 +1,10 @@ import path from "node:path"; -import { exists, readUtf8File } from "@ignored/hardhat-vnext-utils/fs"; +import { + exists, + readJsonFile, + readUtf8File, +} from "@ignored/hardhat-vnext-utils/fs"; export const ROOT_PATH_TO_FIXTURE: string = path.join( process.cwd(), @@ -20,6 +24,7 @@ export const TELEMETRY_FOLDER_PATH: string = path.join( export async function checkIfSubprocessWasExecuted( resultFilePath: string, + isJsonFile: boolean = false, ): Promise { // Checks if the subprocess was executed by waiting for a file to be created. // Uses an interval to periodically check for the file. If the file isn't found @@ -34,7 +39,13 @@ export async function checkIfSubprocessWasExecuted( if (await exists(resultFilePath)) { try { - await readUtf8File(resultFilePath); // Wait for the file to be readable + // Wait for the file to be readable. The file could exist but the writing could be in progress. + if (isJsonFile) { + await readJsonFile(resultFilePath); + } else { + await readUtf8File(resultFilePath); + } + clearInterval(intervalId); resolve(true); } catch (_err) {} From b9df707e918eec0972bf5a2064c4c9c03c918acb Mon Sep 17 00:00:00 2001 From: Christopher Dedominici <18092467+ChristopherDedominici@users.noreply.github.com> Date: Tue, 6 Aug 2024 11:48:03 +0200 Subject: [PATCH 24/27] Smaller fixes based on the PR's comments --- .../cli/telemetry/analytics/analytics.ts | 4 +++ .../cli/telemetry/analytics/subprocess.ts | 25 ++++++++----------- .../cli/telemetry/telemetry-permissions.ts | 2 +- .../analytics/analytics-subprocess.js | 20 --------------- 4 files changed, 15 insertions(+), 36 deletions(-) delete mode 100644 v-next/hardhat/test/fixture-projects/cli/telemetry/analytics/analytics-subprocess.js diff --git a/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts b/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts index 415a75190c..77ecb25f5c 100644 --- a/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts +++ b/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts @@ -79,6 +79,10 @@ async function sendAnalytics( async function createSubprocessToSendAnalytics( payload: TelemetryConsentPayload | Payload, ): Promise { + // The HARDHAT_TEST_SUBPROCESS_RESULT_PATH env variable is used in the tests to instruct the subprocess to write the payload to a file + // instead of sending it. + // During testing, the subprocess file is a ts file, whereas in production, it is a js file (compiled code). + // The following lines adjust the file extension based on whether the environment is for testing or production. const fileExt = process.env.HARDHAT_TEST_SUBPROCESS_RESULT_PATH !== undefined ? "ts" : "js"; const subprocessFile = `${import.meta.dirname}/subprocess.${fileExt}`; diff --git a/v-next/hardhat/src/internal/cli/telemetry/analytics/subprocess.ts b/v-next/hardhat/src/internal/cli/telemetry/analytics/subprocess.ts index 5d14b155f0..08c15355a7 100644 --- a/v-next/hardhat/src/internal/cli/telemetry/analytics/subprocess.ts +++ b/v-next/hardhat/src/internal/cli/telemetry/analytics/subprocess.ts @@ -2,26 +2,21 @@ import { writeJsonFile } from "@ignored/hardhat-vnext-utils/fs"; import { postJsonRequest } from "@ignored/hardhat-vnext-utils/request"; // These keys are expected to be public +// TODO: replace with prod values const ANALYTICS_URL = "https://www.google-analytics.com/mp/collect"; const API_SECRET = "iXzTRik5RhahYpgiatSv1w"; const MEASUREMENT_ID = "G-ZFZWHGZ64H"; -async function main(): Promise { - const payload = JSON.parse(process.argv[2]); - - if (process.env.HARDHAT_TEST_SUBPROCESS_RESULT_PATH === undefined) { - await postJsonRequest(ANALYTICS_URL, payload, { - queryParams: { - api_secret: API_SECRET, - measurement_id: MEASUREMENT_ID, - }, - }); - - return; - } +const payload = JSON.parse(process.argv[2]); +if (process.env.HARDHAT_TEST_SUBPROCESS_RESULT_PATH === undefined) { + await postJsonRequest(ANALYTICS_URL, payload, { + queryParams: { + api_secret: API_SECRET, + measurement_id: MEASUREMENT_ID, + }, + }); +} else { // ATTENTION: only for testing await writeJsonFile(process.env.HARDHAT_TEST_SUBPROCESS_RESULT_PATH, payload); } - -await main(); diff --git a/v-next/hardhat/src/internal/cli/telemetry/telemetry-permissions.ts b/v-next/hardhat/src/internal/cli/telemetry/telemetry-permissions.ts index 16bba81851..649c704446 100644 --- a/v-next/hardhat/src/internal/cli/telemetry/telemetry-permissions.ts +++ b/v-next/hardhat/src/internal/cli/telemetry/telemetry-permissions.ts @@ -62,7 +62,7 @@ export async function isTelemetryAllowed(): Promise { /** * Determines if telemetry is allowed in the current environment. * This function checks various environmental factors to decide if telemetry data can be collected. - * It verifies that the environment is not a continuous integration (CI) environment, that the terminal is interactive, + * It verifies that the environment is not a CI environment, that the terminal is interactive, * and that telemetry has not been explicitly disabled through an environment variable. * * @returns True if telemetry is allowed in the environment, false otherwise. diff --git a/v-next/hardhat/test/fixture-projects/cli/telemetry/analytics/analytics-subprocess.js b/v-next/hardhat/test/fixture-projects/cli/telemetry/analytics/analytics-subprocess.js deleted file mode 100644 index 6ce089b6fc..0000000000 --- a/v-next/hardhat/test/fixture-projects/cli/telemetry/analytics/analytics-subprocess.js +++ /dev/null @@ -1,20 +0,0 @@ -// This file will be copied to the "analytics" folder so it will be executed as a subprocess. -// This will allow checking that the subprocess received the correct payload. - -// We need to directly use the node method "fs" instead of "writeJsonFile" because a TypeScript ESM file cannot be imported without compiling it first -import { writeFileSync } from "node:fs"; -import path from "node:path"; - -const PATH_TO_RESULT_FILE = path.join( - process.cwd(), - "test", - "fixture-projects", - "cli", - "telemetry", - "analytics", - "result.json", -); - -const stringifiedPayload = process.argv[2]; - -writeFileSync(PATH_TO_RESULT_FILE, stringifiedPayload); From 76c442277950a0079c48d67443133a8dd73c988b Mon Sep 17 00:00:00 2001 From: Christopher Dedominici <18092467+ChristopherDedominici@users.noreply.github.com> Date: Fri, 9 Aug 2024 16:07:04 +0200 Subject: [PATCH 25/27] add debug module --- .../internal/cli/telemetry/analytics/analytics.ts | 7 ++++++- .../src/internal/cli/telemetry/analytics/utils.ts | 14 +++++++++----- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts b/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts index 77ecb25f5c..be77ae5440 100644 --- a/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts +++ b/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts @@ -8,6 +8,7 @@ import type { import os from "node:os"; import { spawnDetachedSubProcess } from "@ignored/hardhat-vnext-utils/subprocess"; +import debug from "debug"; import { getHardhatVersion } from "../../../utils/package.js"; import { @@ -17,7 +18,7 @@ import { import { getAnalyticsClientId } from "./utils.js"; -// TODO:log const log = debug("hardhat:core:global-dir"); +const log = debug("hardhat:cli:telemetry:analytics"); const SESSION_ID = Math.random().toString(); const ENGAGEMENT_TIME_MSEC = "10000"; @@ -79,6 +80,10 @@ async function sendAnalytics( async function createSubprocessToSendAnalytics( payload: TelemetryConsentPayload | Payload, ): Promise { + log( + `Sending analytics for '${payload.events[0].name}'. Payload: ${JSON.stringify(payload)}`, + ); + // The HARDHAT_TEST_SUBPROCESS_RESULT_PATH env variable is used in the tests to instruct the subprocess to write the payload to a file // instead of sending it. // During testing, the subprocess file is a ts file, whereas in production, it is a js file (compiled code). diff --git a/v-next/hardhat/src/internal/cli/telemetry/analytics/utils.ts b/v-next/hardhat/src/internal/cli/telemetry/analytics/utils.ts index dd564d169d..444af3aeeb 100644 --- a/v-next/hardhat/src/internal/cli/telemetry/analytics/utils.ts +++ b/v-next/hardhat/src/internal/cli/telemetry/analytics/utils.ts @@ -8,6 +8,9 @@ import { readJsonFile, writeJsonFile, } from "@ignored/hardhat-vnext-utils/fs"; +import debug from "debug"; + +const log = debug("hardhat:cli:telemetry:analytics:utils"); const ANALYTICS_FILE_NAME = "analytics.json"; @@ -15,9 +18,9 @@ export async function getAnalyticsClientId(): Promise { let clientId = await readAnalyticsClientId(); if (clientId === undefined) { - // TODO:log log("Client Id not found, generating a new one"); - clientId = crypto.randomUUID(); + log("Client Id not found, generating a new one"); + clientId = crypto.randomUUID(); await writeAnalyticsClientId(clientId); } @@ -28,7 +31,7 @@ async function readAnalyticsClientId(): Promise { const globalTelemetryDir = await getTelemetryDir(); const filePath = path.join(globalTelemetryDir, ANALYTICS_FILE_NAME); - // TODO:log log(`Looking up Client Id at ${filePath}`); + log(`Looking up Client Id at '${filePath}'`); if ((await exists(filePath)) === false) { return undefined; @@ -36,7 +39,8 @@ async function readAnalyticsClientId(): Promise { const data: AnalyticsFile = await readJsonFile(filePath); const clientId = data.analytics.clientId; - // TODO:log log(`Client Id found: ${clientId}`); + + log(`Client Id found: ${clientId}`); return clientId; } @@ -50,5 +54,5 @@ async function writeAnalyticsClientId(clientId: string): Promise { }, }); - // TODO:log log(`Stored clientId ${clientId}`); + log(`Stored clientId '${clientId}'`); } From ab5014bb7cb6926fe6641431531b8218de5f476c Mon Sep 17 00:00:00 2001 From: Christopher Dedominici <18092467+ChristopherDedominici@users.noreply.github.com> Date: Fri, 9 Aug 2024 16:36:37 +0200 Subject: [PATCH 26/27] add logs in telemetry-permission file --- .../cli/telemetry/telemetry-permissions.ts | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/v-next/hardhat/src/internal/cli/telemetry/telemetry-permissions.ts b/v-next/hardhat/src/internal/cli/telemetry/telemetry-permissions.ts index 649c704446..ea77cda5db 100644 --- a/v-next/hardhat/src/internal/cli/telemetry/telemetry-permissions.ts +++ b/v-next/hardhat/src/internal/cli/telemetry/telemetry-permissions.ts @@ -7,11 +7,14 @@ import { readJsonFile, writeJsonFile, } from "@ignored/hardhat-vnext-utils/fs"; +import debug from "debug"; import { confirmationPromptWithTimeout } from "../prompt/prompt.js"; import { sendTelemetryConsentAnalytics } from "./analytics/analytics.js"; +const log = debug("hardhat:cli:telemetry:telemetry-permissions"); + interface TelemetryConsent { consent: boolean; } @@ -24,6 +27,8 @@ interface TelemetryConsent { * @returns True if the user consents to telemetry and if current environment supports telemetry, false otherwise. */ export async function ensureTelemetryConsent(): Promise { + log("Ensuring that user has provided telemetry consent"); + if (!isTelemetryAllowedInEnvironment()) { return false; } @@ -55,6 +60,7 @@ export async function isTelemetryAllowed(): Promise { } const consent = await getTelemetryConsent(); + log(`Telemetry consent value: ${consent}`); return consent !== undefined ? consent : false; } @@ -68,13 +74,16 @@ export async function isTelemetryAllowed(): Promise { * @returns True if telemetry is allowed in the environment, false otherwise. */ export function isTelemetryAllowedInEnvironment(): boolean { - return ( + const allowed = (!isCi() && process.stdout.isTTY === true && process.env.HARDHAT_DISABLE_TELEMETRY_PROMPT !== "true") || // ATTENTION: used in tests to force telemetry execution - process.env.HARDHAT_TEST_INTERACTIVE_ENV === "true" - ); + process.env.HARDHAT_TEST_INTERACTIVE_ENV === "true"; + + log(`Telemetry is allowed in the current environment: ${allowed}`); + + return allowed; } /** @@ -108,6 +117,7 @@ async function requestTelemetryConsent(): Promise { } // Store user's consent choice + log(`Storing telemetry consent with value: ${consent}`); await writeJsonFile(await getTelemetryConsentFilePath(), { consent }); await sendTelemetryConsentAnalytics(consent); @@ -116,6 +126,8 @@ async function requestTelemetryConsent(): Promise { } async function confirmTelemetryConsent(): Promise { + log("Prompting user for telemetry consent"); + return confirmationPromptWithTimeout( "telemetryConsent", "Help us improve Hardhat with anonymous crash reports & basic usage data?", From e9ecaccf0fd3c5cab34446e4ad4f78cbaf1f42e1 Mon Sep 17 00:00:00 2001 From: Christopher Dedominici <18092467+ChristopherDedominici@users.noreply.github.com> Date: Fri, 9 Aug 2024 16:41:04 +0200 Subject: [PATCH 27/27] additional debug message --- .../hardhat/src/internal/cli/telemetry/analytics/analytics.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts b/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts index be77ae5440..e55ddb090d 100644 --- a/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts +++ b/v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts @@ -100,6 +100,8 @@ async function createSubprocessToSendAnalytics( } await spawnDetachedSubProcess(subprocessFile, [JSON.stringify(payload)], env); + + log("Payload sent to detached subprocess"); } async function buildPayload(