pwa-kit-dev command for tailing logs #789

raiyaj · 2022-10-20T20:23:06Z

Adds the ability to tail logs for deployed Managed Runtime environments!

Description

Types of Changes

Bug fix (non-breaking change that fixes an issue)
New feature (non-breaking change that adds functionality)
Documentation update
Breaking change (could cause existing functionality to not work as expected)
Other changes (non-breaking changes that does not fit any of the above)

Breaking changes include:

Removing a public function or component or prop

Adding a required argument to a function

Changing the data type of a function parameter or return value

Adding a new peer dependency to package.json

Changes

Incorporate Oli's managedRuntimeCommand helper into the pwa-kit-dev CLI to make sharing common options like cloudOrigin and credentialsFile easier, and use it for push and save-credentials
Add a tail-logs command
Add a tail-logs npm script to each template's package.json
Add/update tests
Upgrade commander's major version

How to Test-Drive This PR

Ensure your production API key is in ~/.mobify
Checkout this branch
If you haven't done so already, install lerna, then run npm ci at the root of the repo
Tail logs! Run node packages/pwa-kit-dev/bin/pwa-kit-dev.js tail-logs -p <slug> -e <slug> for an environment in production. Once you generate some traffic on the environment, you should see something like this:
Other things to try:
- Make sure the command's help text includes the project, environment, cloud origin, and credentials file options
- Start tailing, but don't open the environment URL until > 10 minutes later (the idle timeout). The connection should remain open and stream the logs once you open it.

Checklists

General

Changes are covered by test cases
CHANGELOG.md updated with a short description of changes (not required for documentation updates)

Accessibility Compliance

You must check off all items in one of the follow two lists:

There are no changes to UI

or...

Changes were tested with a Screen Reader (iOS VoiceOver or Android Talkback) and had no issues
Changes comply with WCAG 2.0 guidelines levels A and AA
Changes to common UI patterns and interactions comply with WAI-ARIA best practices

Localization

Changes include a UI text update in the Retail React App (which requires translation)

salesforce-cla · 2022-10-20T20:23:11Z

Thanks for the contribution! Unfortunately we can't verify the commit author(s): Mahdi Soleimani <m***@s***.com>. One possible solution is to add that email to your GitHub account. Alternatively you can change your commits to another email and force push the change. After getting your commits associated with your GitHub account, refresh the status of this Pull Request.

olibrook · 2022-10-20T23:15:30Z

packages/pwa-kit-dev/bin/pwa-kit-dev.js

+            }
+        })
+        .action(async (_, opts) => {
+            const {project, environment, cloudApiBase, credentialsFile} = opts.optsWithGlobals()


Really nice – I like that optsWithGlobals makes the order of the args insignificant. Couldn't work out how to do this myself the other day.

There are a set of commands in here for which these "global" flags can never be of interest – basically anything that is not an "MRT Command" (build, lint, etc).

I think that's harmless, but I came up with this mrtCommand helper here that might be worth thinking about.

The only advantage I can think of with the mrtCommand approach is that the --cloudURLBase still appear in the command-specific help. Here's what we see with your approach now:

(venv) ➜ pwa-kit-dev git:(tail-logs) ./bin/pwa-kit-dev.js logs --help Usage: pwa-kit-dev logs [options] tail environment logs Options: -p, --project <project_slug> the project slug -e, --environment <environment_slug> the environment slug -h, --help display help for command

It's a nice-to-have.

Interesting, I like the mrtCommand idea, I'll take a shot at refactoring the commands to use it (though you've done most of that in your PR :)). I think your point about the global options not appearing for command-specific help is important, cause I also made credentialsFile global, which means I've taken it out of the push and save-credentials help. And the wrapper seems similar to what other people are doing to get around that problem.

I also like the fancy color-coded logs you have there! I wanna give that a try too :)

olibrook · 2022-10-20T23:16:56Z

packages/pwa-kit-dev/bin/pwa-kit-dev.js

+            }
+
+            const token = await scriptUtils.createToken(project, environment, cloudApiBase, credentials.api_key)
+            const url = new URL(cloudApiBase.replace('cloud', 'logs'))


Probs want to validate that URL actually looks "cloud-ish" if you're going to replace like that!

I'll add validation to the option definition itself 😀

packages/pwa-kit-dev/bin/pwa-kit-dev.js

olibrook · 2022-10-20T23:22:30Z

@raiyaj I think what you have here looks excellent. I think there's an open question about --tail.

If we do --tail now, my question is do we have a plan to do anything other than --tail in the CLI in future? What would an implementation of pwa-kit-dev logs be now, without the --tail option?

I think y'all were discussing showing the --help text. I mean, fine, if we have other plans for a --tail-free version of the command. If we don't have plans, I can't help thinking that it's just 6 extra characters to type every single time, forever!

What do other tools do? I feel like heroku logs without --tail used to show you one "page" of logs, no?

raiyaj · 2022-10-21T00:06:41Z

my question is do we have a plan to do anything other than --tail in the CLI in future?

@olibrook Way back we talked about adding the ability to query past logs using the CLI, but since we have no plans to implement it anytime soon, I agree that the --tail option is kinda redundant since that's the only functionality we're offering.

What do other tools do? I feel like heroku logs without --tail used to show you one "page" of logs, no?

I checked heroku logs and aws logs tail, and you're right - both display recent logs and only poll for new logs with an extra --tail or --follow arg.

@johnboxall 's point about --tail generally being a long-running program that exits on a signal seems valid too. For now I'm going to add the --tail arg and show help without it, but will get team feedback before merging.

salesforce-cla · 2022-10-25T20:53:11Z

Thanks for the contribution! It looks like @kenjush is an internal user so signing the CLA is not required. However, we need to confirm this.

raiyaj · 2022-11-01T18:55:26Z

packages/pwa-kit-dev/package.json

@@ -61,7 +61,7 @@
    "babel-plugin-dynamic-import-node-babel-7": "^2.0.7",
    "babel-plugin-formatjs": "10.2.20",
    "chalk": "^4.1.2",
-    "commander": "^8.3.0",
+    "commander": "^9.3.0",


I kept this library upgrade from the initial spike. I noticed PWA Kit team's 3PP approval for this lib is only for v2.20.3 (?!), so I'll look into requesting approval for v9.

Here's my approval request: https://gus.lightning.force.com/lightning/r/ADM_Third_Party_Software__c/a0qEE0000005Si1YAE/view

raiyaj · 2022-11-01T18:56:15Z

packages/pwa-kit-dev/package-lock.json

@@ -6,7 +6,7 @@
 	"packages": {
 		"": {
 			"name": "pwa-kit-dev",
-			"version": "2.3.0-dev",
+			"version": "2.4.0-dev",


I'm guessing this is leftover from the recent 2.3.0-dev release.

raiyaj · 2022-11-01T19:02:29Z

packages/pwa-kit-dev/bin/pwa-kit-dev.js

@@ -184,7 +224,7 @@ const main = () => {
                'a message to include along with the uploaded bundle in Managed Runtime'
            )
                // The default message is loaded dynamically as part of `uploadBundle(...)`
-                .default(undefined, '<git branch>:<git commit hash>')
+                .default(null, '<git branch>:<git commit hash>')


I changed this to null because I noticed that when it's undefined, the default message is not shown in the help output because of this.

raiyaj · 2022-11-01T20:21:51Z

packages/pwa-kit-dev/scripts/upload.test.js

+beforeEach(() => {
+    realFail = Utils.fail
+    Utils.fail.mockImplementation((errorMessage) => {
+        throw new Error(errorMessage)
+    })
+})


At one point a test in this file was failing because we added a new required option to uploadBundle, and it was tripping me up because the option validation has no effect in the tests unless Utils.fail throws an error.

olibrook · 2022-11-01T21:31:29Z

packages/pwa-kit-dev/bin/pwa-kit-dev.js

+    managedRuntimeCommand('logs')
+        .description(`display environment logs (without --tail, displays this help text and exits)`)
+        .addOption(
+            new program.Option('-p, --project <projectSlug>', 'the project slug').default(
+                null,
+                "the 'name' key from package.json"
+            )
+        )


I find this help text kind of horrendous! Do we have a plan to support pwa-kit-dev logs without --tail soon? You know, in the next few months?

I suspect we don't and we'll be looking at this help text in a year thinking "weird, who thought that was a good idea?"!

Can we do one of:

Rename the command to tail-logs.

Remove the --tail flag and just tail when you run pwa-kit-dev logs, the only thing we have planned.

Keep --tail, but provide some behaviour for pwa-kit-dev logs right away? You could show the first page of logs and then exit, for instance.

If you call it tail-logs I think there's an easy way to build a logs command in future, while maintaining backwards compatibility with an alias...

Ok, I'm convinced. I went with option 1.

kenjush

🚢 🚢

kenjush and others added 8 commits June 15, 2022 17:22

added a log command

a54d174

removed debug code

bb882d7

updated the logs command to work with websocket endpoint

96165b9

Delete unused deps

85528c8

Prettier log lines

4e9e1a4

Ensure you can set the path to the API backend on the CLI

6dd7689

Remove debugging

9898f03

update logs command for new logging infra

3984e71

salesforce-cla bot added the cla:missing label Oct 20, 2022

raiyaj and others added 3 commits October 20, 2022 13:53

Merge branch 'develop' into tail-logs

6b14881

fix bug from merge and make --credentialsFile a global option

0d68168

Merge branch 'develop' into tail-logs

5ffef6e

olibrook reviewed Oct 20, 2022

View reviewed changes

packages/pwa-kit-dev/bin/pwa-kit-dev.js Outdated Show resolved Hide resolved

raiyaj added 9 commits October 24, 2022 11:24

catch JSON.parse error and set url search params more cleanly

65becca

make logs more readable

50583a3

tweak request id parsing

6dacab1

fix save-credentials validation bug

65e37a5

parse the log level too :D

3335c7b

tweak log level formatting

7404383

validate cloud-api-base

7a4c293

fix validation

07708ef

add Oli's managedRuntimeCommand wrapper

2ac4a05

raiyaj added 2 commits October 25, 2022 14:54

read default project slug from package.json (untested)

25ca92c

add npm scripts to templates

093cb59

raiyaj and others added 6 commits November 1, 2022 09:26

make sure project slug default shows if not provided, add --tail arg

a300204

Merge branch 'develop' into tail-logs

5b43f28

tweak test

1fee362

rename variables

86d085a

adjust chalk effects

89bc506

simplify log parsing

7e496a5

raiyaj commented Nov 1, 2022

View reviewed changes

raiyaj marked this pull request as ready for review November 1, 2022 18:56

raiyaj requested a review from a team as a code owner November 1, 2022 18:56

show default msgs in help

1fcdcab

raiyaj commented Nov 1, 2022

View reviewed changes

raiyaj added 2 commits November 1, 2022 13:22

rename variable

735e94e

undo unnecessary change

3da9889

olibrook reviewed Nov 1, 2022

View reviewed changes

rename cmd to tail-logs

0c4ed29

salesforce-cla bot added cla:signed and removed cla:missing labels Nov 2, 2022

raiyaj added 2 commits November 2, 2022 10:18

update npm scripts

4931b6c

handle empty app logs

0d0ab75

raiyaj force-pushed the tail-logs branch from 70b0879 to 0d0ab75 Compare November 3, 2022 18:08

only bold warnings and errors

2e036ff

kenjush approved these changes Nov 3, 2022

View reviewed changes

raiyaj and others added 2 commits November 3, 2022 13:43

update changelog

2e6528d

Merge branch 'develop' into tail-logs

8b04cd9

vmarta merged commit 6f5e79b into develop Nov 7, 2022

vmarta deleted the tail-logs branch November 7, 2022 19:28

This was referenced Dec 13, 2022

Add explicit ws dependency #861

Closed

Add explicit ws dependency #865

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pwa-kit-dev command for tailing logs #789

pwa-kit-dev command for tailing logs #789

raiyaj commented Oct 20, 2022 •

edited

Loading

salesforce-cla bot commented Oct 20, 2022

olibrook Oct 20, 2022

raiyaj Oct 20, 2022 •

edited

Loading

raiyaj Oct 20, 2022

olibrook Oct 20, 2022

raiyaj Oct 20, 2022

olibrook commented Oct 20, 2022

raiyaj commented Oct 21, 2022 •

edited

Loading

salesforce-cla bot commented Oct 25, 2022

raiyaj Nov 1, 2022 •

edited

Loading

raiyaj Nov 1, 2022

raiyaj Nov 1, 2022

raiyaj Nov 1, 2022

raiyaj Nov 1, 2022 •

edited

Loading

olibrook Nov 1, 2022

olibrook Nov 1, 2022

raiyaj Nov 2, 2022

kenjush left a comment

pwa-kit-dev command for tailing logs #789

pwa-kit-dev command for tailing logs #789

Conversation

raiyaj commented Oct 20, 2022 • edited Loading

Description

Types of Changes

Changes

How to Test-Drive This PR

Checklists

General

Accessibility Compliance

Localization

salesforce-cla bot commented Oct 20, 2022

Choose a reason for hiding this comment

raiyaj Oct 20, 2022 • 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

olibrook commented Oct 20, 2022

raiyaj commented Oct 21, 2022 • edited Loading

salesforce-cla bot commented Oct 25, 2022

raiyaj Nov 1, 2022 • 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

raiyaj Nov 1, 2022 • 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

kenjush left a comment

Choose a reason for hiding this comment

raiyaj commented Oct 20, 2022 •

edited

Loading

raiyaj Oct 20, 2022 •

edited

Loading

raiyaj commented Oct 21, 2022 •

edited

Loading

raiyaj Nov 1, 2022 •

edited

Loading

raiyaj Nov 1, 2022 •

edited

Loading