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

Add ability to retrieve and tail app function logs #490

Merged
merged 9 commits into from
May 7, 2021

Conversation

gcorne
Copy link
Contributor

@gcorne gcorne commented Apr 30, 2021

Description and Context

This PR adds support for accessing serverless app function logs through expanding the hs logs command. The new options will remain undocumented for now.

An example of the format is hs logs --appFunction=weatherCard --app=/hello-world-app

TODO

  • See if yargs supports having option work as both a positional and option. If so, add --endpoint so that there is symmetry
  • Add some unit test coverage

Who to Notify

@miketalley


const accountId = getAccountId(options);

trackCommandUsage('logs', { latest }, accountId);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

One thing that I need to figure out is how we want to track the two, which may require a backend change to support additional event attributes.

Copy link
Contributor

Choose a reason for hiding this comment

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

What about adding appPath and functionName to the tracking object along with latest?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What about adding appPath and functionName to the tracking object along with latest?

I think the backend needs to support any event attributes that we add.

spinner,
tailCall,
fetchLatest,
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

One thing that I am not sure about is whether the indirection and passing in of functions is easy enough to follow.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's clear from the function name.

accountId,
accountName: options.portal,
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we not need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we do want to show the account name instead of the id, we do need to incorporate something like this in the new code. That said, I think the way to handle that is to get the account config using the id so that it works consistently even in cases where the defaultPortal is used. We also should be consistent across all commands in terms of how we refer to an account in the output.

spinner,
tailCall,
fetchLatest,
});
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's clear from the function name.


const accountId = getAccountId(options);

trackCommandUsage('logs', { latest }, accountId);
Copy link
Contributor

Choose a reason for hiding this comment

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

What about adding appPath and functionName to the tracking object along with latest?

Comment on lines 1 to 71
const readline = require('readline');

const { outputLogs } = require('@hubspot/cli-lib/lib/logs');
const {
logServerlessFunctionApiErrorInstance,
ApiErrorContext,
} = require('@hubspot/cli-lib/errorHandlers');
const { base64EncodeString } = require('@hubspot/cli-lib/lib/encoding');

const TAIL_DELAY = 5000;

const handleKeypressToExit = exit => {
readline.emitKeypressEvents(process.stdin);
process.stdin.setRawMode(true);
process.stdin.on('keypress', (str, key) => {
if (key && ((key.ctrl && key.name == 'c') || key.name === 'escape')) {
exit();
}
});
};

const tailLogs = async ({
accountId,
compact,
spinner,
fetchLatest,
tailCall,
}) => {
let initialAfter;

spinner.start();

try {
const latestLog = await fetchLatest();
initialAfter = base64EncodeString(latestLog.id);
} catch (e) {
// A 404 means no latest log exists(never executed)
if (e.statusCode !== 404) {
await logServerlessFunctionApiErrorInstance(
accountId,
e,
new ApiErrorContext({ accountId })
);
}
}

const tail = async after => {
const latestLog = await tailCall(after);

if (latestLog.results.length) {
spinner.clear();
outputLogs(latestLog, {
compact,
});
}

setTimeout(() => {
tail(latestLog.paging.next.after);
}, TAIL_DELAY);
};

handleKeypressToExit(() => {
spinner.stop();
process.exit();
});
tail(initialAfter);
};

module.exports = {
tailLogs,
};
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@gcorne
Copy link
Contributor Author

gcorne commented May 7, 2021

@miketalley I made an adjustment to fix situations where a function or endpoint hasn't been executed so there weren't any logs. This also improves situations where a fetching the latest log fails due to some intermittent error.

@@ -8,9 +9,16 @@ const full = require('./fixtures/schema/full.json');
const multiple = require('./fixtures/schema/multiple.json');

describe('cli-lib/schema', () => {
const originalChalkLevel = chalk.level;
beforeEach(() => {
chalk.level = 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@drewjenkins due to an upgrade of chalk, you're approach of monkey patching process.env.FORCE_COLOR stopped working. I switched to a more targeted approach using https://github.com/chalk/chalk#chalklevel.

@gcorne
Copy link
Contributor Author

gcorne commented May 7, 2021

I think that I am going to leave changes to how endpoints work for later. I also will work on adding usage tracking, which will require a backend change first.

@gcorne gcorne merged commit 93d2d15 into master May 7, 2021
@gcorne gcorne deleted the add/app-function-logs branch May 7, 2021 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants