Add logic to send analytics data #5545

ChristopherDedominici · 2024-07-23T15:51:47Z

How it works in short:

so far there are 2 types of analytics that can be sent (like in the old hardhat).

the response to the telemetry consent
information about the executed tasks

The first one is a very specific type of analytics, so it cannot be extended or modified.
The second one is more general so other types of analytics' payloads will have the same structure.

The creation of the payload is done in the main process, then a detached subprocess is spawn to perform the http request.

In the tests I created a fake subprocess file that tests that the correct payload is received by the subprocess.
I also did a bit of a "hacky" thing to run the tests in the CI:
the analytics have a check that avoids the execution in CI environments. The check is performed on several env variables. So before the test I overwrite them, and then I reset them after the test.

TODO

There are debug logging functions that are commented because the debug log PR is not merged yet
I need to create an analytics test environment to check that the information are correctly sent

vercel · 2024-07-23T15:51:51Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
hardhat	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Aug 15, 2024 0:42am

changeset-bot · 2024-07-23T15:51:51Z

⚠️ No Changeset found

Latest commit: 082b407

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

…ure/add-logic-to-send-telemetry-data

schaable

Looks good, I left a couple comments

v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts

v-next/hardhat/src/internal/cli/telemetry/analytics/utils.ts

v-next/hardhat/test/internal/cli/telemetry/analytics/analytics.ts

alcuadrado · 2024-07-28T18:18:59Z

v-next/core/src/global-dir.ts

+ *
+ * @returns A promise that resolves to the path of the telemetry directory.
+ */
+export async function getTelemetryDir(packageName?: string): Promise<string> {


Can we make sure that this dir is not deleted by the clean --global task? /cc @schaable

I don't think so, as the global flag deletes the cache path and telemetry uses the data path.

alcuadrado · 2024-07-28T18:20:35Z

v-next/hardhat/src/internal/cli/telemetry/analytics/analytics-subprocess.ts

+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";


Are these the test or the production values?

Production values

I'll updated the code with test variables. Before merging the code, I'll replace them with production values.

DO NOT REMOVE THIS COMMENT, it works as a reminder

v-next/hardhat/package.json

v-next/hardhat/src/internal/cli/telemetry/analytics/analytics-subprocess.ts

alcuadrado · 2024-07-28T18:29:21Z

v-next/hardhat/src/internal/cli/telemetry/analytics/utils.ts

+}
+
+export function getUserType(): string {
+  return isCi() ? "CI" : "Developer";


We don't send analytics hits from CIs anymore, so we should remove the userType

alcuadrado · 2024-07-28T18:30:57Z

v-next/hardhat/test/fixture-projects/cli/telemetry/analytics/analytics-subprocess.js

+  "result.json",
+);
+
+(() => {


This lambda is not needed.

alcuadrado · 2024-08-05T19:08:00Z

v-next/hardhat/src/internal/cli/telemetry/analytics/analytics.ts

+async function createSubprocessToSendAnalytics(
+  payload: TelemetryConsentPayload | Payload,
+): Promise<void> {
+  const fileExt =


Let's document this here with a comment because we'll forget it

Done here, with other smaller fixes

alcuadrado · 2024-08-05T19:10:04Z

v-next/hardhat/src/internal/cli/telemetry/analytics/subprocess.ts

+  await writeJsonFile(process.env.HARDHAT_TEST_SUBPROCESS_RESULT_PATH, payload);
+}
+
+await main();


We have top-level-await now

Done here, with other smaller fixes

alcuadrado · 2024-08-05T19:14:00Z

v-next/hardhat/test/fixture-projects/cli/telemetry/analytics/analytics-subprocess.js

@@ -0,0 +1,20 @@
+// This file will be copied to the "analytics" folder so it will be executed as a subprocess.


Not used anymore

Done here, with other smaller fixes

alcuadrado · 2024-08-05T19:27:57Z

There are just a few small comments regarding the PR, but we need to actually validate that this is working as intended in analytics, and in particular, we should validate that the custom dimensions work. Note that we are using a new analytics property for testing, so no setup has been done there.

…ure/add-logic-to-send-telemetry-data

kanej · 2024-08-16T16:08:19Z

Both Chris and I have separately done a manual test that events are coming through with the expected custom dimensions. This PR is pointing at a test Google Analytics environment.

ChristopherDedominici added 5 commits July 22, 2024 11:57

Modify telemetry functions

34a361d

add uuid npm package

24b35bb

add a method in hh-core to get the telemetry directory

4407167

add main logic (still a few minors TODOs)

40b00f5

add tests

a664c3b

github-actions bot assigned kanej Jul 23, 2024

github-actions bot added the status:triaging label Jul 23, 2024

vercel bot deployed to Preview July 23, 2024 15:52 View deployment

ChristopherDedominici linked an issue Jul 23, 2024 that may be closed by this pull request

Port or reimplement and test the analytics integration #5450

Closed

ChristopherDedominici added status:ready This issue is ready to be worked on and removed status:triaging labels Jul 23, 2024

ChristopherDedominici assigned ChristopherDedominici and unassigned kanej Jul 23, 2024

ChristopherDedominici added 2 commits July 24, 2024 11:57

small code reorganization

648592c

Merge branch 'v-next' of github.com:NomicFoundation/hardhat into feat…

e5d5d4a

…ure/add-logic-to-send-telemetry-data

vercel bot deployed to Preview July 24, 2024 09:58 View deployment

ChristopherDedominici requested review from alcuadrado and schaable July 24, 2024 10:01

ChristopherDedominici marked this pull request as ready for review July 24, 2024 10:04

schaable reviewed Jul 24, 2024

View reviewed changes

Small PR comments' fixes

94a3abe

vercel bot deployed to Preview July 24, 2024 16:41 View deployment

alcuadrado reviewed Jul 28, 2024

View reviewed changes

v-next/hardhat/package.json Outdated Show resolved Hide resolved

alcuadrado reviewed Jul 28, 2024

View reviewed changes

v-next/hardhat/src/internal/cli/telemetry/analytics/analytics-subprocess.ts Outdated Show resolved Hide resolved

alcuadrado reviewed Jul 28, 2024

View reviewed changes

vercel bot deployed to Preview August 5, 2024 11:26 View deployment

modify helper: wait for subprocess file to be readable

3e59fa5

ChristopherDedominici force-pushed the feature/add-logic-to-send-telemetry-data branch from f556dd2 to 3e59fa5 Compare August 5, 2024 13:06

vercel bot deployed to Preview August 5, 2024 13:07 View deployment

add logic to wait for the file to be readable

39a1511

vercel bot deployed to Preview August 5, 2024 13:47 View deployment

alcuadrado reviewed Aug 5, 2024

View reviewed changes

Smaller fixes based on the PR's comments

b9df707

vercel bot deployed to Preview August 6, 2024 09:49 View deployment

ChristopherDedominici added 2 commits August 9, 2024 15:52

Merge branch 'v-next' of github.com:NomicFoundation/hardhat into feat…

093ae33

…ure/add-logic-to-send-telemetry-data

add debug module

76c4422

vercel bot deployed to Preview August 9, 2024 14:08 View deployment

add logs in telemetry-permission file

ab5014b

vercel bot deployed to Preview August 9, 2024 14:38 View deployment

additional debug message

e9ecacc

vercel bot deployed to Preview August 9, 2024 14:42 View deployment

schaable added the v-next A Hardhat v3 development task label Aug 9, 2024

Merge branch 'v-next' of github.com:NomicFoundation/hardhat into feat…

082b407

…ure/add-logic-to-send-telemetry-data

vercel bot deployed to Preview August 15, 2024 12:42 View deployment

alcuadrado approved these changes Aug 19, 2024

View reviewed changes

ChristopherDedominici added this pull request to the merge queue Aug 20, 2024

Merged via the queue into v-next with commit 2d4f8db Aug 20, 2024
22 checks passed

ChristopherDedominici deleted the feature/add-logic-to-send-telemetry-data branch August 20, 2024 12:16

ChristopherDedominici mentioned this pull request Aug 20, 2024

Use the telemetry logic to report if the telemetry consent was given or not #5502

Closed

github-actions bot locked as resolved and limited conversation to collaborators Nov 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add logic to send analytics data #5545

Add logic to send analytics data #5545

ChristopherDedominici commented Jul 23, 2024 •

edited by kanej

Loading

vercel bot commented Jul 23, 2024 •

edited

Loading

changeset-bot bot commented Jul 23, 2024 •

edited

Loading

schaable left a comment

alcuadrado Jul 28, 2024

schaable Jul 29, 2024

alcuadrado Jul 28, 2024

ChristopherDedominici Jul 29, 2024

ChristopherDedominici Jul 29, 2024 •

edited

Loading

alcuadrado Jul 28, 2024

ChristopherDedominici Jul 29, 2024

alcuadrado Jul 28, 2024

ChristopherDedominici Jul 29, 2024

alcuadrado Aug 5, 2024

ChristopherDedominici Aug 6, 2024

alcuadrado Aug 5, 2024

ChristopherDedominici Aug 6, 2024

alcuadrado Aug 5, 2024

ChristopherDedominici Aug 6, 2024

alcuadrado commented Aug 5, 2024

kanej commented Aug 16, 2024

		@@ -0,0 +1,20 @@
		// This file will be copied to the "analytics" folder so it will be executed as a subprocess.

Add logic to send analytics data #5545

Add logic to send analytics data #5545

Conversation

ChristopherDedominici commented Jul 23, 2024 • edited by kanej Loading

TODO

vercel bot commented Jul 23, 2024 • edited Loading

changeset-bot bot commented Jul 23, 2024 • edited Loading

⚠️ No Changeset found

schaable left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChristopherDedominici Jul 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alcuadrado commented Aug 5, 2024

kanej commented Aug 16, 2024

ChristopherDedominici commented Jul 23, 2024 •

edited by kanej

Loading

vercel bot commented Jul 23, 2024 •

edited

Loading

changeset-bot bot commented Jul 23, 2024 •

edited

Loading

ChristopherDedominici Jul 29, 2024 •

edited

Loading