Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhance telemetry data and exclude tensorboard tracebacks #1172

Merged
merged 24 commits into from
Oct 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,8 @@ WORKDIR ${CML_RUNNER_PATH}
# SET SPECIFIC ENVIRONMENT VARIABLES
ENV IN_DOCKER=1
ENV RUNNER_ALLOW_RUNASROOT=1
# Environment variable used by cml to detect it's been installed using the docker image.
ENV _CML_CONTAINER_IMAGE=true
0x2b3bfa0 marked this conversation as resolved.
Show resolved Hide resolved
0x2b3bfa0 marked this conversation as resolved.
Show resolved Hide resolved

# DEFINE ENTRY POINT AND COMMAND
# Smart entrypoint understands commands like `bash` or `/bin/sh` but defaults to `cml`;
Expand Down
28 changes: 24 additions & 4 deletions bin/cml.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ const setupOpts = (opts) => {

const { markdownfile } = opts;
opts.markdownFile = markdownfile;
opts.cmlCommand = opts._[0];
opts.cml = new CML(opts);
};

Expand Down Expand Up @@ -76,9 +75,29 @@ const setupLogger = (opts) => {
});
};

const setupTelemetry = async (opts) => {
const { cml, cmlCommand: action } = opts;
opts.telemetryEvent = await jitsuEventPayload({ action, cml });
const setupTelemetry = async (opts, yargs) => {
const { cml, _: command } = opts;

const options = {};
for (const [name, option] of Object.entries(opts.options)) {
// Skip options with default values (i.e. not explicitly set by users)
if (opts[name] && !yargs.parsed.defaulted[name]) {
switch (option.telemetryData) {
case 'name':
options[name] = null;
break;
case 'full':
options[name] = opts[name];
break;
}
}
}

opts.telemetryEvent = await jitsuEventPayload({
action: command.join(':'),
extra: { options },
cml
});
};

const runPlugin = async ({ $0: executable, command }) => {
Expand All @@ -99,6 +118,7 @@ const handleError = (message, error) => {

(async () => {
setupLogger({ log: 'debug' });

try {
await yargs
.env('CML')
Expand Down
9 changes: 7 additions & 2 deletions bin/cml/asset/publish.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@ exports.handler = async (opts) => {
else await fs.writeFile(file, output);
};

exports.builder = (yargs) => yargs.env('CML_ASSET').options(exports.options);
exports.builder = (yargs) =>
yargs
.env('CML_ASSET')
.option('options', { default: exports.options, hidden: true })
.options(exports.options);

exports.options = kebabcaseKeys({
url: {
Expand Down Expand Up @@ -51,7 +55,8 @@ exports.options = kebabcaseKeys({
},
rmWatermark: {
type: 'boolean',
description: 'Avoid CML watermark.'
description: 'Avoid CML watermark.',
telemetryData: 'name'
},
mimeType: {
type: 'string',
Expand Down
6 changes: 5 additions & 1 deletion bin/cml/check/create.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@ exports.handler = async (opts) => {
await cml.checkCreate({ ...opts, report });
};

exports.builder = (yargs) => yargs.env('CML_CHECK').options(exports.options);
exports.builder = (yargs) =>
yargs
.env('CML_CHECK')
.option('options', { default: exports.options, hidden: true })
0x2b3bfa0 marked this conversation as resolved.
Show resolved Hide resolved
.options(exports.options);

exports.options = kebabcaseKeys({
token: {
Expand Down
15 changes: 11 additions & 4 deletions bin/cml/comment/create.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@ exports.handler = async (opts) => {
console.log(await cml.commentCreate(opts));
};

exports.builder = (yargs) => yargs.env('CML_COMMENT').options(exports.options);
exports.builder = (yargs) =>
yargs
.env('CML_COMMENT')
.option('options', { default: exports.options, hidden: true })
.options(exports.options);

exports.options = kebabcaseKeys({
pr: {
Expand All @@ -30,7 +34,8 @@ exports.options = kebabcaseKeys({
publishUrl: {
type: 'string',
default: 'https://asset.cml.dev',
description: 'Self-hosted image server URL'
description: 'Self-hosted image server URL',
telemetryData: 'name'
},
watch: {
type: 'boolean',
Expand All @@ -44,7 +49,8 @@ exports.options = kebabcaseKeys({
native: {
type: 'boolean',
description:
"Uses driver's native capabilities to upload assets instead of CML's storage; not available on GitHub"
"Uses driver's native capabilities to upload assets instead of CML's storage; not available on GitHub",
telemetryData: 'name'
},
update: {
type: 'boolean',
Expand All @@ -55,6 +61,7 @@ exports.options = kebabcaseKeys({
rmWatermark: {
type: 'boolean',
description:
'Avoid watermark; CML needs a watermark to be able to distinguish CML comments from others'
'Avoid watermark; CML needs a watermark to be able to distinguish CML comments from others',
telemetryData: 'name'
}
});
1 change: 1 addition & 0 deletions bin/cml/pr.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,6 @@ exports.builder = (yargs) =>
])
)
)
.option('options', { default: options, hidden: true })
.check(({ globpath }) => globpath)
.strict();
6 changes: 5 additions & 1 deletion bin/cml/pr/create.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@ exports.handler = async (opts) => {
console.log(link);
};

exports.builder = (yargs) => yargs.env('CML_PR').options(exports.options);
exports.builder = (yargs) =>
yargs
.env('CML_PR')
.option('options', { default: exports.options, hidden: true })
.options(exports.options);

exports.options = kebabcaseKeys({
md: {
Expand Down
6 changes: 5 additions & 1 deletion bin/cml/repo/prepare.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@ exports.handler = async (opts) => {
await cml.ci(opts);
};

exports.builder = (yargs) => yargs.env('CML_REPO').options(exports.options);
exports.builder = (yargs) =>
yargs
.env('CML_REPO')
.option('options', { default: exports.options, hidden: true })
.options(exports.options);

exports.options = kebabcaseKeys({
unshallow: {
Expand Down
1 change: 1 addition & 0 deletions bin/cml/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,6 @@ exports.builder = (yargs) =>
])
)
)
.option('options', { default: options, hidden: true })
.check(() => process.argv.some((arg) => arg.startsWith('-')))
.strict();
21 changes: 15 additions & 6 deletions bin/cml/runner/launch.js
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,11 @@ exports.handler = async (opts) => {
}
};

exports.builder = (yargs) => yargs.env('CML_RUNNER').options(exports.options);
exports.builder = (yargs) =>
yargs
.env('CML_RUNNER')
.option('options', { default: exports.options, hidden: true })
.options(exports.options);

exports.options = kebabcaseKeys({
labels: {
Expand Down Expand Up @@ -454,13 +458,15 @@ exports.options = kebabcaseKeys({
type: 'boolean',
conflicts: ['single', 'reuseIdle'],
description:
"Don't launch a new runner if an existing one has the same name or overlapping labels"
"Don't launch a new runner if an existing one has the same name or overlapping labels",
telemetryData: 'name'
},
reuseIdle: {
type: 'boolean',
conflicts: ['reuse', 'single'],
description:
"Creates a new runner only if the matching labels don't exist or are already busy"
"Creates a new runner only if the matching labels don't exist or are already busy",
telemetryData: 'name'
},
workdir: {
type: 'string',
Expand All @@ -476,7 +482,8 @@ exports.options = kebabcaseKeys({
cloud: {
type: 'string',
choices: ['aws', 'azure', 'gcp', 'kubernetes'],
description: 'Cloud to deploy the runner'
description: 'Cloud to deploy the runner',
telemetryData: 'full'
},
cloudRegion: {
type: 'string',
Expand Down Expand Up @@ -529,12 +536,14 @@ exports.options = kebabcaseKeys({
type: 'number',
default: -1,
description:
'Maximum spot instance bidding price in USD. Defaults to the current spot bidding price'
'Maximum spot instance bidding price in USD. Defaults to the current spot bidding price',
telemetryData: 'name'
},
cloudStartupScript: {
type: 'string',
description:
'Run the provided Base64-encoded Linux shell script during the instance initialization'
'Run the provided Base64-encoded Linux shell script during the instance initialization',
telemetryData: 'name'
},
cloudAwsSecurityGroup: {
type: 'string',
Expand Down
2 changes: 1 addition & 1 deletion bin/cml/tensorboard/connect.e2e.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe('tbLink', () => {
error = err;
}

expect(error.message).toBe(`Tensorboard took too long. ${message}`);
expect(error.message).toBe(`Tensorboard took too long`);
});

test('valid url is returned', async () => {
Expand Down
17 changes: 11 additions & 6 deletions bin/cml/tensorboard/connect.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ const kebabcaseKeys = require('kebabcase-keys');
const { spawn } = require('child_process');
const { homedir } = require('os');
const tempy = require('tempy');
const winston = require('winston');

const { exec, watermarkUri, sleep } = require('../../../src/utils');

Expand All @@ -28,8 +29,8 @@ const tbLink = async (opts = {}) => {
chrono = chrono + chronoStep;
}

const error = await fs.readFile(stderror, 'utf8');
throw new Error(`Tensorboard took too long. ${error}`);
winston.error(await fs.readFile(stderror, 'utf8'));
0x2b3bfa0 marked this conversation as resolved.
Show resolved Hide resolved
throw new Error(`Tensorboard took too long`);
};

const launchAndWaitLink = async (opts = {}) => {
Expand All @@ -51,8 +52,8 @@ const launchAndWaitLink = async (opts = {}) => {
proc.unref();
proc.on('exit', async (code, signal) => {
if (code || signal) {
const error = await fs.readFile(stderrPath, 'utf8');
throw new Error(`Tensorboard failed with error: ${error}`);
winston.error(await fs.readFile(stderrPath, 'utf8'));
throw new Error(`Tensorboard failed with error ${code || signal}`);
}
});

Expand Down Expand Up @@ -95,7 +96,10 @@ exports.handler = async (opts) => {
};

exports.builder = (yargs) =>
yargs.env('CML_TENSORBOARD').options(exports.options);
yargs
.env('CML_TENSORBOARD')
.option('options', { default: exports.options, hidden: true })
.options(exports.options);

exports.options = kebabcaseKeys({
credentials: {
Expand Down Expand Up @@ -136,6 +140,7 @@ exports.options = kebabcaseKeys({
},
rmWatermark: {
type: 'boolean',
description: 'Avoid CML watermark'
description: 'Avoid CML watermark',
telemetryData: 'name'
}
});
6 changes: 5 additions & 1 deletion bin/cml/workflow/rerun.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@ exports.handler = async (opts) => {
await cml.pipelineRerun(opts);
};

exports.builder = (yargs) => yargs.env('CML_WORKFLOW').options(exports.options);
exports.builder = (yargs) =>
yargs
.env('CML_WORKFLOW')
.option('options', { default: exports.options, hidden: true })
.options(exports.options);

exports.options = kebabcaseKeys({
id: {
Expand Down
20 changes: 20 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@
"git-url-parse": "^13.1.0",
"globby": "^11.0.4",
"https-proxy-agent": "^5.0.1",
"is-docker": "2.2.1",
"js-base64": "^3.7.2",
"kebabcase-keys": "^1.0.0",
"node-fetch": "^2.6.5",
Expand Down
5 changes: 3 additions & 2 deletions src/analytics.e2e.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ describe('analytics tests', () => {
const cml = new CML({ repo: REPO, token: TOKEN });
const action = 'test';
const cloud = 'azure';
const container = 'cml';
const more = { one: 1, two: 2 };
const extra = { cloud, ...more };
const extra = { cloud, container, ...more };
const error = 'Ouch!';
const regex = /\d+\.\d+\.\d+/;

Expand All @@ -25,7 +26,7 @@ describe('analytics tests', () => {
expect(pl.backend).toBe(cloud);
expect(pl.error).toBe(error);
expect(Object.keys(pl.extra).sort()).toEqual(
['ci'].concat(Object.keys(more)).sort()
['ci', 'container'].concat(Object.keys(more)).sort()
);

if (isCI()) {
Expand Down
Loading