optional telemetry

[DavidGOrtega]

Add optional telemetry to CML to help prioritise features/issues based on usage.

• opens #1093
• Updated to JSON spec
• slightly fixes #1093

[0x2b3bfa0]

Premature review: consider talking the deterministic function from this Notion page.

const { promisify } = require('util');
const { scrypt } = require('crypto');
const { v4: uuidv4, v5: uuidv5, parse } = require('uuid');

async function deterministic(data) {
  const namespace = uuidv5('iterative.ai', uuidv5.DNS);
  const name = await promisify(scrypt)(data, parse(namespace), 8, {
    N: 1 << 16,
    r: 8,
    p: 1,
    maxmem: 128 * 1024 ** 2,
  });
  return uuidv5(name.toString('hex'), namespace);
}

function random() {
  return uuidv4();
}

deterministic('example').then((uuid) => {
  console.log(uuid);
  console.log(random());
});

Marking as outdated (resolved) after https://github.com/iterative/cml/pull/1092/commits/ec6b5e36491f6f1979ec623c530915c6b5159c0c

[DavidGOrtega]

Read desc ☝️

[DavidGOrtega]

#1094

Nice! Its a good start. I was thinking the same but in another PR because I want to refactor also more things. I think the cml.js and subcommands are not very DRY either.

Also it might delay the PR due to testing... but I like the effort... damn... why you just not did wait to the refactor? 😁

[DavidGOrtega]

Unlike in the Go implementation, there is no code to prevent the program from exiting while telemetry data is still being sent.

Adding await waitForever() to the first line of the telemetry.send function doesn't prevent the program from exiting. 🤔 Did I overlook something?

I do not follow you @0x2b3bfa0 could you please elaborate? where is the waitForever?

[DavidGOrtega]

The PR is slow down due to the process.exit and telemetry hook. It needs a bit of thought 😞

[DavidGOrtega]

Im dismissing @0x2b3bfa0 because I have already his latest golantical suggestions.

[DavidGOrtega]

@dacbd I have updated the code can you please review one more time?

[casperdcl]

(self-requesting review for verifying spec)

[0x2b3bfa0]

Still trying to figure out how to avoid injecting telemetry-specific code in every command. What if we wrap the entry point on bin/cml.js with some kind of “success” logic?

https://github.com/iterative/cml/blob/6271cb8847bc98b8bf14bc30df5513959896d01f/bin/cml.js#L96-L115

E.g. yargs...fail(handleError).parse() can be replaced with handleSuccess(yargs...fail(handleError).parse()) and, provided that handleError contains a process.exit they'll be mutually exclusive.

Update: see https://github.com/iterative/cml/pull/1124#issue-1333079853 for the proposed changes.

[DavidGOrtega]

lgtm

But not approved 🙈